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

assignment.type.incompatible only with explicit "extends @Nullable Object" on type parameter #2995

Open
cpovirk opened this issue Dec 17, 2019 · 2 comments

Comments

@cpovirk
Copy link
Contributor

cpovirk commented Dec 17, 2019

I could be confused, but: I would expect class Map<K> and class Map<K extends @Nullable Object> to be equivalent. It appears that they're not. Here's some code that uses the longer version:

$ cat Map.java 
import org.checkerframework.checker.nullness.qual.Nullable;

interface Set<E> {}

class Map<K extends @Nullable Object> {
  Set<K> keySet = new KeySet();

  class KeySet implements Set<K> {}
}

$ checker/bin/javac -processor org.checkerframework.checker.nullness.NullnessChecker Map.java
Map.java:6: error: [assignment.type.incompatible] incompatible types in assignment.
  Set<K> keySet = new KeySet();
                  ^
  found   : @Initialized @NonNull Map<K extends @Initialized @NonNull Object>.@Initialized @NonNull KeySet
  required: @Initialized @NonNull Set<K extends @Initialized @Nullable Object>
1 error

However, if I tweak the class declaration to the "equivalent" shorter version, then the file compiles successfully. That's the behavior I would have expected in either case.

If I suppress the error, the .class files that I get for the long version is almost identical to that for the short version, according to javap -v. The only difference appears to be in the order of the RuntimeVisibleTypeAnnotations.

Source file and -version -verbose -AprintAllQualifiers output attached.

@cpovirk
Copy link
Contributor Author

cpovirk commented Dec 19, 2019

This turns out to affect implementations of the JDK's Map type, since it's defined with explicit <K extends @Nullable Object, V extends @Nullable Object>:

public interface Map<K extends @Nullable Object, V extends @Nullable Object> {

(similarly in typetools/jdk)

Map appears to be one of only 3 classes under checker/jdk/nullness that uses the long version of such bounds. (The other 2 are Map.Entry and HashMap.) Would it be helpful for me to send a PR to change them to plain <K, V> as a workaround until this bug is fixed (assuming that it is a bug)?

@smillst
Copy link
Member

smillst commented Dec 19, 2019

Yes, this is a bug; your test case should not issue any warnings. This may be related to #2926.

Would it be helpful for me to send a PR to change them to plain <K, V> as a workaround until this bug is fixed?

I think that would be helpful. Thanks!

cpovirk added a commit to cpovirk/checker-framework that referenced this issue Dec 19, 2019
It is supposed to be a no-op. As such, the stub files usually omit it.

Currently, it actually changes behavior in a bad way:
typetools#2995

Until that bug is fixed, this commit serves as a workaround.
cpovirk added a commit to cpovirk/typetoolsjdk that referenced this issue Dec 19, 2019
It is supposed to be a no-op. As such, the stub files usually omit it.

Currently, it actually changes behavior in a bad way:
typetools/checker-framework#2995

Until that bug is fixed, this commit serves as a workaround.
smillst pushed a commit that referenced this issue Dec 20, 2019
It is supposed to be a no-op. As such, the stub files usually omit it.

Currently, it actually changes behavior in a bad way:
#2995

Until that bug is fixed, this commit serves as a workaround.
smillst pushed a commit to typetools/jdk that referenced this issue Dec 20, 2019
It is supposed to be a no-op. As such, the stub files usually omit it.

Currently, it actually changes behavior in a bad way:
typetools/checker-framework#2995

Until that bug is fixed, this commit serves as a workaround.
cpovirk added a commit to google/guava that referenced this issue Apr 24, 2020
This passes the Checker Framework only with significant preparations:

- I use custom stub files.
- I pass a particular set of flags to the Checker Framework.
- I add `@SuppressWarnings` on `toArray` methods.
- I replace | and & with || and &&.
- I remove `extends @nullable Object`, as it currently can cause problems: typetools/checker-framework#2995

Another branch will address some of these issues but not all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants