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

AnnotatedIntersectionTypes are not defaulted correctly #868

Closed
CharlesZ-Chen opened this issue Aug 12, 2016 · 2 comments · Fixed by #3790
Closed

AnnotatedIntersectionTypes are not defaulted correctly #868

CharlesZ-Chen opened this issue Aug 12, 2016 · 2 comments · Fixed by #3790

Comments

@CharlesZ-Chen
Copy link
Contributor

CharlesZ-Chen commented Aug 12, 2016

given test case:

import org.checkerframework.checker.nullness.qual.Nullable;
import java.util.List;
public class Test {
    <E extends @Nullable Object>
    void test (E e) {
        e.toString();
    }    
}

The nullness checker will give a "dereference.of.nullable" error on e.toString(), because the erased type of type variable E would have an annotation @nullable from it's upper bound @Nullable Object as written in the source code.

However, if the upper bound of type variable E becomes an intersection type with @nullable, as below test case showed:

import org.checkerframework.checker.nullness.qual.Nullable;
import java.util.List;
public class Test {
    <E extends @Nullable Object & @Nullable List>
    void test2 (E e) {
        e.toString();
    } 
}

At this time nullness checker won't give any errors and pass this test case.

It seems nullness checker should be expected to give a consistent response for above two test cases, while currently it failed the first one but passed the second one.

Moreover, if add a print statement to print the annotations field of erased atm in AnnotatedTypeVariable#getErased():

@Override
        public AnnotatedTypeMirror getErased() {
            // |T extends A&B| = |A|
            AnnotatedTypeMirror atm = this.getUpperBound().getErased();
            System.out.println("===== typeVar: " + this + "\terased: " + atm + "\terasedAnnos: " + atm.getAnnotations());
            return atm;
        }

The result would be:

first test case:
typeVar: E extends @Initialized @Nullable Object
erased: @Initialized @Nullable Object
erasedAnnos: [@org.checkerframework.checker.initialization.qual.Initialized, @org.checkerframework.checker.nullness.qual.Nullable]

second test case:
typeVar: E extends @Initialized @Nullable Object & @Initialized @Nullable List
erased: @Initialized @Nullable Object & @Initialized @Nullable List
erasedAnnos: [@org.checkerframework.checker.initialization.qual.Initialized, @org.checkerframework.checker.nullness.qual.NonNull]

It seems in the second test case, the erased annotations is @nonnull, but the corresponding erased type is @Initialized @Nullable Object & @Initialized @Nullable List.

I tried to debug it, but it turns out the logic behind getErased() is quite complex. Does anyone have some idea about how to fix it?

Thanks!

@smillst
Copy link
Member

smillst commented Aug 17, 2016

The problem is that the explicit upper bound of a type variable is defaulted to @NonNull even when it is an intersection with annotations on all direct super types. So, an error is issued on a call to test2 with a null argument:

import java.util.List;
import org.checkerframework.checker.nullness.qual.Nullable;

public class Issue868 {
    <E extends @Nullable Object & @Nullable List> void test2(E e) {
        e.toString();
    }

    void use() {
        test2(null);
    }
}
Issue868.java:15: error: [type.argument.type.incompatible] incompatible types in type argument.
        test2(null);
             ^
  found   : null
  required: @Initialized @Nullable Object & @Initialized @Nullable List
1 error

I'll look into fix this.

@smillst smillst self-assigned this Aug 17, 2016
@smillst smillst changed the title AnnotatedTypeVariable#getErased() doesn't handle annotations properly when upper bound of this type variable is Intersection type AnnotatedIntersectionTypes are not defaulted correctly Aug 17, 2016
smillst added a commit that referenced this issue Aug 17, 2016
@mernst mernst added this to the High milestone Aug 17, 2016
@mernst mernst modified the milestones: Critical, High Sep 14, 2016
@mernst mernst modified the milestones: High, Critical Sep 6, 2017
@wmdietl
Copy link
Member

wmdietl commented Sep 27, 2017

This error still occurs in master.

smillst added a commit that referenced this issue Oct 20, 2020
Defaulting rules for the primary annotation of an intersection type:
1. One or more of the bounds have an explicit annotation: the default annotation is the first explicit annotation.  (I tried making the default the glb of the (explicit or defaulted) annotations on the bounds, but I found this confusing in the case of casts.  We can discuss this further.)
2. Zero bounds have an explicit annotation: the default annotation of the location is applied.  (So, either explicit upper bound defaults, or for casts, the annotation of the cast expression.)

Warning:
If any explicit annotation on a bound is not the same as the primary annotation, then a warning is issued.  (I was going to put this in a separate pull request, but it needs to be in this one so that the test cases are easier to understand.)  

Changes to AnnotatedIntersectionType:
1. Adding a primary annotation to an intersection type causes that annotation to be copied to its bounds.  This is similar to what happens when an annotation is added to a type variable.
2. New method `getBounds` returns a list of the bounds of the intersection type.  (This used to be done via `directSuperTypes` method.
3.  The bounds can be any AnnotatedTypeMirror if the intersection arises from capture conversion.

 Fixes #868; Fixes #3349; Fixes #3362; Fixes #3659.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants