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

Correct AnnotatedIntersectionTypes #3790

Merged
merged 23 commits into from
Oct 20, 2020
Merged

Conversation

smillst
Copy link
Member

@smillst smillst commented Oct 16, 2020

General facts about intersections in javac that should make this pull request easier to review:

  1. In source code, intersections can only be written as a cast type or an upper bound of a type parameter.
    a. For cast types, the intersection type is parsed into an IntersectionTypeTree.
    b. For type variable upper bounds, the intersection type is parsed into a list of trees
  2. In byte code, the Checker Framework only reads annotations on intersections for type parameter upper bounds.
  3. Intersection types can also arise from capture conversion and least upper bounds.
  4. The bounds of explicitly written intersections must be declared types, but intersections from LUB or capture conversion can be any kind of type.

Changes in this pull request:
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.

Copy link
Member

@mernst mernst left a comment

Choose a reason for hiding this comment

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

Overall, this looks good. Thanks! I have a couple clarifying questions.

* location of the intersection type.
*
* <p>For example, {@code @NonNull Object & @Initialized @Nullable Serializable} is changed
* to {@code @NonNull @Initialized Object & @Initialized @NonNull Serializable}.
Copy link
Member

Choose a reason for hiding this comment

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

In this example, the annotation isn't just copied to the primary annotation location, but to every bound of the intersection.

Also, it would be helpful to indicate that the primary annotation location is different than all the annotations on the bounds. The example doesn't actually show the primary annotation location. So, I find the example a bit confusing.

@@ -1016,6 +990,26 @@ protected boolean visitIntersectionSupertype(
return result;
}

/**
* An intersection is a subtype if one of its bounds is a subtype of {@code supertype}.
Copy link
Member

Choose a reason for hiding this comment

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

When this is called, have the primary annotations already been applied to every bound, so that each bound stands on its own without any need to refer to the primary annotations?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. This method is only called on fully-annotated types. That means that the intersection type has a primary annotation for every qualifier hierarchy.

Copy link
Member Author

Choose a reason for hiding this comment

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

TOOD: Add comment.

@mernst mernst assigned smillst and unassigned mernst Oct 17, 2020
@msridhar
Copy link
Contributor

Does this also fix #3569? Just curious.

@mernst
Copy link
Member

mernst commented Oct 18, 2020

Yes, it does. Good catch. I have updated the pull request description and the linked issues.

@smillst smillst merged commit 9dbf0c7 into typetools:master Oct 20, 2020
@smillst smillst deleted the warn-intersections branch October 20, 2020 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants