Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to EISOP 3.39-eisop1 from 3.34-eisop1 #439

Merged
merged 17 commits into from
Apr 7, 2024

Conversation

Ao-senXiong
Copy link

@Ao-senXiong Ao-senXiong commented Mar 29, 2024

If this get merged, we can close #413.

PICO's change will be pull in gradually after this change.

Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for performing this large merge! I appreciate that tracking down all these changes took time.

.ci-build-without-test.sh Outdated Show resolved Hide resolved
src/checkers/inference/InferenceValidator.java Outdated Show resolved Hide resolved
Tree varTree,
ExpressionTree valueExp,
@CompilerMessageKey String errorKey,
Object... extraArgs) {
if (!validateTypeOf(varTree)) {
return;
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why true if the tree isn't valid?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems wrong over there, too, doesn't it? Let's clean this up in separate PRs.

@@ -583,7 +594,7 @@ protected void commonAssignmentCheck(
mono.getCanonicalName(),
mono.getCanonicalName(),
valueType.toString());
return;
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The true is again confusing. Can you add a comment that clarifies?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, this one should be return false because of the check fails. And the code here is outdate with super method and should be updated in a seperate PR. @wmdietl I think I will update the copied code after this update.

src/checkers/inference/InferenceVisitor.java Outdated Show resolved Hide resolved
src/checkers/inference/InferenceVisitor.java Outdated Show resolved Hide resolved
src/checkers/inference/VariableAnnotator.java Outdated Show resolved Hide resolved
src/dataflow/qual/DataFlowTop.java Outdated Show resolved Hide resolved
@wmdietl wmdietl assigned Ao-senXiong and unassigned wmdietl Apr 6, 2024
@Ao-senXiong Ao-senXiong requested a review from wmdietl April 7, 2024 16:14
@Ao-senXiong Ao-senXiong assigned wmdietl and unassigned Ao-senXiong Apr 7, 2024
@@ -159,7 +159,7 @@ public void matchAndReplacePrimary(final AnnotatedTypeMirror typeUse, final Anno
}

@Override
protected String defaultErrorMessage(org.checkerframework.framework.type.AnnotatedTypeMirror type1, org.checkerframework.framework.type.AnnotatedTypeMirror type2, Void aVoid) {
public String defaultErrorMessage(org.checkerframework.framework.type.AnnotatedTypeMirror type1, org.checkerframework.framework.type.AnnotatedTypeMirror type2, Void aVoid) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a separate PR, look why this uses the fully-qualified name. The method before uses just ATM.

Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@wmdietl wmdietl merged commit e740339 into opprop:master Apr 7, 2024
12 checks passed
@Ao-senXiong Ao-senXiong deleted the checker-framework-3.39.0-eisop1 branch May 30, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants