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.

Sign up for GitHub

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

Jump to bottom

Model the enclosing expression node for ObjectCreationNode and type check the enclosing expression for inner classes #405

Merged
merged 21 commits into from
Feb 23, 2023
Merged

Model the enclosing expression node for ObjectCreationNode and type check the enclosing expression for inner classes #405

merged 21 commits into from
Feb 23, 2023

Conversation

al3xliu
Copy link
Collaborator

@al3xliu al3xliu commented Feb 19, 2023 •
edited by wmdietl
Loading

Fixes #282 .

@wmdietl wmdietl mentioned this pull request Feb 20, 2023
Closed
wmdietl
wmdietl reviewed Feb 21, 2023
View reviewed changes
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.

Thank you for all the improvements to this!

checker/tests/nullness-enclosingexpr/NullnessEnclosingExprTest.java Outdated
class InnerWithExplicitEnclosingExpression1 {
InnerWithExplicitEnclosingExpression1(
@UnknownInitialization NullnessEnclosingExprTest NullnessEnclosingExprTest.this) {
// This will NOT lead to a NPE as we annotate @UnknownInitialization to the enclosing
Copy link
Member

Choose a reason for hiding this comment

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

It should not lead to an NPE, because the dereference should be forbidden! Do you not get an error from the Nullness Checker on the dereference of f? If not, this is a problem!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I cannot get an error on this behavior, do you think we need to fix it in this pr?

al3xliu and others added 12 commits February 20, 2023 19:02
@al3xliu
Merge branch 'master' into issue282
3fbde03
@al3xliu
initial new issue282
fa8bc4d
@al3xliu @wmdietl
Update framework/src/main/java/org/checkerframework/common/basetype/B…
e448f3d 
…aseTypeVisitor.java

Co-authored-by: Werner Dietl <wdietl@gmail.com>
@al3xliu @wmdietl
Update framework/src/main/java/org/checkerframework/common/basetype/B…
987503f 
…aseTypeVisitor.java

Co-authored-by: Werner Dietl <wdietl@gmail.com>
@al3xliu @wmdietl
Update framework/src/main/java/org/checkerframework/common/basetype/B…
8dc1ca6 
…aseTypeVisitor.java

Co-authored-by: Werner Dietl <wdietl@gmail.com>
@al3xliu @wmdietl
Update framework/src/main/java/org/checkerframework/common/basetype/B…
8b28ede 
…aseTypeVisitor.java

Co-authored-by: Werner Dietl <wdietl@gmail.com>
@al3xliu @wmdietl
Update framework/src/main/java/org/checkerframework/common/aliasing/A…
fb1423d 
…liasingTransfer.java

Co-authored-by: Werner Dietl <wdietl@gmail.com>
@al3xliu @wmdietl
Update framework/src/main/java/org/checkerframework/common/basetype/B…
b9e46d4 
…aseTypeVisitor.java

Co-authored-by: Werner Dietl <wdietl@gmail.com>
@al3xliu @wmdietl
Update framework/src/main/java/org/checkerframework/framework/source/…
7155f8a 
…SourceChecker.java

Co-authored-by: Werner Dietl <wdietl@gmail.com>
@al3xliu @wmdietl
Update checker/src/test/java/org/checkerframework/checker/test/junit/…
09857c8 
…NullnessEnclosingExprTest.java

Co-authored-by: Werner Dietl <wdietl@gmail.com>
@al3xliu
refine code
251566e
@al3xliu
refine code
f4bd9ac
@al3xliu al3xliu requested a review from wmdietl February 21, 2023 01:21
wmdietl
wmdietl reviewed Feb 21, 2023
View reviewed changes
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 the quick fixes! A few more comments.

dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/CFGTranslationPhaseOne.java Outdated
if (enclosingExpr != null) {
scan(enclosingExpr, p);
enclosingExprNode = scan(enclosingExpr, p);
} else if (enclosingClassType.getKind() == TypeKind.DECLARED) {
Copy link
Member

Choose a reason for hiding this comment

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

What other options are there? Maybe variable enclosingClassType shouldn't contain Class if it's not guaranteed to be a class?

Copy link
Collaborator Author

@al3xliu al3xliu Feb 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! The TypeKind of a static enclosing type is Typekind.None, so we need this condition. I change the name in my local.

@al3xliu al3xliu requested a review from wmdietl February 21, 2023 20:01
This was referenced Feb 22, 2023
Closed
Open
wmdietl
wmdietl approved these changes Feb 22, 2023
View reviewed changes
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! Small comments I'll merge and then I'll merge once tests pass.

@wmdietl wmdietl merged commit 3e41fa5 into eisop:master Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wmdietl wmdietl wmdietl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ObjectCreationNode does not model receiver expression in object creations
2 participants
@al3xliu @wmdietl