changeset 3707:645100bea379

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
author mcimadamore
date Mon, 17 Oct 2016 15:02:46 +0100
parents 1b4b4f571ccf
children 296c87505118
files src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java test/tools/javac/8167000/T8167000.java test/tools/javac/8167000/T8167000.out test/tools/javac/8167000/T8167000b.java test/tools/javac/8167000/T8167000b.out test/tools/javac/8167000/T8167000c.java test/tools/javac/8167000/T8167000c.out test/tools/javac/generics/rawOverride/7062745/GenericOverrideTest.java
diffstat 9 files changed, 219 insertions(+), 160 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java	Thu Oct 13 17:31:01 2016 -0700
+++ b/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/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java	Thu Oct 13 17:31:01 2016 -0700
+++ b/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/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/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/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/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/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/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/test/tools/javac/generics/rawOverride/7062745/GenericOverrideTest.java	Thu Oct 13 17:31:01 2016 -0700
+++ b/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;