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

Clarify defaulting of type variable uses #746

Merged
merged 7 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,121 +18,130 @@
*/
public enum TypeUseLocation {

/** Apply default annotations to all unannotated raw types of fields. */
/** Apply default annotations to unannotated top-level types of fields. */
FIELD,

/**
* Apply default annotations to all unannotated raw types of local variables, casts, and
* Apply default annotations to unannotated top-level types of local variables, casts, and
* instanceof.
*/
LOCAL_VARIABLE,

/** Apply default annotations to all unannotated raw types of resource variables. */
/** Apply default annotations to unannotated top-level types of resource variables. */
RESOURCE_VARIABLE,

/** Apply default annotations to all unannotated raw types of exception parameters. */
/** Apply default annotations to unannotated top-level types of exception parameters. */
EXCEPTION_PARAMETER,

/** Apply default annotations to all unannotated raw types of receiver types. */
/** Apply default annotations to unannotated top-level types of receiver types. */
RECEIVER,

/**
* Apply default annotations to all unannotated raw types of formal parameter types, excluding
* Apply default annotations to unannotated top-level types of formal parameter types, excluding
* the receiver.
*/
PARAMETER,

/** Apply default annotations to all unannotated raw types of return types. */
/** Apply default annotations to unannotated top-level types of return types. */
RETURN,

/** Apply default annotations to all unannotated raw types of constructor result types. */
/** Apply default annotations to unannotated top-level types of constructor result types. */
CONSTRUCTOR_RESULT,

/**
* Apply default annotations to unannotated lower bounds for type parameters and wildcards, both
* explicit ones in {@code super} clauses, and implicit lower bounds when no explicit {@code
* extends} or {@code super} clause is present.
* Apply default annotations to unannotated top-level lower bounds of type parameters and
* wildcards, both explicit ones in {@code super} clauses, and implicit lower bounds when no
* explicit {@code extends} or {@code super} clause is present.
*/
LOWER_BOUND,

/**
* Apply default annotations to unannotated, but explicit lower bounds of wildcards: {@code <?
* super C>}. Type parameters have no syntax for explicit lower bound types.
* Apply default annotations to unannotated top-level explicit lower bounds of wildcards: {@code
* <? super C>}. Type parameters have no syntax for explicit lower bound types.
*/
EXPLICIT_LOWER_BOUND,

/**
* Apply default annotations to unannotated, but implicit lower bounds for type parameters and
* Apply default annotations to unannotated implicit lower bounds of type parameters and
* wildcards: {@code <T>} and {@code <?>}, possibly with explicit upper bounds.
*/
// Note: no distinction between implicit lower bound when upper bound is explicit or not, in
// contrast to what we do for upper bounds. We can add that if a type system needs it.
IMPLICIT_LOWER_BOUND,

/**
* Apply default annotations to unannotated upper bounds for type parameters and wildcards: both
* explicit ones in {@code extends} clauses, and implicit upper bounds when no explicit {@code
* extends} or {@code super} clause is present.
* Apply default annotations to unannotated top-level upper bounds of type parameters and
* wildcards: both explicit ones in {@code extends} clauses, and implicit upper bounds when no
* explicit {@code extends} or {@code super} clause is present.
*
* <p>Especially useful for parametrized classes that provide a lot of static methods with the
* same generic parameters as the class.
*/
UPPER_BOUND,

/**
* Apply default annotations to unannotated, but explicit type parameter and wildcard upper
* Apply default annotations to unannotated top-level explicit type parameter and wildcard upper
* bounds: {@code <T extends C>} and {@code <? extends C>}.
*/
EXPLICIT_UPPER_BOUND,

/**
* Apply default annotations to unannotated, but explicit type parameter upper bounds: {@code <T
* extends C>}.
* Apply default annotations to unannotated top-level explicit type parameter upper bounds:
* {@code <T extends C>}.
*/
EXPLICIT_TYPE_PARAMETER_UPPER_BOUND,

/**
* Apply default annotations to unannotated, but explicit wildcard upper bounds: {@code <?
* Apply default annotations to unannotated top-level explicit wildcard upper bounds: {@code <?
* extends C>}.
*/
EXPLICIT_WILDCARD_UPPER_BOUND,

/**
* Apply default annotations to unannotated upper bounds for type parameters and wildcards
* Apply default annotations to unannotated upper bounds of type parameters and wildcards
* without explicit upper bounds: {@code <T>}, {@code <?>}, and {@code <? super C>}.
*/
IMPLICIT_UPPER_BOUND,

/**
* Apply default annotations to unannotated upper bounds for type parameters without explicit
* Apply default annotations to unannotated upper bounds of type parameters without explicit
* upper bounds: {@code <T>}.
*/
IMPLICIT_TYPE_PARAMETER_UPPER_BOUND,

/**
* Apply default annotations to unannotated upper bounds for wildcards without a super bound:
* Apply default annotations to unannotated upper bounds of wildcards without a super bound:
* {@code <?>}.
*/
IMPLICIT_WILDCARD_UPPER_BOUND_NO_SUPER,

/**
* Apply default annotations to unannotated upper bounds for wildcards with a super bound:
* {@code <? super C>}.
* Apply default annotations to unannotated upper bounds of wildcards with a super bound: {@code
* <? super C>}.
*/
IMPLICIT_WILDCARD_UPPER_BOUND_SUPER,

/**
* Apply default annotations to unannotated upper bounds for wildcards with or without a super
* Apply default annotations to unannotated upper bounds of wildcards with or without a super
* bound: {@code <?>} or {@code <? super C>}.
*/
IMPLICIT_WILDCARD_UPPER_BOUND,

/**
* Apply default annotations to unannotated type variable uses: {@code T}.
* Apply default annotations to unannotated type variable uses that are not top-level local
* variables: {@code T field} or {@code List<T> local}.
*
* <p>Such uses of type variables are not flow-sensitively refined and are therefore usually
* parametric.
*
* <p>To get parametric polymorphism: add a qualifier that is meta-annotated with {@link
* ParametricTypeVariableUseQualifier} to your type system and use it as default for {@code
* TYPE_VARIABLE_USE}, which is treated like no annotation on the type variable use.
*
* <p>We could name this constant {@code TYPE_VARIABLE_USE_NOT_TOP_LEVEL_LOCAL_VARIABLE} and
* introduce a separate constant {@code TYPE_VARIABLE_USE_TOP_LEVEL_LOCAL_VARIABLE}. At the
* moment we use the {@code LOCAL_VARIABLE} default for unannotated top-level type variable uses
* of local variables: {@code T local}.
*/
TYPE_VARIABLE_USE,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,9 @@ public static boolean noStringMatchesAnyRegex(
* @param iterable an iterable
* @return a list of the results of applying {@code f} to the elements of {@code iterable}
*/
@SuppressWarnings(
Copy link
Member Author

Choose a reason for hiding this comment

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

@Ao-senXiong Can you try to understand why the behavior for the two test cases in the PR changed?
I tried to only clean-up the code, but these two tests changed behavior.
The changes do make sense to me, but they do come as a bit of a surprise.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, also :framework:checkInterning and :dataflow:checkResourceLeak start to fail, so I must have goofed up somewhere...

Copy link
Member Author

Choose a reason for hiding this comment

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

It would also be nice to minimize test cases for these two type systems, to detect these problems earlier.

Copy link
Member

Choose a reason for hiding this comment

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

Working on this.

Copy link
Member

@Ao-senXiong Ao-senXiong Apr 25, 2024

Choose a reason for hiding this comment

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

@wmdietl Here is the minimal test case to reproduce interning checking failure in this branch:

public class Demo {

    public <T extends Dummy<?,?>> T test(){
        T t = getT();
        return t;
    }

    public <T extends Dummy<?, ?>> T getT(){
        return null;
    }

    public class Dummy<A,B>{

    }
}
java -jar checker/dist/checker.jar -processor org.checkerframework.checker.interning.InterningChecker Demo.java
Demo.java:5: error: [return.type.incompatible] incompatible types in return.
        return t;
               ^
  type of expression: T[ extends @UnknownInterned Demo.@UnknownInterned Dummy<?[ extends @UnknownInterned Object super @InternedDistinct Void], ?[ extends @UnknownInterned Object super @InternedDistinct Void]> super @UnknownInterned Void]
  method return type: T[ extends @UnknownInterned Demo.@UnknownInterned Dummy<?[ extends @UnknownInterned Object super @InternedDistinct Void], ?[ extends @UnknownInterned Object super @InternedDistinct Void]> super @InternedDistinct Void]

The lower bound of type parameter is not incompatible. If I use the master branch, it is fine.

Copy link
Member

Choose a reason for hiding this comment

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

A follow up here, looks like the type refinement works fine, however, the actual type of right hand side is changed.
In master:

InterningVisitor about to test whether actual is a subtype of expected (at Demo.java:4:15): actual tree = METHOD_INVOCATION getT()
     actual: TYPEVAR T[ extends @UnknownInterned Demo.@UnknownInterned Dummy<?[ extends @UnknownInterned Object super @InternedDistinct Void], ?[ extends @UnknownInterned Object super @InternedDistinct Void]> super @InternedDistinct Void]
   expected: TYPEVAR T[ extends @UnknownInterned Demo.@UnknownInterned Dummy<?[ extends @UnknownInterned Object super @InternedDistinct Void], ?[ extends @UnknownInterned Object super @InternedDistinct Void]> super @UnknownInterned Void]

In this branch

InterningVisitor about to test whether actual is a subtype of expected (at Demo.java:4:15): actual tree = METHOD_INVOCATION getT()
     actual: TYPEVAR T[ extends @UnknownInterned Demo.@UnknownInterned Dummy<?[ extends @UnknownInterned Object super @InternedDistinct Void], ?[ extends @UnknownInterned Object super @InternedDistinct Void]> super @UnknownInterned Void]
   expected: TYPEVAR T[ extends @UnknownInterned Demo.@UnknownInterned Dummy<?[ extends @UnknownInterned Object super @InternedDistinct Void], ?[ extends @UnknownInterned Object super @InternedDistinct Void]> super @UnknownInterned Void]

Copy link
Member

Choose a reason for hiding this comment

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

This can also be confirmed by this test case works fine in this branch. I will debug and see why the right hand side of return type of method invocation is changed.

public class Demo {

    public <T extends Dummy<?,?>> T test(){
        T t = null;
        return t;
    }

    public class Dummy<A,B>{

    }
}

"builder:required.method.not.known") // TODO: `FROM elt : iterable` gives error.
// Error message says this is expected for un-constrained type variables, which we have here.
public static <
@KeyForBottom FROM extends @Nullable @UnknownKeyFor Object,
@KeyForBottom TO extends @Nullable @UnknownKeyFor Object>
Expand Down
Loading
Loading