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

TYPE_USE locations and their semantics for nullness #17

Closed
wmdietlGC opened this issue Jan 10, 2019 · 13 comments
Closed

TYPE_USE locations and their semantics for nullness #17

wmdietlGC opened this issue Jan 10, 2019 · 13 comments
Labels
design An issue that is resolved by making a decision, about whether and how something should work. nullness For issues specific to nullness analysis.
Milestone

Comments

@wmdietlGC
Copy link
Collaborator

wmdietlGC commented Jan 10, 2019

In bytecode, a TYPE_USE annotation uses a TargetType constant to store what an annotation applies to. It is useful to go through all locations and discuss their semantics, also to ensure we cover all source locations.
A typepath further refines where in the type the annotation applies. Here it is enough to distinguish top-level from non-top level uses.
For nested types, only the deepest nested type can be annotated. All enclosing types are implicitly NotNull and no explicit annotations on outer types are allowed. Note that type arguments to outer types might still be annotated. (This is a bit tricky to check, because depending on the type, an empty type path might be legal or not.)
The reference implementation should enforce correct usage of type annotations.

  • CLASS_TYPE_PARAMETER(0x00)
    Annotation on the type parameter declaration. The lower bound is always NotNull, so forbid annotation.

  • METHOD_TYPE_PARAMETER(0x01)
    Annotation on the type parameter declaration. The lower bound is always NotNull, so forbid annotation.

  • CLASS_EXTENDS(0x10)
    Annotation on the class extends and implements clauses.
    On the top-level annotations make no sense. In type argument positions they are allowed.
    For example: class C extends @Nullable List<@Nullable String> { … }
    The super-class/super-interface are always not-null and no annotations are allowed.
    Type arguments in those types can use annotations.

  • CLASS_TYPE_PARAMETER_BOUND(0x11)
    Annotation on the type parameter bound. Can occur, both on the top-level and inside.

  • METHOD_TYPE_PARAMETER_BOUND(0x12)
    Annotation on the type parameter bound. Can occur, both on the top-level and inside.

  • FIELD(0x13)
    Can occur with arbitrary path.

  • METHOD_RETURN(0x14)
    Can occur with arbitrary path.

  • METHOD_RECEIVER(0x15)
    Method receiver types are always non-null. Forbid all annotations. Even for a generic class, I don't think the receiver ever needs an annotation.

  • METHOD_FORMAL_PARAMETER(0x16)
    Can occur with arbitrary path.

  • THROWS(0x17)
    Types in throws clauses are always non-null and no explicit annotations are allowed. No type arguments are allowed for exception types.
    For example void foo() throws @Nullable MyException { … } is forbidden.

  • LOCAL_VARIABLE(0x40, true)
    Can occur with arbitrary path.

  • RESOURCE_VARIABLE(0x41, true)
    Can occur with arbitrary path.

  • EXCEPTION_PARAMETER(0x42, true)
    Types in catch clauses are always non-null and no explicit annotations are allowed. No type arguments are allowed for exception types.

  • INSTANCEOF(0x43, true)
    See discussion about casts/instanceof in separate issue. The top-level annotation should depend on the cast expression. Type arguments could be annotated, but JVM won’t check.

  • NEW(0x44, true)
    An object creation is always non-null and no top-level annotations are allowed.
    Type arguments can use annotations.

  • CONSTRUCTOR_REFERENCE(0x45, true)
    A constructor reference is always non-null and no top-level annotations are allowed.

  • METHOD_REFERENCE(0x46, true)
    A method reference is always non-null and no top-level annotations are allowed.

  • CAST(0x47, true)
    See discussion about casts/instanceof in separate issue. The top-level annotation should depend on the cast expression. Type arguments could be annotated, but JVM won’t check.

  • CONSTRUCTOR_INVOCATION_TYPE_ARGUMENT(0x48, true)
    Can occur with arbitrary path.

  • METHOD_INVOCATION_TYPE_ARGUMENT(0x49, true)
    Can occur with arbitrary path.

  • CONSTRUCTOR_REFERENCE_TYPE_ARGUMENT(0x4A, true)
    Can occur with arbitrary path.

  • METHOD_REFERENCE_TYPE_ARGUMENT(0x4B, true)
    Can occur with arbitrary path.

  • UNKNOWN(0xFF)
    Should never occur.

@cushon
Copy link
Collaborator

cushon commented Jan 10, 2019

An empty path [] applies the annotation to the top-level type

I think that isn't quite right, see: JDK-8215035.

If the value of path_length is 0,

  • and the type being annotated is a nested type, then the annotation applies to the outermost part of the type for which a type annotation is admissible,
  • otherwise the annotation appears directly on the type itself.

@wmdietlGC
Copy link
Collaborator Author

Thanks for pointing this out!
Do we ever need to annotate an "outer" type? That is, do we have a JLS/JVMS guarantee that the outer class field is always not-null?

Would we ever need to write:

class Outer {
  class Inner {
    void foo(@Nullable Outer.@NotNull Inner param) {...}
  }
}

So maybe we can say that nullness annotations are only allowed on the most-deeply-nested type and all outer classes are NotNull.

@cushon
Copy link
Collaborator

cushon commented Jan 10, 2019

Do we ever need to annotate an "outer" type?

I don't think so for @Nullable. I agree with the general discussion of which target_type locations are relevant, I just wanted to clarify the distinction between an empty type_path and annotating the 'top-level' type. I think it reads OK if you just omit the part about 'empty path' and talk about 'top-level' types more abstractly.

@wmdietlGC
Copy link
Collaborator Author

I've changed the empty path sentence and added a paragraph about nested types. PTAL.

@cushon
Copy link
Collaborator

cushon commented Jan 10, 2019

LG. For "only the deepest nested type can be annotated", the idea is to disallow Middle.@Nullable Outer.Inner, but not necessarily Outer<@Nullable Foo>.Inner?

@wmdietlGC
Copy link
Collaborator Author

Yes, type arguments to outer classes are still necessary. So legal:

  • @Nullable Outer
  • Outer.@Nullable Inner
  • Outer2<@Nullable String>.Inner2
  • Outer2<@Nullable String[]>.@Nullable Inner2

but illegal:

  • @Nullable Outer.Inner
  • @NotNull Outer2<String>.@Nullable Inner
    etc.

@wmdietlGC wmdietlGC changed the title TYPE_USE locations and their semantics TYPE_USE locations and their semantics for nullness Jan 10, 2019
@wmdietlGC wmdietlGC added the nullness For issues specific to nullness analysis. label Jan 10, 2019
@kevinb9n
Copy link
Collaborator

The "For nested types..." part should I think clarify that it's about non-static nested types? This may be clear to those used to the terminology but wasn't to me. If a nested class is static, then the outer class name is used as a simple name qualifier only and I would think it certain that it makes no sense for annotations to be applied to it.

@kevinb9n
Copy link
Collaborator

INSTANCEOF -- we should not permit anything that a user could think was being checked but wasn't.

@kevinb9n
Copy link
Collaborator

Where does that long list say anything about arrays?

@wmdietlGC
Copy link
Collaborator Author

Where does that long list say anything about arrays?

One of the possible type paths is an array. See http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/8aab93346792/src/share/classes/com/sun/tools/javac/code/TypeAnnotationPosition.java#l43 which defines four options:

  • ARRAY,
  • INNER_TYPE,
  • WILDCARD, and
  • TYPE_ARGUMENT.

For reference, target types are here: http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/8aab93346792/src/share/classes/com/sun/tools/javac/code/TargetType.java#l45

So wherever a type-use annotation can appear can be specified by a combination of a target type and a type path.

@dzharkov
Copy link
Collaborator

I wonder how defaulting annotations should work with TYPE_USE applicability:

  • Should it automatically be applied to local variables?
  • Should it be applied to type parameters without explicit bound?

(see the comment by @sdeleuze in our KEEP)

@dzharkov
Copy link
Collaborator

NB: Some time ago when we were designing a similar artifact it was proposed to use a different enum instead of ElementType (see the KEEP)
The enum contains a special entry for TYPE_ARGUMENT in addition to TYPE_USE

@kevinb9n
Copy link
Collaborator

@dzharkov

  1. For now we're trying to leave it fully checker-dependent how they deal with local variables.
  2. This is being debated in other issues, though it's (I think) a bit of a mess right now and I can't cleanly point to which one.

In general I think the topic of this issue is suitably specified in the design draft now.

@kevinb9n kevinb9n added the design An issue that is resolved by making a decision, about whether and how something should work. label Mar 13, 2024
@kevinb9n kevinb9n added this to the 0.3 milestone Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design An issue that is resolved by making a decision, about whether and how something should work. nullness For issues specific to nullness analysis.
Projects
None yet
Development

No branches or pull requests

4 participants