8167000: Refine handling of multiple maximally specific abstract methods
Summary: Bring the compiler in sync with spec changes in JDK-7034913
Reviewed-by: vromero, dlsmith
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java Thu Oct 13 17:31:01 2016 -0700
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java Mon Oct 17 15:02:46 2016 +0100
@@ -30,6 +30,7 @@
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 @@
* 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 @@
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 {
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java Thu Oct 13 17:31:01 2016 -0700
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java Mon Oct 17 15:02:46 2016 +0100
@@ -1691,28 +1691,6 @@
}
}
//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 @@
*/
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
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/8167000/T8167000.java Mon Oct 17 15:02:46 2016 +0100
@@ -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
+ }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/8167000/T8167000.out Mon Oct 17 15:02:46 2016 +0100
@@ -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
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/8167000/T8167000b.java Mon Oct 17 15:02:46 2016 +0100
@@ -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
+ }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/8167000/T8167000b.out Mon Oct 17 15:02:46 2016 +0100
@@ -0,0 +1,2 @@
+T8167000b.java:18:26: compiler.err.unreported.exception.need.to.catch.or.throw: T
+1 error
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/8167000/T8167000c.java Mon Oct 17 15:02:46 2016 +0100
@@ -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(); };
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/8167000/T8167000c.out Mon Oct 17 15:02:46 2016 +0100
@@ -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
--- a/langtools/test/tools/javac/generics/rawOverride/7062745/GenericOverrideTest.java Thu Oct 13 17:31:01 2016 -0700
+++ b/langtools/test/tools/javac/generics/rawOverride/7062745/GenericOverrideTest.java Mon Oct 17 15:02:46 2016 +0100
@@ -120,10 +120,10 @@
}
}
- 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 @@
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 @@
} 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;