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

GLB used during capture conversion isn't the same as Types#glb. #4888

Merged
merged 12 commits into from
Aug 19, 2021
16 changes: 16 additions & 0 deletions checker/tests/tainting/CaptureSubtype.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import org.checkerframework.checker.tainting.qual.Tainted;
import org.checkerframework.checker.tainting.qual.Untainted;

public class CaptureSubtype {

class MyGeneric<T extends @Tainted Number> {}

class SubGeneric<T extends @Untainted Number> extends MyGeneric<T> {}

class UseMyGeneric {
SubGeneric<? extends @Tainted Object> wildcardUnbounded = new SubGeneric<@Untainted Number>();

MyGeneric<?> wildcardOutsideUB = wildcardUnbounded;
MyGeneric<? extends @Untainted Number> wildcardInsideUB2 = wildcardUnbounded;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5057,6 +5057,11 @@ private void annotateCapturedTypeVar(
typeVarToAnnotatedTypeArg, typeVariable.getUpperBound());
AnnotatedTypeMirror upperBound =
AnnotatedTypes.annotatedGLB(this, typeVarUpperBound, wildcard.getExtendsBound());
// There is a bug in javac such that the upper bound of the captured type variable is not the
// greatest lower bound. So the captureTypeVar.getUnderlyingType().getUpperBound() may not
// be the same type as upperbound.getUnderlyingType(). See
// framework/tests/all-systems/Issue4890Interfaces.java,
// framework/tests/all-systems/Issue4890.java and framework/tests/all-systems/Issue4877.java.
capturedTypeVar.setUpperBound(upperBound);

// typeVariable's lower bound is a NullType, so there's nothing to substitute.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,22 +440,14 @@ public Boolean visitDeclared_Declared(
return true;
}

// JLS 11: 4.10.2. Subtyping among Class and Interface Types
// 4th paragraph, bullet 1.
AnnotatedDeclaredType subtypeAsSuper =
AnnotatedTypes.castedAsSuper(subtype.atypeFactory, subtype, supertype);

if (isSubtypeVisitHistory.contains(subtypeAsSuper, supertype, currentTop)) {
if (isSubtypeVisitHistory.contains(subtype, supertype, currentTop)) {
return true;
}

final boolean result =
visitTypeArgs(
subtypeAsSuper,
supertype,
subtype.isUnderlyingTypeRaw(),
supertype.isUnderlyingTypeRaw());
isSubtypeVisitHistory.put(subtypeAsSuper, supertype, currentTop, result);
subtype, supertype, subtype.isUnderlyingTypeRaw(), supertype.isUnderlyingTypeRaw());
isSubtypeVisitHistory.put(subtype, supertype, currentTop, result);

return result;
}
Expand All @@ -464,8 +456,8 @@ public Boolean visitDeclared_Declared(
* Returns true if the type arguments in {@code supertype} contain the type arguments in {@code
* subtype} and false otherwise. See {@link #isContainedBy} for an explanation of containment.
*
* @param subtype a possible subtype (its underlying type is the same as {@code supertype})
* @param supertype a possible supertype (its underlying type is the same as {@code subtype})
* @param subtype a possible subtype
* @param supertype a possible supertype
* @param subtypeRaw whether {@code subtype} is a raw type
* @param supertypeRaw whether {@code supertype} is a raw type
* @return true if the type arguments in {@code supertype} contain the type arguments in {@code
Expand All @@ -476,25 +468,30 @@ protected boolean visitTypeArgs(
AnnotatedDeclaredType supertype,
final boolean subtypeRaw,
final boolean supertypeRaw) {
AnnotatedTypeFactory typeFactory = subtype.atypeFactory;

// JLS 11: 4.10.2. Subtyping among Class and Interface Types
// 4th paragraph, bullet 1.
AnnotatedDeclaredType subtypeAsSuper =
AnnotatedTypes.castedAsSuper(typeFactory, subtype, supertype);

if (ignoreRawTypes && (subtypeRaw || supertypeRaw)) {
return true;
}

final List<? extends AnnotatedTypeMirror> subtypeTypeArgs = subtype.getTypeArguments();
final List<? extends AnnotatedTypeMirror> subtypeTypeArgs = subtypeAsSuper.getTypeArguments();
final List<? extends AnnotatedTypeMirror> supertypeTypeArgs = supertype.getTypeArguments();

if (subtypeTypeArgs.size() != supertypeTypeArgs.size()) {
throw new BugInCF("Type arguments are not the same size: %s %s", subtype, supertype);
throw new BugInCF("Type arguments are not the same size: %s %s", subtypeAsSuper, supertype);
}
// This method, `visitTypeArgs`, is called even if `subtype` doesn't have type arguments.
if (subtypeTypeArgs.isEmpty()) {
return true;
}

final TypeElement supertypeElem = (TypeElement) supertype.getUnderlyingType().asElement();
AnnotationMirror covariantAnno =
supertype.atypeFactory.getDeclAnnotation(supertypeElem, Covariant.class);
AnnotationMirror covariantAnno = typeFactory.getDeclAnnotation(supertypeElem, Covariant.class);

List<Integer> covariantArgIndexes =
(covariantAnno == null)
Expand All @@ -505,7 +502,8 @@ protected boolean visitTypeArgs(
// JLS 11: 4.10.2. Subtyping among Class and Interface Types
// 4th paragraph, bullet 2
try {
if (isContainedMany(subtype.getTypeArguments(), supertypeTypeArgs, covariantArgIndexes)) {
if (isContainedMany(
subtypeAsSuper.getTypeArguments(), supertypeTypeArgs, covariantArgIndexes)) {
return true;
}
} catch (Exception e) {
Expand All @@ -514,9 +512,11 @@ protected boolean visitTypeArgs(
// 5th paragraph:
// Instead of calling isSubtype with the captured type, just check for containment.
AnnotatedDeclaredType capturedSubtype =
(AnnotatedDeclaredType) subtype.atypeFactory.applyCaptureConversion(subtype);
(AnnotatedDeclaredType) typeFactory.applyCaptureConversion(subtype);
AnnotatedDeclaredType capturedSubtypeAsSuper =
AnnotatedTypes.castedAsSuper(typeFactory, capturedSubtype, supertype);
return isContainedMany(
capturedSubtype.getTypeArguments(), supertypeTypeArgs, covariantArgIndexes);
capturedSubtypeAsSuper.getTypeArguments(), supertypeTypeArgs, covariantArgIndexes);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -794,17 +794,16 @@ public static AnnotatedTypeMirror leastUpperBound(
*/
public static AnnotatedTypeMirror annotatedGLB(
AnnotatedTypeFactory atypeFactory, AnnotatedTypeMirror type1, AnnotatedTypeMirror type2) {
TypeMirror glbJava =
TypesUtils.greatestLowerBound(
type1.getUnderlyingType(), type2.getUnderlyingType(), atypeFactory.getProcessingEnv());
Types types = atypeFactory.types;
if (types.isSubtype(type1.getUnderlyingType(), type2.getUnderlyingType())) {
return glbSubtype(atypeFactory.getQualifierHierarchy(), type1, type2);
} else if (types.isSubtype(type2.getUnderlyingType(), type1.getUnderlyingType())) {
return glbSubtype(atypeFactory.getQualifierHierarchy(), type2, type1);
}

TypeMirror glbJava =
TypesUtils.greatestLowerBound(
type1.getUnderlyingType(), type2.getUnderlyingType(), atypeFactory.getProcessingEnv());

if (types.isSameType(type1.getUnderlyingType(), glbJava)) {
return glbSubtype(atypeFactory.getQualifierHierarchy(), type1, type2);
} else if (types.isSameType(type2.getUnderlyingType(), glbJava)) {
Expand Down Expand Up @@ -834,6 +833,20 @@ public static AnnotatedTypeMirror annotatedGLB(
newBounds.add(type1.deepCopy());
} else if (types.isSameType(bound.getUnderlyingType(), type2.getUnderlyingType())) {
newBounds.add(type2.deepCopy());
} else if (type1.getKind() == TypeKind.INTERSECTION) {
AnnotatedIntersectionType intertype1 = (AnnotatedIntersectionType) type1;
for (AnnotatedTypeMirror otherBound : intertype1.getBounds()) {
if (types.isSameType(bound.getUnderlyingType(), otherBound.getUnderlyingType())) {
newBounds.add(otherBound.deepCopy());
}
}
} else if (type2.getKind() == TypeKind.INTERSECTION) {
AnnotatedIntersectionType intertype2 = (AnnotatedIntersectionType) type2;
for (AnnotatedTypeMirror otherBound : intertype2.getBounds()) {
if (types.isSameType(bound.getUnderlyingType(), otherBound.getUnderlyingType())) {
newBounds.add(otherBound.deepCopy());
}
}
} else {
throw new BugInCF(
"Neither %s nor %s is one of the intersection bounds in %s. Bound: %s",
Expand Down
15 changes: 15 additions & 0 deletions framework/tests/all-systems/Issue4877.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
@SuppressWarnings("all") // Just check for crashes.
public class Issue4877 {

interface NInterface<T extends NInterface<T>> {}

interface OInterface<T extends NInterface<T>> {}

interface XInterface {}

static class QClass<M extends NInterface<M> & XInterface> {}

static final class PClass<T extends NInterface<T>> implements OInterface<T> {
QClass<? extends T> f;
}
}
25 changes: 25 additions & 0 deletions framework/tests/all-systems/Issue4890.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import java.util.function.Function;
import org.checkerframework.checker.nullness.qual.NonNull;

@SuppressWarnings("all") // Just check for crashes.
public class Issue4890 {

class R<P extends PK, E extends N<P>, K> {}

interface PK {}

interface N<K extends PK> {}

interface Q<K extends S<P>, P extends @NonNull Object> extends N<K> {}

interface S<P> extends PhysicalPK {}

interface PhysicalPK extends PK {}

interface I<T, R> extends Function<T, R> {}

class T {

I<String, R<? extends S<Integer>, ? extends Q<?, Integer>, ?>> reader;
}
}
29 changes: 29 additions & 0 deletions framework/tests/all-systems/Issue4890Interfaces.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
@SuppressWarnings("all") // Just check for crashes.
public class Issue4890Interfaces {
// The capture of BigClass2<Interface1, ? extends SubInterface2<?>> is BigClass<Interface1, cap1>
// where cap1 is a fresh type variable with upper bound of
// SubInterface2<?> & Interface2<Interface1> and lower bound of nulltype.
BigClass2<Interface1, ? extends SubInterface2<?>> r;
// The type of r.getI2() is cap1 extends SubInterface2<?> & Interface2<Interface1> so
// both of the following assignments should be legal.
SubInterface2<?> s = r.getI2();
// Interface2<Interface1> s2 = r.getI2(); // javac error here, but no error with IntelliJ and
// Eclipse.

BigClass2<SubInterface1, ? extends SubInterface2<?>> t;
SubInterface2<?> s3 = t.getI2();
// Interface2<SubInterface1> s4 = t.getI2(); // javac error here, but no error with IntelliJ and
// Eclipse.

abstract static class BigClass2<I1 extends Interface1, I2 extends Interface2<I1>> {
abstract I2 getI2();
}

interface Interface1 {}

interface SubInterface1 extends Interface1 {}

interface Interface2<A extends Interface1> {}

interface SubInterface2<C extends SubInterface1> extends Interface2<C> {}
}