7034798: Ambiguity error for abstract method call is too eager

Summary: Javac should wait and see if ambiguous methods can be reconciled at the end of an overload resolution round
Reviewed-by: jjg, vromero

src/share/classes/com/sun/tools/javac/comp/Resolve.java

@@ -1573,7 +1573,6 @@ public class Resolve {
                           allowBoxing,
                           useVarargs,
                           operator);
-        reportVerboseResolutionDiagnostic(env.tree.pos(), name, site, argtypes, typeargtypes, bestSoFar);
         return bestSoFar;
     }
     // where
@@ -2224,7 +2223,7 @@ public class Resolve {
         return lookupMethod(env, pos, env.enclClass.sym, resolveMethodCheck,
                 new BasicLookupHelper(name, env.enclClass.sym.type, argtypes, typeargtypes) {
                     @Override
-                    Symbol lookup(Env<AttrContext> env, MethodResolutionPhase phase) {
+                    Symbol doLookup(Env<AttrContext> env, MethodResolutionPhase phase) {
                         return findFun(env, name, argtypes, typeargtypes,
                                 phase.isBoxingRequired(),
                                 phase.isVarargsRequired());
@@ -2256,7 +2255,7 @@ public class Resolve {
                                   List<Type> typeargtypes) {
         return lookupMethod(env, pos, location, resolveContext, new BasicLookupHelper(name, site, argtypes, typeargtypes) {
             @Override
-            Symbol lookup(Env<AttrContext> env, MethodResolutionPhase phase) {
+            Symbol doLookup(Env<AttrContext> env, MethodResolutionPhase phase) {
                 return findMethod(env, site, name, argtypes, typeargtypes,
                         phase.isBoxingRequired(),
                         phase.isVarargsRequired(), false);
@@ -2355,7 +2354,7 @@ public class Resolve {
                               List<Type> typeargtypes) {
         return lookupMethod(env, pos, site.tsym, resolveContext, new BasicLookupHelper(names.init, site, argtypes, typeargtypes) {
             @Override
-            Symbol lookup(Env<AttrContext> env, MethodResolutionPhase phase) {
+            Symbol doLookup(Env<AttrContext> env, MethodResolutionPhase phase) {
                 return findConstructor(pos, env, site, argtypes, typeargtypes,
                         phase.isBoxingRequired(),
                         phase.isVarargsRequired());
@@ -2413,7 +2412,7 @@ public class Resolve {
         return lookupMethod(env, pos, site.tsym, resolveMethodCheck,
                 new BasicLookupHelper(names.init, site, argtypes, typeargtypes) {
                     @Override
-                    Symbol lookup(Env<AttrContext> env, MethodResolutionPhase phase) {
+                    Symbol doLookup(Env<AttrContext> env, MethodResolutionPhase phase) {
                         return findDiamond(env, site, argtypes, typeargtypes,
                                 phase.isBoxingRequired(),
                                 phase.isVarargsRequired());
@@ -2503,7 +2502,7 @@ public class Resolve {
             return lookupMethod(env, pos, syms.predefClass, currentResolutionContext,
                     new BasicLookupHelper(name, syms.predefClass.type, argtypes, null, BOX) {
                 @Override
-                Symbol lookup(Env<AttrContext> env, MethodResolutionPhase phase) {
+                Symbol doLookup(Env<AttrContext> env, MethodResolutionPhase phase) {
                     return findMethod(env, site, name, argtypes, typeargtypes,
                             phase.isBoxingRequired(),
                             phase.isVarargsRequired(), true);
@@ -2668,6 +2667,13 @@ public class Resolve {
          */
         abstract Symbol lookup(Env<AttrContext> env, MethodResolutionPhase phase);
 
+        /**
+         * Dump overload resolution info
+         */
+        void debug(DiagnosticPosition pos, Symbol sym) {
+            //do nothing
+        }
+
         /**
          * Validate the result of the lookup
          */
@@ -2685,17 +2691,30 @@ public class Resolve {
         }
 
         @Override
-        Symbol access(Env<AttrContext> env, DiagnosticPosition pos, Symbol location, Symbol sym) {
+        final Symbol lookup(Env<AttrContext> env, MethodResolutionPhase phase) {
+            Symbol sym = doLookup(env, phase);
             if (sym.kind == AMBIGUOUS) {
                 AmbiguityError a_err = (AmbiguityError)sym;
                 sym = a_err.mergeAbstracts(site);
             }
+            return sym;
+        }
+
+        abstract Symbol doLookup(Env<AttrContext> env, MethodResolutionPhase phase);
+
+        @Override
+        Symbol access(Env<AttrContext> env, DiagnosticPosition pos, Symbol location, Symbol sym) {
             if (sym.kind >= AMBIGUOUS) {
                 //if nothing is found return the 'first' error
                 sym = accessMethod(sym, pos, location, site, name, true, argtypes, typeargtypes);
             }
             return sym;
         }
+
+        @Override
+        void debug(DiagnosticPosition pos, Symbol sym) {
+            reportVerboseResolutionDiagnostic(pos, name, site, argtypes, typeargtypes, sym);
+        }
     }
 
     /**
@@ -2924,7 +2943,9 @@ public class Resolve {
                 MethodResolutionPhase prevPhase = currentResolutionContext.step;
                 Symbol prevBest = bestSoFar;
                 currentResolutionContext.step = phase;
-                bestSoFar = phase.mergeResults(bestSoFar, lookupHelper.lookup(env, phase));
+                Symbol sym = lookupHelper.lookup(env, phase);
+                lookupHelper.debug(pos, sym);
+                bestSoFar = phase.mergeResults(bestSoFar, sym);
                 env.info.pendingResolutionPhase = (prevBest == bestSoFar) ? prevPhase : phase;
             }
             return lookupHelper.access(env, pos, location, bestSoFar);
@@ -3630,35 +3651,39 @@ public class Resolve {
          * is more specific than the others, attempt to merge their signatures.
          */
         Symbol mergeAbstracts(Type site) {
-            Symbol fst = ambiguousSyms.last();
-            Symbol res = fst;
-            for (Symbol s : ambiguousSyms.reverse()) {
-                Type mt1 = types.memberType(site, res);
-                Type mt2 = types.memberType(site, s);
-                if ((s.flags() & ABSTRACT) == 0 ||
-                        !types.overrideEquivalent(mt1, mt2) ||
-                        !types.isSameTypes(fst.erasure(types).getParameterTypes(),
-                                       s.erasure(types).getParameterTypes())) {
-                    //ambiguity cannot be resolved
-                    return this;
-                } else {
-                    Type mst = mostSpecificReturnType(mt1, mt2);
-                    if (mst == null) {
-                        // Theoretically, this can't happen, but it is possible
-                        // due to error recovery or mixing incompatible class files
+            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;
                     }
-                    Symbol mostSpecific = mst == mt1 ? res : s;
-                    List<Type> allThrown = chk.intersect(mt1.getThrownTypes(), mt2.getThrownTypes());
-                    Type newSig = types.createMethodTypeWithThrown(mostSpecific.type, allThrown);
-                    res = new MethodSymbol(
-                            mostSpecific.flags(),
-                            mostSpecific.name,
-                            newSig,
-                            mostSpecific.owner);
+                    Type mst = mostSpecificReturnType(mt, mt2);
+                    if (mst == null || mst != mt) {
+                        found = false;
+                        break;
+                    }
+                    allThrown = chk.intersect(allThrown, mt2.getThrownTypes());
+                }
+                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(mt, allThrown),
+                                s.owner);
                 }
             }
-            return res;
+            return this;
         }
 
         @Override

test/tools/javac/resolve/ResolveHarness.java

@@ -32,6 +32,8 @@
 
 import com.sun.source.util.JavacTask;
 import com.sun.tools.javac.api.ClientCodeWrapper.DiagnosticSourceUnwrapper;
+import com.sun.tools.javac.code.Flags;
+import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type.MethodType;
 import com.sun.tools.javac.util.JCDiagnostic;
 
@@ -154,7 +156,7 @@ public class ResolveHarness implements javax.tools.DiagnosticListener<JavaFileOb
         //check all candidates have been used up
         for (Map.Entry<ElementKey, Candidate> entry : candidatesMap.entrySet()) {
             if (!seenCandidates.contains(entry.getKey())) {
-                error("Redundant @Candidate annotation on method " + entry.getKey().elem);
+                error("Redundant @Candidate annotation on method " + entry.getKey().elem + " sig = " + entry.getKey().elem.asType());
             }
         }
     }
@@ -250,7 +252,7 @@ public class ResolveHarness implements javax.tools.DiagnosticListener<JavaFileOb
         void process(Diagnostic<? extends JavaFileObject> diagnostic) {
             Element siteSym = getSiteSym(diagnostic);
             if (siteSym.getSimpleName().length() != 0 &&
-                    siteSym.getAnnotation(TraceResolve.class) == null) {
+                    ((Symbol)siteSym).outermostClass().getAnnotation(TraceResolve.class) == null) {
                 return;
             }
             int candidateIdx = 0;
@@ -308,7 +310,11 @@ public class ResolveHarness implements javax.tools.DiagnosticListener<JavaFileOb
 
         @Override
         void process(Diagnostic<? extends JavaFileObject> diagnostic) {
-            Element methodSym = methodSym(diagnostic);
+            Symbol methodSym = (Symbol)methodSym(diagnostic);
+            if ((methodSym.flags() & Flags.GENERATEDCONSTR) != 0) {
+                //skip resolution of default constructor (put there by javac)
+                return;
+            }
             Candidate c = getCandidateAtPos(methodSym,
                     asJCDiagnostic(diagnostic).getLineNumber(),
                     asJCDiagnostic(diagnostic).getColumnNumber());
@@ -470,23 +476,10 @@ public class ResolveHarness implements javax.tools.DiagnosticListener<JavaFileOb
         }
 
         String computeKey(Element e) {
-            StringBuilder buf = new StringBuilder();
-            if (predefTranslationMap.containsKey(e.getSimpleName().toString())) {
-                //predef element
-                buf.append("<predef>.");
-                String replacedName = predefTranslationMap.get(e.getSimpleName().toString());
-                buf.append(e.toString().replace(e.getSimpleName().toString(), replacedName));
-            } else if (e.getSimpleName().toString().startsWith("_")) {
-                buf.append("<predef>.");
-                buf.append(e.toString());
-            } else {
-                while (e != null) {
-                    buf.append(e.toString());
-                    e = e.getEnclosingElement();
-                }
-                buf.append(jfo.getName());
-            }
-            return buf.toString();
+            String simpleName = e.getSimpleName().toString();
+            String opName = predefTranslationMap.get(simpleName);
+            String name = opName != null ? opName : simpleName;
+            return name + e.asType();
         }
 
         @Override

test/tools/javac/resolve/tests/AbstractMerge.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+@TraceResolve
+class AbstractMerge {
+
+    interface A {
+        @Candidate(applicable=Phase.BASIC)
+        java.io.Serializable m1();
+        @Candidate(applicable=Phase.BASIC)
+        java.io.Serializable m2();
+        @Candidate(applicable=Phase.BASIC)
+        java.io.Serializable m3();
+        @Candidate(applicable=Phase.BASIC)
+        java.io.Serializable m4();
+        @Candidate(applicable=Phase.BASIC)
+        java.io.Serializable m5();
+        @Candidate(applicable=Phase.BASIC)
+        java.io.Serializable m6();
+    }
+
+    interface B {
+        @Candidate(applicable=Phase.BASIC)
+        Cloneable m1();
+        @Candidate(applicable=Phase.BASIC)
+        Cloneable m2();
+        @Candidate(applicable=Phase.BASIC)
+        Cloneable m3();
+        @Candidate(applicable=Phase.BASIC)
+        Cloneable m4();
+        @Candidate(applicable=Phase.BASIC)
+        Cloneable m5();
+        @Candidate(applicable=Phase.BASIC)
+        Cloneable m6();
+    }
+
+    interface C {
+        @Candidate(applicable=Phase.BASIC, mostSpecific=true)
+        Object[] m1();
+        @Candidate(applicable=Phase.BASIC, mostSpecific=true)
+        Object[] m2();
+        @Candidate(applicable=Phase.BASIC, mostSpecific=true)
+        Object[] m3();
+        @Candidate(applicable=Phase.BASIC, mostSpecific=true)
+        Object[] m4();
+        @Candidate(applicable=Phase.BASIC, mostSpecific=true)
+        Object[] m5();
+        @Candidate(applicable=Phase.BASIC, mostSpecific=true)
+        Object[] m6();
+    }
+
+    interface ABC extends A, B, C { }
+    interface ACB extends A, C, B { }
+    interface BAC extends B, A, C { }
+    interface BCA extends B, C, A { }
+    interface CAB extends C, A, B { }
+    interface CBA extends C, B, A { }
+
+    {
+        ABC abc = null;
+        abc.m1();
+    }
+
+    {
+        ACB acb = null;
+        acb.m2();
+    }
+
+    {
+        BAC bac = null;
+        bac.m3();
+    }
+
+    {
+        BCA bca = null;
+        bca.m4();
+    }
+
+    {
+        CAB cab = null;
+        cab.m5();
+    }
+
+    {
+        CBA cba = null;
+        cba.m6();
+    }
+}

test/tools/javac/resolve/tests/InnerOverOuter.java

@@ -21,7 +21,7 @@
  * questions.
  */
 
-@TraceResolve
+@TraceResolve(keys={"compiler.err.cant.apply.symbol"})
 class Test {
 
     //no annotation here - this should NOT even be considered!
@@ -30,7 +30,6 @@ class Test {
     //no annotation here - this should NOT even be considered!
     void m(Object... o) { }
 
-    @TraceResolve(keys={"compiler.err.cant.apply.symbol"})
     class Inner {
         @Candidate
         void m(String s) {