7188968: New instance creation expression using diamond is checked twice

Summary: Unify method and constructor check logic
Reviewed-by: jjg

src/share/classes/com/sun/tools/javac/code/Symbol.java

@@ -168,6 +168,10 @@ public abstract class Symbol implements Element {
         return owner;
     }
 
+    public Symbol baseSymbol() {
+        return this;
+    }
+
     /** The symbol's erased type.
      */
     public Type erasure(Types types) {

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

@@ -1730,39 +1730,16 @@ public class Attr extends JCTree.Visitor {
         List<Type> typeargtypes = attribTypes(tree.typeargs, localEnv);
 
         if (TreeInfo.isDiamond(tree) && !clazztype.isErroneous()) {
-            clazztype = attribDiamond(localEnv, tree, clazztype, argtypes, typeargtypes);
-            clazz.type = clazztype;
-        } else if (allowDiamondFinder &&
-                tree.def == null &&
-                !clazztype.isErroneous() &&
-                clazztype.getTypeArguments().nonEmpty() &&
-                findDiamonds) {
-            boolean prevDeferDiags = log.deferDiagnostics;
-            Queue<JCDiagnostic> prevDeferredDiags = log.deferredDiagnostics;
-            Type inferred = null;
-            try {
-                //disable diamond-related diagnostics
-                log.deferDiagnostics = true;
-                log.deferredDiagnostics = ListBuffer.lb();
-                inferred = attribDiamond(localEnv,
-                        tree,
-                        clazztype,
-                        argtypes,
-                        typeargtypes);
-            }
-            finally {
-                log.deferDiagnostics = prevDeferDiags;
-                log.deferredDiagnostics = prevDeferredDiags;
-            }
-            if (inferred != null &&
-                    !inferred.isErroneous() &&
-                    inferred.tag == CLASS &&
-                    types.isAssignable(inferred, pt().tag == NONE ? clazztype : pt(), Warner.noWarnings)) {
-                String key = types.isSameType(clazztype, inferred) ?
-                    "diamond.redundant.args" :
-                    "diamond.redundant.args.1";
-                log.warning(tree.clazz.pos(), key, clazztype, inferred);
+            Pair<Symbol, Type> diamondResult =
+                    attribDiamond(localEnv, tree, clazztype, argtypes, typeargtypes);
+            tree.clazz.type = types.createErrorType(clazztype);
+            tree.constructor = diamondResult.fst;
+            tree.constructorType = diamondResult.snd;
+            if (!diamondResult.snd.isErroneous()) {
+                tree.clazz.type = clazztype = diamondResult.snd.getReturnType();
+                tree.constructorType = types.createMethodTypeWithReturn(diamondResult.snd, syms.voidType);
             }
+            clazztype = chk.checkClassType(tree.clazz, tree.clazz.type, true);
         }
 
         // If we have made no mistakes in the class type...
@@ -1796,7 +1773,7 @@ public class Attr extends JCTree.Visitor {
             // Resolve the called constructor under the assumption
             // that we are referring to a superclass instance of the
             // current instance (JLS ???).
-            else {
+            else if (!TreeInfo.isDiamond(tree)) {
                 //the following code alters some of the fields in the current
                 //AttrContext - hence, the current context must be dup'ed in
                 //order to avoid downstream failures
@@ -1805,17 +1782,47 @@ public class Attr extends JCTree.Visitor {
                 rsEnv.info.varArgs = false;
                 tree.constructor = rs.resolveConstructor(
                     tree.pos(), rsEnv, clazztype, argtypes, typeargtypes);
-                tree.constructorType = tree.constructor.type.isErroneous() ?
-                    syms.errType :
-                    checkConstructor(clazztype,
-                        tree.constructor,
-                        rsEnv,
-                        tree.args,
-                        argtypes,
-                        typeargtypes,
-                        rsEnv.info.varArgs);
-                if (rsEnv.info.varArgs)
-                    Assert.check(tree.constructorType.isErroneous() || tree.varargsElement != null);
+                if (cdef == null) { //do not check twice!
+                    tree.constructorType = checkId(tree,
+                            clazztype,
+                            tree.constructor,
+                            rsEnv,
+                            new ResultInfo(MTH, newMethodTemplate(syms.voidType, argtypes, typeargtypes)),
+                            rsEnv.info.varArgs);
+                    if (rsEnv.info.varArgs)
+                        Assert.check(tree.constructorType.isErroneous() || tree.varargsElement != null);
+                }
+                if (tree.def == null &&
+                        !clazztype.isErroneous() &&
+                        clazztype.getTypeArguments().nonEmpty() &&
+                        findDiamonds) {
+                    boolean prevDeferDiags = log.deferDiagnostics;
+                    Queue<JCDiagnostic> prevDeferredDiags = log.deferredDiagnostics;
+                    Type inferred = null;
+                    try {
+                        //disable diamond-related diagnostics
+                        log.deferDiagnostics = true;
+                        log.deferredDiagnostics = ListBuffer.lb();
+                        inferred = attribDiamond(localEnv,
+                                tree,
+                                clazztype,
+                                argtypes,
+                                typeargtypes).snd;
+                    } finally {
+                        log.deferDiagnostics = prevDeferDiags;
+                        log.deferredDiagnostics = prevDeferredDiags;
+                    }
+                    if (!inferred.isErroneous()) {
+                        inferred = inferred.getReturnType();
+                    }
+                    if (inferred != null &&
+                            types.isAssignable(inferred, pt().tag == NONE ? syms.objectType : pt(), Warner.noWarnings)) {
+                        String key = types.isSameType(clazztype, inferred) ?
+                            "diamond.redundant.args" :
+                            "diamond.redundant.args.1";
+                        log.warning(tree.clazz.pos(), key, clazztype, inferred);
+                    }
+                }
             }
 
             if (cdef != null) {
@@ -1872,24 +1879,16 @@ public class Attr extends JCTree.Visitor {
 
                 // Reassign clazztype and recompute constructor.
                 clazztype = cdef.sym.type;
-                boolean useVarargs = tree.varargsElement != null;
-                Symbol sym = rs.resolveConstructor(
-                    tree.pos(), localEnv, clazztype, argtypes,
-                    typeargtypes, true, useVarargs);
-                Assert.check(sym.kind < AMBIGUOUS || tree.constructor.type.isErroneous());
+                Symbol sym = tree.constructor = rs.resolveConstructor(
+                    tree.pos(), localEnv, clazztype, argtypes, typeargtypes);
+                Assert.check(sym.kind < AMBIGUOUS);
                 tree.constructor = sym;
-                if (tree.constructor.kind > ERRONEOUS) {
-                    tree.constructorType =  syms.errType;
-                }
-                else {
-                    tree.constructorType = checkConstructor(clazztype,
-                            tree.constructor,
-                            localEnv,
-                            tree.args,
-                            argtypes,
-                            typeargtypes,
-                            useVarargs);
-                }
+                tree.constructorType = checkId(tree,
+                    clazztype,
+                    tree.constructor,
+                    localEnv,
+                    new ResultInfo(VAL, newMethodTemplate(syms.voidType, argtypes, typeargtypes)),
+                    localEnv.info.varArgs);
             }
 
             if (tree.constructor != null && tree.constructor.kind == MTH)
@@ -1899,7 +1898,7 @@ public class Attr extends JCTree.Visitor {
         chk.validate(tree.typeargs, localEnv);
     }
 
-    Type attribDiamond(Env<AttrContext> env,
+    Pair<Symbol, Type> attribDiamond(Env<AttrContext> env,
                         final JCNewClass tree,
                         final Type clazztype,
                         List<Type> argtypes,
@@ -1909,7 +1908,7 @@ public class Attr extends JCTree.Visitor {
             //if the type of the instance creation expression is erroneous,
             //or if it's an interface, or if something prevented us to form a valid
             //mapping, return the (possibly erroneous) type unchanged
-            return clazztype;
+            return new Pair<Symbol, Type>(syms.noSymbol, clazztype);
         }
 
         //dup attribution environment and augment the set of inference variables
@@ -1928,26 +1927,21 @@ public class Attr extends JCTree.Visitor {
                     argtypes,
                     typeargtypes);
 
-        Type owntype = types.createErrorType(clazztype);
-        if (constructor.kind == MTH) {
-            ResultInfo diamondResult = new ResultInfo(VAL, resultInfo.pt, new Check.NestedCheckContext(resultInfo.checkContext) {
-                @Override
-                public void report(DiagnosticPosition pos, JCDiagnostic details) {
-                    enclosingContext.report(tree.clazz.pos(),
-                            diags.fragment("cant.apply.diamond.1", diags.fragment("diamond", clazztype.tsym), details));
-                }
-            });
-            owntype = checkMethod(site,
-                    constructor,
-                    diamondResult,
-                    localEnv,
-                    tree.args,
-                    argtypes,
-                    typeargtypes,
-                    localEnv.info.varArgs).getReturnType();
-        }
+        Type constructorType = types.createErrorType(clazztype);
+        ResultInfo diamondResult = new ResultInfo(MTH, newMethodTemplate(resultInfo.pt, argtypes, typeargtypes), new Check.NestedCheckContext(resultInfo.checkContext) {
+            @Override
+            public void report(DiagnosticPosition _unused, JCDiagnostic details) {
+                enclosingContext.report(tree.clazz,
+                        diags.fragment("cant.apply.diamond.1", diags.fragment("diamond", clazztype.tsym), details));
+            }
+        });
+        constructorType = checkId(tree, site,
+                constructor,
+                localEnv,
+                diamondResult,
+                localEnv.info.varArgs);
 
-        return chk.checkClassType(tree.clazz.pos(), owntype, true);
+        return new Pair<Symbol, Type>(constructor.baseSymbol(), constructorType);
     }
 
     /** Make an attributed null check tree.
@@ -2563,10 +2557,9 @@ public class Attr extends JCTree.Visitor {
                 }
                 break;
             case MTH: {
-                JCMethodInvocation app = (JCMethodInvocation)env.tree;
                 owntype = checkMethod(site, sym,
                         new ResultInfo(VAL, resultInfo.pt.getReturnType(), resultInfo.checkContext),
-                        env, app.args, resultInfo.pt.getParameterTypes(),
+                        env, TreeInfo.args(env.tree), resultInfo.pt.getParameterTypes(),
                         resultInfo.pt.getTypeArguments(), env.info.varArgs);
                 break;
             }
@@ -2757,21 +2750,6 @@ public class Attr extends JCTree.Visitor {
         }
     }
 
-    /**
-     * Check that constructor arguments conform to its instantiation.
-     **/
-    public Type checkConstructor(Type site,
-                            Symbol sym,
-                            Env<AttrContext> env,
-                            final List<JCExpression> argtrees,
-                            List<Type> argtypes,
-                            List<Type> typeargtypes,
-                            boolean useVarargs) {
-        Type owntype = checkMethod(site, sym, new ResultInfo(VAL, syms.voidType), env, argtrees, argtypes, typeargtypes, useVarargs);
-        chk.checkType(env.tree.pos(), owntype.getReturnType(), syms.voidType);
-        return owntype;
-    }
-
     public void visitLiteral(JCLiteral tree) {
         result = check(
             tree, litType(tree.typetag).constType(tree.value), VAL, resultInfo);

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

@@ -970,9 +970,11 @@ public class MemberEnter extends JCTree.Visitor implements Completer {
                 List<Type> thrown = List.nil();
                 long ctorFlags = 0;
                 boolean based = false;
+                boolean addConstructor = true;
                 if (c.name.isEmpty()) {
                     JCNewClass nc = (JCNewClass)env.next.tree;
                     if (nc.constructor != null) {
+                        addConstructor = nc.constructor.kind != ERR;
                         Type superConstrType = types.memberType(c.type,
                                                                 nc.constructor);
                         argtypes = superConstrType.getParameterTypes();
@@ -985,10 +987,12 @@ public class MemberEnter extends JCTree.Visitor implements Completer {
                         thrown = superConstrType.getThrownTypes();
                     }
                 }
-                JCTree constrDef = DefaultConstructor(make.at(tree.pos), c,
-                                                    typarams, argtypes, thrown,
-                                                    ctorFlags, based);
-                tree.defs = tree.defs.prepend(constrDef);
+                if (addConstructor) {
+                    JCTree constrDef = DefaultConstructor(make.at(tree.pos), c,
+                                                        typarams, argtypes, thrown,
+                                                        ctorFlags, based);
+                    tree.defs = tree.defs.prepend(constrDef);
+                }
             }
 
             // If this is a class, enter symbols for this and super into

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

@@ -1972,10 +1972,12 @@ public class Resolve {
                 steps = steps.tail;
             }
             if (sym.kind >= AMBIGUOUS) {
-                final JCDiagnostic details = sym.kind == WRONG_MTH ?
-                                currentResolutionContext.candidates.head.details :
+                Symbol errSym =
+                        currentResolutionContext.resolutionCache.get(currentResolutionContext.firstErroneousResolutionPhase());
+                final JCDiagnostic details = errSym.kind == WRONG_MTH ?
+                                ((InapplicableSymbolError)errSym).errCandidate().details :
                                 null;
-                Symbol errSym = new ResolveError(WRONG_MTH, "diamond error") {
+                errSym = new InapplicableSymbolError(errSym.kind, "diamondError") {
                     @Override
                     JCDiagnostic getDiagnostic(DiagnosticType dkind, DiagnosticPosition pos,
                             Symbol location, Type site, Name name, List<Type> argtypes, List<Type> typeargtypes) {
@@ -2015,16 +2017,23 @@ public class Resolve {
         for (Scope.Entry e = site.tsym.members().lookup(names.init);
              e.scope != null;
              e = e.next()) {
+            final Symbol sym = e.sym;
             //- System.out.println(" e " + e.sym);
-            if (e.sym.kind == MTH &&
-                (e.sym.flags_field & SYNTHETIC) == 0) {
+            if (sym.kind == MTH &&
+                (sym.flags_field & SYNTHETIC) == 0) {
                     List<Type> oldParams = e.sym.type.tag == FORALL ?
-                            ((ForAll)e.sym.type).tvars :
+                            ((ForAll)sym.type).tvars :
                             List.<Type>nil();
                     Type constrType = new ForAll(site.tsym.type.getTypeArguments().appendList(oldParams),
-                            types.createMethodTypeWithReturn(e.sym.type.asMethodType(), site));
+                            types.createMethodTypeWithReturn(sym.type.asMethodType(), site));
+                    MethodSymbol newConstr = new MethodSymbol(sym.flags(), names.init, constrType, site.tsym) {
+                        @Override
+                        public Symbol baseSymbol() {
+                            return sym;
+                        }
+                    };
                     bestSoFar = selectBest(env, site, argtypes, typeargtypes,
-                            new MethodSymbol(e.sym.flags(), names.init, constrType, site.tsym),
+                            newConstr,
                             bestSoFar,
                             allowBoxing,
                             useVarargs,

test/tools/javac/6840059/T6840059.out

 T6840059.java:15:9: compiler.err.cant.apply.symbol.1: kindname.constructor, T6840059, java.lang.Integer, java.lang.String, kindname.class, T6840059, (compiler.misc.no.conforming.assignment.exists: (compiler.misc.inconvertible.types: java.lang.String, java.lang.Integer))
-T6840059.java:15:25: compiler.err.cant.apply.symbol.1: kindname.constructor, T6840059, java.lang.Integer, compiler.misc.no.args, kindname.class, T6840059, (compiler.misc.arg.length.mismatch)
-2 errors
+1 error

test/tools/javac/6857948/T6857948.out

 T6857948.java:16:32: compiler.err.cant.resolve.location.args: kindname.method, nosuchfunction, , , (compiler.misc.location: kindname.class, Test, null)
-T6857948.java:16:50: compiler.err.cant.apply.symbol.1: kindname.constructor, Foo, java.lang.String, compiler.misc.no.args, kindname.class, Foo, (compiler.misc.arg.length.mismatch)
-2 errors
+1 error

test/tools/javac/diags/examples/KindnameConstructor.java

@@ -23,12 +23,10 @@
 
 // key: compiler.misc.kindname.constructor
 // key: compiler.misc.kindname.class
-// key: compiler.misc.no.args
 // key: compiler.err.cant.apply.symbol.1
-// key: compiler.misc.arg.length.mismatch
 // key: compiler.misc.no.conforming.assignment.exists
 // key: compiler.misc.inconvertible.types
-// key: compiler.misc.count.error.plural
+// key: compiler.misc.count.error
 // key: compiler.err.error
 // run: backdoor
 

test/tools/javac/generics/diamond/7002837/T7002837.java

@@ -4,7 +4,7 @@
  *
  * @summary  Diamond: javac generates diamond inference errors when in 'finder' mode
  * @author mcimadamore
- * @compile -Werror -XDfindDiamond T7002837.java
+ * @compile/fail/ref=T7002837.out -Werror -XDrawDiagnostics -XDfindDiamond T7002837.java
  *
  */
 

test/tools/javac/generics/diamond/7002837/T7002837.out

+T7002837.java:13:19: compiler.warn.diamond.redundant.args.1: T7002837<java.lang.Integer>, T7002837<java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>
+- compiler.err.warnings.and.werror
+1 error
+1 warning

test/tools/javac/generics/diamond/7188968/T7188968.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 7188968
+ *
+ * @summary  Diamond: javac generates diamond inference errors when in 'finder' mode
+ * @author mcimadamore
+ * @compile/fail/ref=T7188968.out -Xlint:unchecked -XDrawDiagnostics T7188968.java
+ *
+ */
+import java.util.List;
+
+class T7188968 {
+
+    static class Foo<X> {
+        Foo(List<X> ls, Object o) { }
+        static <Z> Foo<Z> makeFoo(List<Z> lz, Object o) { return null; }
+    }
+
+    void test(List l) {
+        new Foo(l, unknown);
+        new Foo(l, unknown) { };
+        new Foo<>(l, unknown);
+        Foo.makeFoo(l, unknown);
+    }
+}

test/tools/javac/generics/diamond/7188968/T7188968.out

+T7188968.java:20:20: compiler.err.cant.resolve.location: kindname.variable, unknown, , , (compiler.misc.location: kindname.class, T7188968, null)
+T7188968.java:21:20: compiler.err.cant.resolve.location: kindname.variable, unknown, , , (compiler.misc.location: kindname.class, T7188968, null)
+T7188968.java:21:29: compiler.warn.unchecked.call.mbr.of.raw.type: T7188968.Foo(java.util.List<X>,java.lang.Object), T7188968.Foo
+T7188968.java:22:22: compiler.err.cant.resolve.location: kindname.variable, unknown, , , (compiler.misc.location: kindname.class, T7188968, null)
+T7188968.java:23:24: compiler.err.cant.resolve.location: kindname.variable, unknown, , , (compiler.misc.location: kindname.class, T7188968, null)
+4 errors
+1 warning

test/tools/javac/positions/T6264029.out

-T6264029.java:15:19: compiler.warn.unchecked.call.mbr.of.raw.type: T6264029A(K), T6264029A
 T6264029.java:15:41: compiler.warn.unchecked.call.mbr.of.raw.type: T6264029A(K), T6264029A
-2 warnings
+1 warning