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

False nullability errors in 3.42.0 #6389

Closed
kennknowles opened this issue Jan 9, 2024 · 4 comments
Closed

False nullability errors in 3.42.0 #6389

kennknowles opened this issue Jan 9, 2024 · 4 comments

Comments

@kennknowles
Copy link
Contributor

kennknowles commented Jan 9, 2024

Reproduce:

git clone https://github.com/kennknowles/beam --branch checkerframework-StructuralEqualityComparer
cd beam
./gradlew :sdks:java:core:compileJava

Output:

> Task :sdks:java:core:compileJava
./beam/sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java:359: error: [dereference.of.nullable] dereference of possibly-null reference currentReader
                currentReader.close();
                ^
./beam/sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java:367: error: [dereference.of.nullable] dereference of possibly-null reference currentReader
                    currentReader.getCurrent(), currentReader.getCurrentTimestamp());
                    ^
./beam/sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java:367: error: [dereference.of.nullable] dereference of possibly-null reference currentReader
                    currentReader.getCurrent(), currentReader.getCurrentTimestamp());
                                                ^
./beam/sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java:373: error: [dereference.of.nullable] dereference of possibly-null reference currentReader
              currentReader.close();
              ^

etc.

Expectation: these are not errors. This field currentReader was set to a non-nullable value with no intervening method calls.

@kennknowles
Copy link
Contributor Author

While bisecting for a good checker version for Beam, confirmed that this (and many others) also occurs in checker 3.40.0

@kennknowles
Copy link
Contributor Author

kennknowles commented Jan 9, 2024

I don't know if it is the same, but most of the issues I see are consistent with the following hypothesis: Given an object o with field x with a non-null value, the algorithm seems to believe that calling o.x.m() can now cause o.x to become null. I suppose this is true if there is a circular dependency where o.x.m() eventually has a handle on o and sets o.x to null? I have never seen this but makes sense - is this the case? Is there a way to elide this? Otherwise literally every method call in the language has the potential to set the receiver to null, basically.

I also saw some cases even if x.m() is declared @Pure and checked against null but a repeat call did not maintain the nullness of the result.

@mernst
Copy link
Member

mernst commented Jan 9, 2024

Kenn, your hypothesis is correct: the Nullness Checker reasons that it is conceivable that a method has a reference to a field and sets the field. Because its aim is an iron-clad guarantee, it issues a warning in this case. In many cases this is a false positive warning, as you point out.

The manual gives ways to specify your program so that the Nullness Checker can reason about (lack of) re-assignment, in section "Side effects, determinism, purity, and type refinement". Maybe one of these can work for you. In general, the solution to a false positive is often writing a specification (that is, annotations) to provide more information to the checker.

I also saw some cases even if x.m() is declared @Pure and checked against null but a repeat call did not maintain the nullness of the result.

This sounds like a bug. Could you please report it? But, a question first: is it possible that x was side-effected between the two calls x.m()? If so, the warning is a correct one.

More generally, if you have specific cases, please let us know about them individually and we can help you reason through them.

@kennknowles
Copy link
Contributor Author

Got it. Yes I will see if I can find a good way to manage these particular errors. If I reproduce the tangentially mentioned case, I will file a new issue.

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

No branches or pull requests

2 participants