Skip to content

Commit

Permalink
Require parameters in overrides to match those of the overridden method.
Browse files Browse the repository at this point in the history
The default CF behavior is to allow for contravariance. JSpecify may
eventually follow CF (jspecify/jspecify#49),
and even if it doesn't, any given checker could choose to support it. I
wouldn't have bothered to make this change except that I was having
problems with the `Ordering.min`/`max` case:

jspecify/jspecify@972fad0#diff-73db3d90f7545c1ed55a628fdf803a47b14f37bd08cb1eb0aedfa45b44bf1ddfR53
https://github.com/google/guava/blob/751d7c0d5f1fe3cbc067987e7d185763affa9ec7/guava/src/com/google/common/collect/ReverseOrdering.java#L54

I have not fully understood what's going wrong in that case. From the
error message, I get the impression that CF is perhaps not "adapting the
formal parameter types of N to the the type parameters of M" (JLS
8.4.2). Or rather, it does but only in the case of a _top-level_ type
variable, meaning _not_ in the array-of-type-variable-usages case?

https://github.com/typetools/checker-framework/blob/fd0abaa8b8fea775aaf71057d93d9415355b665e/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java#L3883

(I forget whether the similar `Lib<E>` case worked, but I suspect so.
Maybe that's covered by CF's contravariance and then by the specifics of
how it compares type variables to one another?)
  • Loading branch information
cpovirk committed Nov 20, 2020
1 parent 28089b9 commit ff45ce7
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedUnionType;
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedWildcardType;
import org.checkerframework.framework.type.AnnotatedTypeParameterBounds;
import org.checkerframework.framework.type.DefaultTypeHierarchy;
import org.checkerframework.framework.type.GenericAnnotatedTypeFactory;
import org.checkerframework.framework.type.QualifierHierarchy;
import org.checkerframework.framework.type.TypeHierarchy;
Expand Down Expand Up @@ -3861,13 +3862,9 @@ private boolean checkParameters() {
boolean result = true;
for (int i = 0; i < overriderParams.size(); ++i) {
boolean success =
atypeFactory
.getTypeHierarchy()
.isSubtype(overriddenParams.get(i), overriderParams.get(i));
if (!success) {
success =
testTypevarContainment(overriddenParams.get(i), overriderParams.get(i));
}
((DefaultTypeHierarchy) atypeFactory.getTypeHierarchy())
.publicAreEqualInHierarchy(
overriddenParams.get(i), overriderParams.get(i));

checkParametersMsg(success, i, overriderParams, overriddenParams);
result &= success;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,11 @@ protected boolean areEqualInHierarchy(
return equalityComparer.areEqualInHierarchy(type1, type2, currentTop);
}

public final boolean publicAreEqualInHierarchy(
final AnnotatedTypeMirror type1, final AnnotatedTypeMirror type2) {
return areEqualInHierarchy(type1, type2);
}

/**
* A declared type is considered a supertype of another declared type only if all of the type
* arguments of the declared type "contain" the corresponding type arguments of the subtype.
Expand Down

0 comments on commit ff45ce7

Please sign in to comment.