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

Scan through members in InitializationFieldAccessVisitor #619

Merged
merged 7 commits into from
Nov 21, 2023
Merged

Conversation

wmdietl
Copy link
Member

@wmdietl wmdietl commented Nov 18, 2023

The problem only occurs when the enclosing type is an annotation type, e.g. if Issue612Min is changed from an @interface to a class, the NPE doesn't happen.
So maybe there is some more localized fix that undoes some special handling for annotation types.

@flo2702 can you have a look at this and see whether you find a better fix? The issue was introduced with #444 .

Fixes #612 .

@@ -24,6 +28,9 @@ public void processClassTree(ClassTree classTree) {
// and InitializationChecker, this checker performs the flow analysis
// (which is handled in the BaseTypeVisitor), but does not perform
// any type checking.
// Thus, this method does nothing.
// Thus, this method does nothing but scan through the members.
if (!assumeInitialized) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@flo2702 This is the key change in this PR. Sorry for adding so many other changes while tracking this down.
I think I'll merge this PR now, to fix the exception. Can you open a new PR if you find a better fix?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. I'll keep looking, but haven't found anything better.

@@ -413,12 +413,12 @@ Before: InitializationStore#93(
~~~~~~~~~
o [ VariableDeclaration ]
(this) [ ImplicitThis ]
(this).f [ FieldAccess ] > CFAV{@Initialized, Object}
o = (this).f [ Assignment ] > CFAV{@Initialized, Object}
(this).f [ FieldAccess ] > CFAV{, T}
Copy link
Member Author

Choose a reason for hiding this comment

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

@flo2702 These changes come a bit as a surprise, but look good to me - field f has type T, so the new types look more precise than before, where it used the erasure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like scanning through all members once in the visitor somehow allows more precise types to be used during the dataflow refinement. Probably something to consider while I look for a better fix, but I admit that I don't really understand how the visitor affects the refinement in this way.

@wmdietl wmdietl enabled auto-merge (squash) November 21, 2023 19:22
@wmdietl wmdietl disabled auto-merge November 21, 2023 20:25
@wmdietl wmdietl merged commit ac37192 into master Nov 21, 2023
15 of 33 checks passed
@wmdietl wmdietl deleted the issue-612 branch November 21, 2023 20:25
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.

NullPointerException in GenericAnnotatedTypeFactory.getFirstNodeOfKindForTree(
2 participants