-
Notifications
You must be signed in to change notification settings - Fork 347
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
Version of @Pure that persists through side effects #1740
Comments
Thanks for the report! At first I thought that making the constructor for Take this example: import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.dataflow.qual.Pure;
class YourClassNameHere {
int i;
void foo(PureData pureData) {
if (pureData.value() != null) {
this.i = 4;
@NonNull Object o = pureData.value();
}
}
interface PureData {
@Pure
@Nullable Object value();
}
} This results in the same message:
A pure method is only guaranteed to return the same value if the heap didn't change. By storing the result of Please let us know whether this explains the behavior. |
Hi Werner, |
Side effects can change the result of a getter, by changing the private variable that it returns. So you would need a much stronger requirement than "is a simple getter". Are you looking for an annotation that is like |
Yes, an annotation for denoting this method returns an immutable piece of data. AutoValue already has strong guarantees here, but if each of the value methods could be annotated with @immutable or something like that I think it could solve this. |
|
OK, happy for a different annotation, but I hope my point has come across. Maybe if the class is considered an immutable POJO than each of its methods should be considered to never return different data based on any heap updates. So maybe the annotations should be placed at the class level to signify this for each method. As you mentioned a method level annotation makes less sense unless it also signifies the underlying data backing the return value in the class isn't changing. Some ideas for method annotations: Maybe if class is @pojo and @immutable this applies to all methods. |
I agree this would be valuable. My main point is that we need to be careful about the semantics because it would be easy to propose an unsound rule, and about the naming so that programmers can understand it. Thanks for the suggestions! |
As a summary, there is no bug. The Checker Framework is working as intended. However, there is a feature request to make the Checker Framework more expressive and issue fewer false positive warnings. I see three ways we could deal with this problem:
I'm leaving this issue open because we would like to solve it. Anyone who wants to can contribute a pull request that implements one of the possible fixes. |
Checker output and YourClassName.java reproduction case attached.
This is fairly common for use of nano protos where initialization might occur in between copying of data based on nullness of a field.
A workaround for now is to use a local variable before the initialization of mutableData and then assign after initialization based on the local variable.
nullness-issue-pure.zip
The text was updated successfully, but these errors were encountered: