8167000: Refine handling of multiple maximally specific abstract methods

Bring the compiler in sync with spec changes in JDK-7034913

Reviewed-by: vromero, dlsmith
This commit is contained in:
Maurizio Cimadamore 2016-10-17 15:02:46 +01:00
parent 1f91f70a58
commit 80ce1c8be2
9 changed files with 219 additions and 160 deletions

View File

@ -30,6 +30,7 @@ import java.util.HashSet;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.function.BiPredicate;
@ -468,89 +469,14 @@ public class Types {
* and return-type substitutable with each method in the original list.
*/
private FunctionDescriptor mergeDescriptors(TypeSymbol origin, List<Symbol> methodSyms) {
//pick argument types - simply take the signature that is a
//subsignature of all other signatures in the list (as per JLS 8.4.2)
List<Symbol> mostSpecific = List.nil();
outer: for (Symbol msym1 : methodSyms) {
Type mt1 = memberType(origin.type, msym1);
for (Symbol msym2 : methodSyms) {
Type mt2 = memberType(origin.type, msym2);
if (!isSubSignature(mt1, mt2)) {
continue outer;
}
}
mostSpecific = mostSpecific.prepend(msym1);
}
if (mostSpecific.isEmpty()) {
return null;
}
//pick return types - this is done in two phases: (i) first, the most
//specific return type is chosen using strict subtyping; if this fails,
//a second attempt is made using return type substitutability (see JLS 8.4.5)
boolean phase2 = false;
Symbol bestSoFar = null;
while (bestSoFar == null) {
outer: for (Symbol msym1 : mostSpecific) {
Type mt1 = memberType(origin.type, msym1);
for (Symbol msym2 : methodSyms) {
Type mt2 = memberType(origin.type, msym2);
if (phase2 ?
!returnTypeSubstitutable(mt1, mt2) :
!isSubtypeInternal(mt1.getReturnType(), mt2.getReturnType())) {
continue outer;
return mergeAbstracts(methodSyms, origin.type, false)
.map(bestSoFar -> new FunctionDescriptor(bestSoFar.baseSymbol()) {
@Override
public Type getType(Type origin) {
Type mt = memberType(origin, getSymbol());
return createMethodTypeWithThrown(mt, bestSoFar.type.getThrownTypes());
}
}
bestSoFar = msym1;
}
if (phase2) {
break;
} else {
phase2 = true;
}
}
if (bestSoFar == null) return null;
//merge thrown types - form the intersection of all the thrown types in
//all the signatures in the list
List<Type> thrown = null;
Type mt1 = memberType(origin.type, bestSoFar);
boolean toErase = !mt1.hasTag(FORALL);
for (Symbol msym2 : methodSyms) {
Type mt2 = memberType(origin.type, msym2);
List<Type> thrown_mt2 = mt2.getThrownTypes();
if (toErase) {
thrown_mt2 = erasure(thrown_mt2);
} else {
/* If bestSoFar is generic then all the methods are generic.
* The opposite is not true: a non generic method can override
* a generic method (raw override) so it's safe to cast mt1 and
* mt2 to ForAll.
*/
ForAll fa1 = (ForAll)mt1;
ForAll fa2 = (ForAll)mt2;
thrown_mt2 = subst(thrown_mt2, fa2.tvars, fa1.tvars);
}
thrown = (thrown == null) ?
thrown_mt2 :
chk.intersect(thrown_mt2, thrown);
}
final List<Type> thrown1 = thrown;
return new FunctionDescriptor(bestSoFar) {
@Override
public Type getType(Type origin) {
Type mt = memberType(origin, getSymbol());
return createMethodTypeWithThrown(mt, thrown1);
}
};
}
boolean isSubtypeInternal(Type s, Type t) {
return (s.isPrimitive() && t.isPrimitive()) ?
isSameType(t, s) :
isSubtype(s, t);
}).orElse(null);
}
FunctionDescriptorLookupError failure(String msg, Object... args) {
@ -2604,6 +2530,106 @@ public class Types {
return false;
}
/**
* This enum defines the strategy for implementing most specific return type check
* during the most specific and functional interface checks.
*/
public enum MostSpecificReturnCheck {
/**
* Return r1 is more specific than r2 if {@code r1 <: r2}. Extra care required for (i) handling
* method type variables (if either method is generic) and (ii) subtyping should be replaced
* by type-equivalence for primitives. This is essentially an inlined version of
* {@link Types#resultSubtype(Type, Type, Warner)}, where the assignability check has been
* replaced with a strict subtyping check.
*/
BASIC() {
@Override
public boolean test(Type mt1, Type mt2, Types types) {
List<Type> tvars = mt1.getTypeArguments();
List<Type> svars = mt2.getTypeArguments();
Type t = mt1.getReturnType();
Type s = types.subst(mt2.getReturnType(), svars, tvars);
return types.isSameType(t, s) ||
!t.isPrimitive() &&
!s.isPrimitive() &&
types.isSubtype(t, s);
}
},
/**
* Return r1 is more specific than r2 if r1 is return-type-substitutable for r2.
*/
RTS() {
@Override
public boolean test(Type mt1, Type mt2, Types types) {
return types.returnTypeSubstitutable(mt1, mt2);
}
};
public abstract boolean test(Type mt1, Type mt2, Types types);
}
/**
* Merge multiple abstract methods. The preferred method is a method that is a subsignature
* of all the other signatures and whose return type is more specific {@see MostSpecificReturnCheck}.
* The resulting preferred method has a thrown clause that is the intersection of the merged
* methods' clauses.
*/
public Optional<Symbol> mergeAbstracts(List<Symbol> ambiguousInOrder, Type site, boolean sigCheck) {
//first check for preconditions
boolean shouldErase = false;
List<Type> erasedParams = ambiguousInOrder.head.erasure(this).getParameterTypes();
for (Symbol s : ambiguousInOrder) {
if ((s.flags() & ABSTRACT) == 0 ||
(sigCheck && !isSameTypes(erasedParams, s.erasure(this).getParameterTypes()))) {
return Optional.empty();
} else if (s.type.hasTag(FORALL)) {
shouldErase = true;
}
}
//then merge abstracts
for (MostSpecificReturnCheck mostSpecificReturnCheck : MostSpecificReturnCheck.values()) {
outer: for (Symbol s : ambiguousInOrder) {
Type mt = memberType(site, s);
List<Type> allThrown = mt.getThrownTypes();
for (Symbol s2 : ambiguousInOrder) {
if (s != s2) {
Type mt2 = memberType(site, s2);
if (!isSubSignature(mt, mt2) ||
!mostSpecificReturnCheck.test(mt, mt2, this)) {
//ambiguity cannot be resolved
continue outer;
} else {
List<Type> thrownTypes2 = mt2.getThrownTypes();
if (!mt.hasTag(FORALL) && shouldErase) {
thrownTypes2 = erasure(thrownTypes2);
} else if (mt.hasTag(FORALL)) {
//subsignature implies that if most specific is generic, then all other
//methods are too
Assert.check(mt2.hasTag(FORALL));
// if both are generic methods, adjust thrown types ahead of intersection computation
thrownTypes2 = subst(thrownTypes2, mt2.getTypeArguments(), mt.getTypeArguments());
}
allThrown = chk.intersect(allThrown, thrownTypes2);
}
}
}
return (allThrown == mt.getThrownTypes()) ?
Optional.of(s) :
Optional.of(new MethodSymbol(
s.flags(),
s.name,
createMethodTypeWithThrown(s.type, allThrown),
s.owner) {
@Override
public Symbol baseSymbol() {
return s;
}
});
}
}
return Optional.empty();
}
// <editor-fold defaultstate="collapsed" desc="Determining method implementation in given site">
class ImplementationCache {

View File

@ -1691,28 +1691,6 @@ public class Resolve {
}
}
//where
Type mostSpecificReturnType(Type mt1, Type mt2) {
Type rt1 = mt1.getReturnType();
Type rt2 = mt2.getReturnType();
if (mt1.hasTag(FORALL) && mt2.hasTag(FORALL)) {
//if both are generic methods, adjust return type ahead of subtyping check
rt1 = types.subst(rt1, mt1.getTypeArguments(), mt2.getTypeArguments());
}
//first use subtyping, then return type substitutability
if (types.isSubtype(rt1, rt2)) {
return mt1;
} else if (types.isSubtype(rt2, rt1)) {
return mt2;
} else if (types.returnTypeSubstitutable(mt1, mt2)) {
return mt1;
} else if (types.returnTypeSubstitutable(mt2, mt1)) {
return mt2;
} else {
return null;
}
}
//where
Symbol ambiguityError(Symbol m1, Symbol m2) {
if (((m1.flags() | m2.flags()) & CLASH) != 0) {
return (m1.flags() & CLASH) == 0 ? m1 : m2;
@ -4112,43 +4090,7 @@ public class Resolve {
*/
Symbol mergeAbstracts(Type site) {
List<Symbol> ambiguousInOrder = ambiguousSyms.reverse();
for (Symbol s : ambiguousInOrder) {
Type mt = types.memberType(site, s);
boolean found = true;
List<Type> allThrown = mt.getThrownTypes();
for (Symbol s2 : ambiguousInOrder) {
Type mt2 = types.memberType(site, s2);
if ((s2.flags() & ABSTRACT) == 0 ||
!types.overrideEquivalent(mt, mt2) ||
!types.isSameTypes(s.erasure(types).getParameterTypes(),
s2.erasure(types).getParameterTypes())) {
//ambiguity cannot be resolved
return this;
}
Type mst = mostSpecificReturnType(mt, mt2);
if (mst == null || mst != mt) {
found = false;
break;
}
List<Type> thrownTypes2 = mt2.getThrownTypes();
if (mt.hasTag(FORALL) && mt2.hasTag(FORALL)) {
// if both are generic methods, adjust thrown types ahead of intersection computation
thrownTypes2 = types.subst(thrownTypes2, mt2.getTypeArguments(), mt.getTypeArguments());
}
allThrown = chk.intersect(allThrown, thrownTypes2);
}
if (found) {
//all ambiguous methods were abstract and one method had
//most specific return type then others
return (allThrown == mt.getThrownTypes()) ?
s : new MethodSymbol(
s.flags(),
s.name,
types.createMethodTypeWithThrown(s.type, allThrown),
s.owner);
}
}
return this;
return types.mergeAbstracts(ambiguousInOrder, site, true).orElse(this);
}
@Override

View File

@ -0,0 +1,34 @@
/*
* @test /nodynamiccopyright/
* @bug 8167000
* @summary Refine handling of multiple maximally specific abstract methods
* @compile/fail/ref=T8167000.out -XDrawDiagnostics -Werror -Xlint:unchecked T8167000.java
*/
import java.util.*;
class T8167000 {
interface J {
List<Number> getAll(String str);
}
interface K {
Collection<Integer> getAll(String str);
}
interface L {
List getAll(String str);
}
interface M {
Collection getAll(String str);
}
static abstract class E implements J, K, L, M {
void test() {
List<String> l = getAll(""); //check that we get an unchecked warning here
}
}
}

View File

@ -0,0 +1,4 @@
T8167000.java:31:36: compiler.warn.prob.found.req: (compiler.misc.unchecked.assign), java.util.List, java.util.List<java.lang.String>
- compiler.err.warnings.and.werror
1 error
1 warning

View File

@ -0,0 +1,21 @@
/*
* @test /nodynamiccopyright/
* @bug 8167000
* @summary Refine handling of multiple maximally specific abstract methods
* @compile/fail/ref=T8167000b.out -XDrawDiagnostics T8167000b.java
*/
public class T8167000b {
interface A {
Integer m() throws Throwable;
}
interface B<X extends Throwable> {
Object m() throws X;
}
static abstract class E<T extends Throwable> implements A, B<T> {
void test() {
Integer l = m(); //error: unhandled T
}
}
}

View File

@ -0,0 +1,2 @@
T8167000b.java:18:26: compiler.err.unreported.exception.need.to.catch.or.throw: T
1 error

View File

@ -0,0 +1,21 @@
/*
* @test /nodynamiccopyright/
* @bug 8167000
* @summary Refine handling of multiple maximally specific abstract methods
* @compile/fail/ref=T8167000c.out -XDrawDiagnostics T8167000c.java
*/
public class T8167000c<X extends Throwable> {
interface A {
Integer m() throws Throwable;
}
interface B<X extends Throwable> {
Object m() throws X;
}
interface E<T extends Throwable> extends A, B<T> { }
void test() {
E<X> ex = () -> { throw new Throwable(); };
}
}

View File

@ -0,0 +1,4 @@
T8167000c.java:19:27: compiler.err.unreported.exception.need.to.catch.or.throw: java.lang.Throwable
- compiler.note.unchecked.filename: T8167000c.java
- compiler.note.unchecked.recompile
1 error

View File

@ -120,10 +120,10 @@ public class GenericOverrideTest extends ComboInstance<GenericOverrideTest> {
}
}
boolean moreSpecificThan(TypeArgumentKind that, boolean strict) {
boolean moreSpecificThan(TypeArgumentKind that) {
switch (this) {
case NONE:
return that == this || !strict;
return that == this;
case UNBOUND:
return that == this || that == NONE;
case INTEGER:
@ -198,6 +198,7 @@ public class GenericOverrideTest extends ComboInstance<GenericOverrideTest> {
void check(Result<?> res) {
boolean errorExpected = false;
boolean loose = false;
int mostSpecific = 0;
//first check that either |R1| <: |R2| or |R2| <: |R1|
@ -208,39 +209,43 @@ public class GenericOverrideTest extends ComboInstance<GenericOverrideTest> {
} else {
mostSpecific = rets[0].moreSpecificThan(rets[1]) ? 1 : 2;
}
} else if (sigs[0] != sigs[1]) {
mostSpecific = sigs[0] == SignatureKind.GENERIC ? 2 : 1;
loose = true;
}
//check that either TA1 <= TA2 or TA2 <= TA1 (unless most specific return found above is raw)
if (!errorExpected) {
if (targs[0] != targs[1]) {
boolean useStrictCheck = targs[0].moreSpecificThan(targs[1], true) ||
targs[1].moreSpecificThan(targs[0], true);
if (!targs[0].moreSpecificThan(targs[1], useStrictCheck) &&
!targs[1].moreSpecificThan(targs[0], useStrictCheck)) {
boolean ta1ms = targs[0].moreSpecificThan(targs[1]);
boolean ta2ms = targs[1].moreSpecificThan(targs[0]);
if (!ta1ms && !ta2ms) {
errorExpected = true;
} else if (mostSpecific != 0) {
errorExpected = !loose && targs[mostSpecific - 1] != TypeArgumentKind.NONE &&
(mostSpecific == 1 ? !ta1ms : !ta2ms);
} else {
int mostSpecific2 = targs[0].moreSpecificThan(targs[1], useStrictCheck) ? 1 : 2;
if (mostSpecific != 0 && mostSpecific2 != mostSpecific) {
errorExpected = mostSpecific == 1 ?
targs[0] != TypeArgumentKind.NONE :
targs[1] != TypeArgumentKind.NONE;
} else {
mostSpecific = mostSpecific2;
}
mostSpecific = ta1ms ? 1 : 2;
}
} else if (mostSpecific == 0) {
//when no signature is better than the other, an arbitrary choice
//must be made - javac always picks the second signature
mostSpecific = 2;
}
}
//finally, check that most specific return type is compatible with expected type
if (mostSpecific == 0) {
//when no signature is better than the other, an arbitrary choice
//must be made - javac always picks the second signature
mostSpecific = 2;
}
if (!errorExpected) {
ReturnTypeKind msrt = mostSpecific == 1 ? rets[0] : rets[1];
TypeArgumentKind msta = mostSpecific == 1 ? targs[0] : targs[1];
SignatureKind mssig = mostSpecific == 1 ? sigs[0] : sigs[1];
//check that most specific is subsignature
errorExpected = sigs[0] != sigs[1] &&
mssig == SignatureKind.GENERIC;
//finally, check that most specific return type is compatible with expected type
if (!msrt.moreSpecificThan(rets[2]) ||
!msta.assignableTo(targs[2], mssig, level)) {
errorExpected = true;