Skip to content

A wildcard with an implicit upper bound applies its own defaulting rules to its corresponding type parameter's bound #3845

@cpovirk

Description

@cpovirk
$ cat DefaultClash.java
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.framework.qual.DefaultQualifier;
import org.checkerframework.framework.qual.TypeUseLocation;

class Holder<T> {
  final T value;

  Holder(T value) {
    this.value = value;
  }
}

interface HolderSupplier<H extends Holder<Object>> {
  H get();
}

@DefaultQualifier(value = Nullable.class, locations = TypeUseLocation.ALL)
class DefaultClash {
  Object go(HolderSupplier<?> s) {
    if (s != null) {
      return s.get();
    }
    return "";
  }
}
checker-framework-3.7.0 $ checker/bin/javac -processor org.checkerframework.checker.nullness.NullnessChecker DefaultClash.java
DefaultClash.java:19: error: [type.argument.type.incompatible] incompatible type argument for type parameter H of HolderSupplier.
  Object go(HolderSupplier<?> s) {
                           ^
  found   : ? extends @Initialized @NonNull Holder<@Initialized @Nullable Object>
  required: @Initialized @NonNull Holder<@Initialized @NonNull Object>
1 error

At a high level, I would expect for HolderSupplier<?> to always be valid, with CF filling in an appropriate value for the implicit upper bound.

Drilling in a little more: If I'm reading the code right, the @Nullable Object argument to Holder comes from the default that's in effect at the use site -- that is, where HolderSupplier<?> is written. I would instead expect for it to use the default of @NonNull Object, since that's the default in effect where Holder<Object> appears in the source code.

Among the code involved is here:

} else if (wildcard.getTypeVariable() != null) {
// Otherwise use the upper bound of the type variable associated with this wildcard.
javaExtendsBound = wildcard.getTypeVariable().getUpperBound();
} else {
// Otherwise use the upper bound of the java wildcard.
javaExtendsBound =
TypesUtils.wildUpperBound(
javaWildcardType, wildcard.atypeFactory.processingEnv);

That gives us a TypeMirror, which is then converted to an unannotated AnnotatedTypeMirror to be filled in later by the defaults in effect at the wildcard site:

AnnotatedTypeMirror.createType(javaExtendsBound, typeFactory, false);

It seems like what we'd want for the type-parameter bound is to create the AnnotatedTypeMirror based on the defaults in effect at the type-parameter declaration.

We do have access at this point to the AnnotatedTypeFactory, so perhaps we could call getAnnotatedType on wildcard.getTypeVariable().asElement() (and then extract getUpperBound()). (When getTypeVariable() is not available, we could probably get what we need from wildcardToTypeParam((com.sun.tools.javac.code.Type.WildcardType) wildcard.getUnderlyingType()).) That would (I think) apply appropriate defaults. But I worry that that would be approaching the problem at the wrong "level." In particular, getAnnotatedType could call back into BoundsInitializer (I think?), and I wonder if that could produce infinite loops with recursive generics like Comparable<E extends Comparable<? super E>> (or otherwise violate assumptions).

The good news is that the example above is just an unnecessary warning, rather than an unsoundness. But it seems likely that it could be turned into unsoundness. And I did actually hit this one in some actual code, unlike half of my reports so far :)

(One other bad idea I considered: Setting the bound not to the type parameter's bounds but rather to a usage of the corresponding type variable. For example, for class Foo<N extends Number> {} Foo<?> foo;, I could treat Foo<?> as Foo<? extends N>. But this is wrong because later logic would sometimes substitute in specific values for N, which would be incorrect.)

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions