From 7f4e903e66a936be0425d58fc791e3836a579e89 Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Wed, 2 Sep 2020 15:04:39 +0200 Subject: [PATCH 1/3] 8240658: Code completion not working for lambdas in method invocations that require type inference --- .../com/sun/tools/javac/code/Flags.java | 7 +- .../com/sun/tools/javac/comp/Attr.java | 20 +- .../com/sun/tools/javac/comp/AttrRecover.java | 276 ++++++++++++++++++ .../sun/tools/javac/comp/DeferredAttr.java | 16 +- .../com/sun/tools/javac/comp/Resolve.java | 19 +- .../jdk/jshell/CompletionSuggestionTest.java | 23 +- .../tools/javac/api/TestGetScopeResult.java | 50 +++- .../javac/api/TestGetTypeMirrorReference.java | 217 ++++++++++++++ .../api/TestGetTypeMirrorReferenceData.java | 63 ++++ 9 files changed, 655 insertions(+), 36 deletions(-) create mode 100644 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java create mode 100644 test/langtools/tools/javac/api/TestGetTypeMirrorReference.java create mode 100644 test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java index f43484bb3b147..aebbc78da3685 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java @@ -220,7 +220,11 @@ public static EnumSet asFlagSet(long flags) { */ public static final long UNION = 1L<<39; - // Flag bit (1L << 40) is available. + /** + * Flags an erroneous TypeSymbol as viable for recovery. + * TypeSymbols only. + */ + public static final long RECOVERABLE = 1L<<40; /** * Flag that marks an 'effectively final' local variable. @@ -508,6 +512,7 @@ public enum Flag { MATCH_BINDING(Flags.MATCH_BINDING), MATCH_BINDING_TO_OUTER(Flags.MATCH_BINDING_TO_OUTER), RECORD(Flags.RECORD), + RECOVERABLE(Flags.RECOVERABLE), SEALED(Flags.SEALED), NON_SEALED(Flags.NON_SEALED) { @Override diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index b6f0dd415513f..cd0505e3dbb7d 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -120,6 +120,7 @@ public class Attr extends JCTree.Visitor { final Annotate annotate; final ArgumentAttr argumentAttr; final MatchBindingsComputer matchBindingsComputer; + final AttrRecover attrRecover; public static Attr instance(Context context) { Attr instance = context.get(attrKey); @@ -157,6 +158,7 @@ protected Attr(Context context) { dependencies = Dependencies.instance(context); argumentAttr = ArgumentAttr.instance(context); matchBindingsComputer = MatchBindingsComputer.instance(context); + attrRecover = AttrRecover.instance(context); Options options = Options.instance(context); @@ -419,8 +421,9 @@ private Env attribToTree(JCTree root, Env env, JCTree JavaFileObject prev = log.useSource(env.toplevel.sourcefile); try { deferredAttr.attribSpeculative(root, env, resultInfo, - null, DeferredAttr.AttributionMode.ANALYZER, + null, DeferredAttr.AttributionMode.ATTRIB_TO_TREE, argumentAttr.withLocalCacheContext()); + attrRecover.doRecovery(); } catch (BreakAttr b) { return b.env; } catch (AssertionError ae) { @@ -738,6 +741,7 @@ public Type attribStat(JCTree tree, Env env) { Env analyzeEnv = analyzer.copyEnvIfNeeded(tree, env); Type result = attribTree(tree, env, statInfo); analyzer.analyzeIfNeeded(tree, analyzeEnv); + attrRecover.doRecovery(); return result; } @@ -2092,6 +2096,7 @@ public void visitIf(JCIf tree) { } void preFlow(JCTree tree) { + attrRecover.doRecovery(); new PostAttrAnalyzer() { @Override public void scan(JCTree tree) { @@ -3114,6 +3119,7 @@ TargetInfo getTargetInfo(JCPolyExpression that, ResultInfo resultInfo, List env, ResultInfo resultInfo) { if (resultInfo.pkind.contains(KindSelector.POLY)) { - Type pt = resultInfo.pt.map(deferredAttr.new RecoveryDeferredTypeMap(AttrMode.SPECULATIVE, sym, env.info.pendingResolutionPhase)); - Type owntype = checkIdInternal(tree, site, sym, pt, env, resultInfo); - resultInfo.pt.map(deferredAttr.new RecoveryDeferredTypeMap(AttrMode.CHECK, sym, env.info.pendingResolutionPhase)); - return owntype; + return attrRecover.recoverMethodInvocation(tree, site, sym, env, resultInfo); } else { return checkIdInternal(tree, site, sym, resultInfo.pt, env, resultInfo); } @@ -4916,9 +4919,12 @@ public void visitAnnotatedType(JCAnnotatedType tree) { } public void visitErroneous(JCErroneous tree) { - if (tree.errs != null) + if (tree.errs != null) { + Env errEnv = env.dup(env.tree); + errEnv.info.returnResult = unknownExprInfo; for (JCTree err : tree.errs) - attribTree(err, env, new ResultInfo(KindSelector.ERR, pt())); + attribTree(err, errEnv, new ResultInfo(KindSelector.ERR, pt())); + } result = tree.type = syms.errType; } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java new file mode 100644 index 0000000000000..ea10f397c79c7 --- /dev/null +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java @@ -0,0 +1,276 @@ +/* + * Copyright (c) 2020, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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. + */ +package com.sun.tools.javac.comp; + +import com.sun.tools.javac.code.Flags; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Symbol.TypeSymbol; +import com.sun.tools.javac.code.Symtab; +import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.code.Type.ErrorType; +import com.sun.tools.javac.code.TypeTag; +import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.comp.Attr.ResultInfo; +import com.sun.tools.javac.comp.DeferredAttr.AttrMode; +import com.sun.tools.javac.tree.JCTree; +import com.sun.tools.javac.tree.JCTree.JCBlock; +import com.sun.tools.javac.tree.JCTree.JCClassDecl; +import com.sun.tools.javac.tree.JCTree.JCErroneous; +import com.sun.tools.javac.tree.JCTree.JCExpression; +import com.sun.tools.javac.tree.JCTree.JCLambda; +import com.sun.tools.javac.tree.JCTree.JCMethodInvocation; +import com.sun.tools.javac.tree.JCTree.JCReturn; +import com.sun.tools.javac.tree.JCTree.JCVariableDecl; +import com.sun.tools.javac.tree.JCTree.Tag; +import com.sun.tools.javac.tree.TreeInfo; +import com.sun.tools.javac.tree.TreeMaker; +import com.sun.tools.javac.tree.TreeTranslator; +import com.sun.tools.javac.util.Context; +import com.sun.tools.javac.util.JCDiagnostic; +import com.sun.tools.javac.util.List; +import com.sun.tools.javac.util.ListBuffer; +import com.sun.tools.javac.util.Names; + +/** This is an error recovery addon for Attr. Currently, it recovers + * method invocations with lambdas, that require type inference. + * + *

This is NOT part of any supported API. + * If you write code that depends on this, you do so at your own risk. + * This code and its internal interfaces are subject to change or + * deletion without notice. + */ +public class AttrRecover { + protected static final Context.Key attrRepairKey = new Context.Key<>(); + + final Attr attr; + final DeferredAttr deferredAttr; + final Names names; + final TreeMaker make; + final Symtab syms; + final Types types; + + public static AttrRecover instance(Context context) { + AttrRecover instance = context.get(attrRepairKey); + if (instance == null) + instance = new AttrRecover(context); + return instance; + } + + protected AttrRecover(Context context) { + context.put(attrRepairKey, this); + + attr = Attr.instance(context); + deferredAttr = DeferredAttr.instance(context); + names = Names.instance(context); + make = TreeMaker.instance(context); + syms = Symtab.instance(context); + types = Types.instance(context); + } + + private final ListBuffer recoveryTodo = new ListBuffer<>(); + + public void doRecovery() { + while (recoveryTodo.nonEmpty()) { + RecoverTodo todo = recoveryTodo.remove(); + ListBuffer rollback = new ListBuffer<>(); + boolean repaired = false; + RECOVER: if (todo.env.tree.hasTag(Tag.APPLY)) { + JCMethodInvocation mit = (JCMethodInvocation) todo.env.tree; + if ((todo.candSym.flags() & Flags.VARARGS) == 0 && + mit.args.length() > todo.candSym.type.getParameterTypes().length()) { + break RECOVER; //too many actual parameters, skip + } + List args = mit.args; + List formals = todo.candSym.type.getParameterTypes(); + while (args.nonEmpty() && formals.nonEmpty()) { + JCExpression arg = args.head; + if (arg.hasTag(JCTree.Tag.LAMBDA)) { + final JCTree.JCLambda lambda = (JCLambda) arg; + if (lambda.paramKind == JCLambda.ParameterKind.IMPLICIT) { + for (JCVariableDecl var : lambda.params) { + var.vartype = null; //reset type + } + } + if (types.isFunctionalInterface(formals.head)) { + Type functionalType = types.findDescriptorType(formals.head); + boolean voidCompatible = functionalType.getReturnType().hasTag(TypeTag.VOID); + lambda.body = new TreeTranslator() { + @Override + public void visitReturn(JCReturn tree) { + result = tree; + if (voidCompatible) { + if (tree.expr != null) { + JCErroneous err = make.Erroneous(List.of(tree)); + result = err; + rollback.append(() -> { + lambda.body = new TreeTranslator() { + @SuppressWarnings("unchecked") + public T translate(T t) { + if (t == err) return (T) tree; + else return super.translate(t); + } + }.translate(lambda.body); + }); + } + } else { + if (tree.expr == null) { + tree.expr = make.Erroneous().setType(syms.errType); + rollback.append(() -> { + tree.expr = null; + }); + } + } + } + @Override + public void visitLambda(JCLambda tree) { + //do not touch nested lambdas + } + @Override + public void visitClassDef(JCClassDecl tree) { + //do not touch nested classes + } + }.translate(lambda.body); + if (!voidCompatible) { + JCReturn ret = make.Return(make.Erroneous().setType(syms.errType)); + ((JCBlock) lambda.body).stats = ((JCBlock) lambda.body).stats.append(ret); + rollback.append(() -> { + ((JCBlock) lambda.body).stats = List.filter(((JCBlock) lambda.body).stats, ret); + }); + } + } + repaired = true; + } + args = args.tail; + formals = formals.tail; + } + List prevArgs = mit.args; + while (formals.nonEmpty()) { + mit.args = mit.args.append(make.Erroneous().setType(syms.errType)); + formals = formals.tail; + repaired = true; + } + rollback.append(() -> { + mit.args = prevArgs; + }); + } + + Type owntype; + if (repaired) { + List args = TreeInfo.args(todo.env.tree); + List pats = todo.resultInfo.pt.getParameterTypes(); + while (pats.length() < args.length()) { + pats = pats.append(syms.errType); + } + owntype = attr.checkMethod(todo.site, todo.candSym, + attr.new ResultInfo(todo.resultInfo.pkind, todo.resultInfo.pt.getReturnType(), todo.resultInfo.checkContext, todo.resultInfo.checkMode), + todo.env, args, pats, + todo.resultInfo.pt.getTypeArguments()); + rollback.stream().forEach(Runnable::run); + } else { + owntype = basicMethodInvocationRecovery(todo.tree, todo.site, todo.errSym, todo.env, todo.resultInfo); + } + todo.tree.type = owntype; + } + } + + Type recoverMethodInvocation(JCTree tree, + Type site, + Symbol sym, + Env env, + ResultInfo resultInfo) { + if ((sym.flags_field & Flags.RECOVERABLE) != 0 && env.info.attributionMode.recover()) { + recoveryTodo.append(new RecoverTodo(tree, site, sym, ((RecoveryErrorType) sym.type).candidateSymbol, attr.copyEnv(env), resultInfo)); + return syms.errType; + } else { + return basicMethodInvocationRecovery(tree, site, sym, env, resultInfo); + } + } + + private Type basicMethodInvocationRecovery(JCTree tree, + Type site, + Symbol sym, + Env env, + ResultInfo resultInfo) { + Type pt = resultInfo.pt.map(deferredAttr.new RecoveryDeferredTypeMap(AttrMode.SPECULATIVE, sym, env.info.pendingResolutionPhase)); + Type owntype = attr.checkIdInternal(tree, site, sym, pt, env, resultInfo); + resultInfo.pt.map(deferredAttr.new RecoveryDeferredTypeMap(AttrMode.CHECK, sym, env.info.pendingResolutionPhase)); + return owntype; + } + + void wrongMethodSymbolCandidate(TypeSymbol errSymbol, Symbol candSym, JCDiagnostic diag) { + List diags = List.of(diag); + boolean recoverable = false; + while (!recoverable && diags.nonEmpty()) { + JCDiagnostic d = diags.head; + diags = diags.tail; + switch (d.getCode()) { + case "compiler.misc.missing.ret.val": + case "compiler.misc.unexpected.ret.val": + case "compiler.misc.infer.arg.length.mismatch": + case "compiler.misc.arg.length.mismatch": + errSymbol.type = new RecoveryErrorType((Type.ErrorType) errSymbol.type, candSym); + errSymbol.flags_field |= Flags.RECOVERABLE; + return ; + default: + break; + } + for (Object a : d.getArgs()) { + if (a instanceof JCDiagnostic) { + diags = diags.prepend((JCDiagnostic) a); + } + } + } + } + + private static class RecoveryErrorType extends ErrorType { + public final Symbol candidateSymbol; + + public RecoveryErrorType(ErrorType original, Symbol candidateSymbol) { + super(original.getOriginalType(), original.tsym); + this.candidateSymbol = candidateSymbol; + } + + } + + private static class RecoverTodo { + public final JCTree tree; + public final Type site; + public final Symbol errSym; + public final Symbol candSym; + public final Env env; + public final ResultInfo resultInfo; + + public RecoverTodo(JCTree tree, Type site, Symbol errSym, Symbol candSym, + Env env, Attr.ResultInfo resultInfo) { + this.tree = tree; + this.site = site; + this.errSym = errSym; + this.candSym = candSym; + this.env = env; + this.resultInfo = resultInfo; + } + + } +} diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java index 66ed1834179a9..f64274ca4de38 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java @@ -1329,20 +1329,28 @@ public void visitReference(JCMemberReference tree) { */ enum AttributionMode { /**Normal, non-speculative, attribution.*/ - FULL(false), + FULL(false, true), /**Speculative attribution on behalf of an Analyzer.*/ - ANALYZER(true), + ATTRIB_TO_TREE(true, true), + /**Speculative attribution on behalf of an Analyzer.*/ + ANALYZER(true, false), /**Speculative attribution.*/ - SPECULATIVE(true); + SPECULATIVE(true, false); - AttributionMode(boolean isSpeculative) { + AttributionMode(boolean isSpeculative, boolean recover) { this.isSpeculative = isSpeculative; + this.recover = recover; } boolean isSpeculative() { return isSpeculative; } + boolean recover() { + return recover; + } + final boolean isSpeculative; + final boolean recover; } } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java index e855c24036043..6fd598340b7a1 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java @@ -96,6 +96,7 @@ public class Resolve { Log log; Symtab syms; Attr attr; + AttrRecover attrRecover; DeferredAttr deferredAttr; Check chk; Infer infer; @@ -126,6 +127,7 @@ protected Resolve(Context context) { names = Names.instance(context); log = Log.instance(context); attr = Attr.instance(context); + attrRecover = AttrRecover.instance(context); deferredAttr = DeferredAttr.instance(context); chk = Check.instance(context); infer = Infer.instance(context); @@ -4043,12 +4045,12 @@ JCDiagnostic getDiagnostic(JCDiagnostic.DiagnosticType dkind, @Override public Symbol access(Name name, TypeSymbol location) { - Symbol sym = bestCandidate(); - return types.createErrorType(name, location, sym != null ? sym.type : syms.errSymbol.type).tsym; - } - - protected Symbol bestCandidate() { - return errCandidate().fst; + Pair cand = errCandidate(); + TypeSymbol errSymbol = types.createErrorType(name, location, cand != null ? cand.fst.type : syms.errSymbol.type).tsym; + if (cand != null) { + attrRecover.wrongMethodSymbolCandidate(errSymbol, cand.fst, cand.snd); + } + return errSymbol; } protected Pair errCandidate() { @@ -4181,11 +4183,12 @@ private List candidateDetails(Map candidates } @Override - protected Symbol bestCandidate() { + protected Pair errCandidate() { Map candidatesMap = mapCandidates(); Map filteredCandidates = filterCandidates(candidatesMap); if (filteredCandidates.size() == 1) { - return filteredCandidates.keySet().iterator().next(); + return Pair.of(filteredCandidates.keySet().iterator().next(), + filteredCandidates.values().iterator().next()); } return null; } diff --git a/test/langtools/jdk/jshell/CompletionSuggestionTest.java b/test/langtools/jdk/jshell/CompletionSuggestionTest.java index b78b25e33dfc8..1a024e298878f 100644 --- a/test/langtools/jdk/jshell/CompletionSuggestionTest.java +++ b/test/langtools/jdk/jshell/CompletionSuggestionTest.java @@ -23,7 +23,7 @@ /* * @test - * @bug 8131025 8141092 8153761 8145263 8131019 8175886 8176184 8176241 8176110 8177466 8197439 8221759 8234896 + * @bug 8131025 8141092 8153761 8145263 8131019 8175886 8176184 8176241 8176110 8177466 8197439 8221759 8234896 8240658 * @summary Test Completion and Documentation * @library /tools/lib * @modules jdk.compiler/com.sun.tools.javac.api @@ -698,6 +698,27 @@ public void testMemberReferences() { assertCompletion("FI2 fi = C::|", true, "statConvert1", "statConvert3"); } + public void testBrokenLambdaCompletion() { + assertEval("interface Consumer { public void consume(T t); }"); + assertEval("interface Function { public R convert(T t); }"); + assertEval(" void m1(T t, Consumer f) { }"); + assertCompletion("m1(\"\", x -> {x.tri|", "trim()"); + assertEval(" void m2(T t, Function f) { }"); + assertCompletion("m2(\"\", x -> {x.tri|", "trim()"); + assertEval(" void m3(T t, Consumer f, int i) { }"); + assertCompletion("m3(\"\", x -> {x.tri|", "trim()"); + assertEval(" void m4(T t, Function f, int i) { }"); + assertCompletion("m4(\"\", x -> {x.tri|", "trim()"); + assertEval(" T m5(Consumer f) { return null; }"); + assertCompletion("String s = m5(x -> {x.tri|", "trim()"); + assertEval(" T m6(Function f) { return null; }"); + assertCompletion("String s = m6(x -> {x.tri|", "trim()"); + assertEval(" T m7(Consumer f, int i) { return null; }"); + assertCompletion("String s = m7(x -> {x.tri|", "trim()"); + assertEval(" T m8(Function f, int i) { return null; }"); + assertCompletion("String s = m8(x -> {x.tri|", "trim()"); + } + @BeforeMethod public void setUp() { super.setUp(); diff --git a/test/langtools/tools/javac/api/TestGetScopeResult.java b/test/langtools/tools/javac/api/TestGetScopeResult.java index 8f32df197d912..b2a2e500e0320 100644 --- a/test/langtools/tools/javac/api/TestGetScopeResult.java +++ b/test/langtools/tools/javac/api/TestGetScopeResult.java @@ -23,12 +23,13 @@ /* * @test - * @bug 8205418 8207229 8207230 8230847 8245786 8247334 8248641 + * @bug 8205418 8207229 8207230 8230847 8245786 8247334 8248641 8240658 * @summary Test the outcomes from Trees.getScope * @modules jdk.compiler/com.sun.tools.javac.api * jdk.compiler/com.sun.tools.javac.comp * jdk.compiler/com.sun.tools.javac.tree * jdk.compiler/com.sun.tools.javac.util + * @compile TestGetScopeResult.java */ import java.io.IOException; @@ -62,6 +63,7 @@ import com.sun.source.util.TreePathScanner; import com.sun.source.util.Trees; import com.sun.tools.javac.api.JavacScope; +import com.sun.tools.javac.api.JavacTaskImpl; import com.sun.tools.javac.api.JavacTool; import com.sun.tools.javac.comp.Analyzer; @@ -192,6 +194,19 @@ static interface BiFunction { } }""", invocationInMethodInvocation); + + String[] infer = { + "c:java.lang.String", + "super:java.lang.Object", + "this:Test" + }; + + doTest("class Test { void test() { cand(\"\", c -> { }); } void cand(T t, I i) { } interface I { public String test(T s); } }", + infer); + doTest("class Test { void test() { cand(\"\", c -> { }); } void cand(T t, I i, int j) { } interface I { public void test(T s); } }", + infer); + doTest("class Test { void test() { cand(\"\", c -> { }); } void cand(T t, I i, int j) { } interface I { public String test(T s); } }", + infer); } public void doTest(String code, String... expected) throws IOException { @@ -208,24 +223,29 @@ public String getCharContent(boolean ignoreEncodingErrors) { } JavacTask t = (JavacTask) c.getTask(null, fm, null, null, null, List.of(new MyFileObject())); CompilationUnitTree cut = t.parse().iterator().next(); - t.analyze(); - List actual = new ArrayList<>(); + ((JavacTaskImpl)t).enter(); - new TreePathScanner() { - @Override - public Void visitLambdaExpression(LambdaExpressionTree node, Void p) { - Scope scope = Trees.instance(t).getScope(new TreePath(getCurrentPath(), node.getBody())); - actual.addAll(dumpScope(scope)); - return super.visitLambdaExpression(node, p); - } - }.scan(cut, null); + for (int r = 0; r < 2; r++) { + List actual = new ArrayList<>(); - List expectedList = List.of(expected); + new TreePathScanner() { + @Override + public Void visitLambdaExpression(LambdaExpressionTree node, Void p) { + Scope scope = Trees.instance(t).getScope(new TreePath(getCurrentPath(), node.getBody())); + actual.addAll(dumpScope(scope)); + return super.visitLambdaExpression(node, p); + } + }.scan(cut, null); + + List expectedList = List.of(expected); - if (!expectedList.equals(actual)) { - throw new IllegalStateException("Unexpected scope content: " + actual + "\n" + - "expected: " + expectedList); + if (!expectedList.equals(actual)) { + throw new IllegalStateException("Unexpected scope content: " + actual + "\n" + + "expected: " + expectedList); + } + + t.analyze(); } } } diff --git a/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java b/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java new file mode 100644 index 0000000000000..e641327a0847b --- /dev/null +++ b/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java @@ -0,0 +1,217 @@ +/* + * Copyright (c) 2020, 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. + */ + +/* + * @test + * @bug 8240658 + * @summary Verify that broken method invocations with lambdas get type inference done + * @modules jdk.compiler + * @compile --enable-preview -source ${jdk.version} TestGetTypeMirrorReference.java + * @run main/othervm --enable-preview TestGetTypeMirrorReference + */ + +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.Tree; +import com.sun.source.util.*; +import java.io.File; +import java.io.IOException; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import javax.lang.model.type.TypeMirror; +import javax.tools.DiagnosticCollector; +import javax.tools.JavaFileObject; +import javax.tools.SimpleJavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.ToolProvider; + +public class TestGetTypeMirrorReference { + + private static final String JDK_VERSION = + Integer.toString(Runtime.getRuntime().version().feature()); + + public static void main(String... args) throws IOException { + analyze("TestGetTypeMirrorReferenceData.java", + """ + package test; + + public class TestGetTypeMirrorReferenceData { + + public TestGetTypeMirrorReferenceData() { + super(); + } + + private static void test() { + Test.of(1).convert((c1)->{ + Object o = c1; + }); + Test.of(1).consume((c2)->{ + Object o = c2; + return null; + }); + Test.of(1).consumeWithParam((c3)->{ + Object o = c3; + }); + convert(0, (c4)->{ + Object o = c4; + }); + consume(0, (c5)->{ + Object o = c5; + }); + convertVarArgs(0, (c6)->{ + Object o = c6; + }, 1, 2, 3, 4); + consumeVarArgs(0, (c7)->{ + Object o = c7; + }, 1, 2, 3, 4); + } + + public R convert(T t, Function f, int i) { + return null; + } + + public void consume(T t, Consumer c, int i) { + } + + public R convertVarArgs(T t, Function c, int... i) { + return null; + } + + public void consumeVarArgs(T t, Consumer c, int... i) { + } + + public static class Test { + + public Test() { + super(); + } + + public static Test of(T t) { + return new Test<>(); + } + + public Test convert(Function c) { + return null; + } + + public void consume(Consumer c) { + } + + public void consumeWithParam(Consumer c, int i) { + } + } + + public interface Function { + + public R map(T t); + } + + public interface Consumer { + + public void run(T t); + } + }"""); + } + + private static void analyze(String fileName, String expectedAST) throws IOException { + try (StandardJavaFileManager fm = ToolProvider.getSystemJavaCompiler().getStandardFileManager(null, null, null)) { + List files = new ArrayList<>(); + File source = new File(System.getProperty("test.src", "."), fileName.replace('/', File.separatorChar)).getAbsoluteFile(); + for (JavaFileObject f : fm.getJavaFileObjects(source)) { + files.add(f); + } + DiagnosticCollector diagnostics = new DiagnosticCollector<>(); + List options = List.of("-source", JDK_VERSION, + "-XDshould-stop.at=FLOW"); + JavacTask ct = (JavacTask) ToolProvider.getSystemJavaCompiler().getTask(null, null, diagnostics, options, null, files); + Trees trees = Trees.instance(ct); + CompilationUnitTree cut = ct.parse().iterator().next(); + + ct.analyze(); + + String actualAST = Arrays.stream(cut.toString().split("\n")) + .map(l -> l.stripTrailing()) + .collect(Collectors.joining("\n")); + + if (!expectedAST.equals(actualAST)) { + throw new AssertionError("Unexpected AST shape!\n" + actualAST); + } + + Pattern p = Pattern.compile("/\\*getTypeMirror:(.*?)\\*/"); + Matcher m = p.matcher(cut.getSourceFile().getCharContent(false)); + + while (m.find()) { + TreePath tp = pathFor(trees, cut, m.start() - 1); + String expected = m.group(1); + if (expected.startsWith("getParentPath:")) { + tp = tp.getParentPath(); + expected = expected.substring("getParentPath:".length()); + } + TypeMirror found = trees.getTypeMirror(tp); + String actual = found != null ? found.getKind() + ":" + typeToString(found) : ""; + + if (!expected.equals(actual)) { + throw new IllegalStateException("expected=" + expected + "; actual=" + actual + "; tree: " + tp.getLeaf()); + } + } + } + } + + private static TreePath pathFor(final Trees trees, final CompilationUnitTree cut, final int pos) { + final TreePath[] result = new TreePath[1]; + + new TreePathScanner() { + @Override public Void scan(Tree node, Void p) { + if ( node != null + && trees.getSourcePositions().getStartPosition(cut, node) <= pos + && pos <= trees.getSourcePositions().getEndPosition(cut, node)) { + result[0] = new TreePath(getCurrentPath(), node); + return super.scan(node, p); + } + return null; + } + }.scan(cut, null); + + return result[0]; + } + + private static String typeToString(TypeMirror type) { + return type.toString(); + } + + static class TestFileObject extends SimpleJavaFileObject { + private final String text; + public TestFileObject(String text) { + super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE); + this.text = text; + } + @Override public CharSequence getCharContent(boolean ignoreEncodingErrors) { + return text; + } + } + +} diff --git a/test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java b/test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java new file mode 100644 index 0000000000000..6b4e28297fd04 --- /dev/null +++ b/test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2020, 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. + */ + +package test; + +public class TestGetTypeMirrorReferenceData { + + private static void test() { + Test.of(1).convert(c1 -> {Object o = c1/*getTypeMirror:DECLARED:java.lang.Integer*/;}); + Test.of(1).consume(c2 -> {Object o = c2/*getTypeMirror:DECLARED:java.lang.Integer*/; return null;}); + Test.of(1).consumeWithParam(c3 -> {Object o = c3/*getTypeMirror:DECLARED:java.lang.Integer*/;}); + convert(0, c4 -> {Object o = c4/*getTypeMirror:DECLARED:java.lang.Integer*/;}); + consume(0, c5 -> {Object o = c5/*getTypeMirror:DECLARED:java.lang.Integer*/;}); + convertVarArgs(0, c6 -> {Object o = c6/*getTypeMirror:DECLARED:java.lang.Integer*/;}, 1, 2, 3, 4); + consumeVarArgs(0, c7 -> {Object o = c7/*getTypeMirror:DECLARED:java.lang.Integer*/;}, 1, 2, 3, 4); + } + public R convert(T t, Function f, int i) { + return null; + } + public void consume(T t, Consumer c, int i) { + } + public R convertVarArgs(T t, Function c, int... i) { + return null; + } + public void consumeVarArgs(T t, Consumer c, int... i) { + } + public static class Test { + public static Test of(T t) { + return new Test<>(); + } + public Test convert(Function c) { + return null; + } + public void consume(Consumer c) {} + public void consumeWithParam(Consumer c, int i) {} + } + public interface Function { + public R map(T t); + } + public interface Consumer { + public void run(T t); + } +} From 32845ddb9a86e16368838a1cd45795226b4b51bb Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Thu, 10 Sep 2020 15:51:23 +0200 Subject: [PATCH 2/3] Improving behavior for vararg method invocations. --- .../com/sun/tools/javac/comp/AttrRecover.java | 14 ++++++++++---- .../javac/api/TestGetTypeMirrorReference.java | 17 +++++++++++++++++ .../api/TestGetTypeMirrorReferenceData.java | 7 +++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java index ea10f397c79c7..8fd09501717ca 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java @@ -29,6 +29,7 @@ import com.sun.tools.javac.code.Symbol.TypeSymbol; import com.sun.tools.javac.code.Symtab; import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.code.Type.ArrayType; import com.sun.tools.javac.code.Type.ErrorType; import com.sun.tools.javac.code.TypeTag; import com.sun.tools.javac.code.Types; @@ -98,7 +99,8 @@ public void doRecovery() { boolean repaired = false; RECOVER: if (todo.env.tree.hasTag(Tag.APPLY)) { JCMethodInvocation mit = (JCMethodInvocation) todo.env.tree; - if ((todo.candSym.flags() & Flags.VARARGS) == 0 && + boolean vararg = (todo.candSym.flags() & Flags.VARARGS) != 0; + if (!vararg && mit.args.length() > todo.candSym.type.getParameterTypes().length()) { break RECOVER; //too many actual parameters, skip } @@ -106,6 +108,8 @@ public void doRecovery() { List formals = todo.candSym.type.getParameterTypes(); while (args.nonEmpty() && formals.nonEmpty()) { JCExpression arg = args.head; + Type formal = formals.tail.nonEmpty() || !vararg + ? formals.head : ((ArrayType) formals.head).elemtype; if (arg.hasTag(JCTree.Tag.LAMBDA)) { final JCTree.JCLambda lambda = (JCLambda) arg; if (lambda.paramKind == JCLambda.ParameterKind.IMPLICIT) { @@ -113,8 +117,8 @@ public void doRecovery() { var.vartype = null; //reset type } } - if (types.isFunctionalInterface(formals.head)) { - Type functionalType = types.findDescriptorType(formals.head); + if (types.isFunctionalInterface(formal)) { + Type functionalType = types.findDescriptorType(formal); boolean voidCompatible = functionalType.getReturnType().hasTag(TypeTag.VOID); lambda.body = new TreeTranslator() { @Override @@ -163,7 +167,9 @@ public void visitClassDef(JCClassDecl tree) { repaired = true; } args = args.tail; - formals = formals.tail; + if (formals.tail.nonEmpty() || !vararg) { + formals = formals.tail; + } } List prevArgs = mit.args; while (formals.nonEmpty()) { diff --git a/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java b/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java index e641327a0847b..5347169658c69 100644 --- a/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java +++ b/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java @@ -88,6 +88,16 @@ private static void test() { consumeVarArgs(0, (c7)->{ Object o = c7; }, 1, 2, 3, 4); + convertVarArgs2(0, (c8)->{ + Object o = c8; + }, (c8)->{ + Object o = c8; + }); + consumeVarArgs2(0, (c9)->{ + Object o = c9; + }, (c9)->{ + Object o = c9; + }); } public R convert(T t, Function f, int i) { @@ -104,6 +114,13 @@ public R convertVarArgs(T t, Function c, int... i) { public void consumeVarArgs(T t, Consumer c, int... i) { } + public R convertVarArgs2(T t, Function... c) { + return null; + } + + public void consumeVarArgs2(T t, Consumer... c) { + } + public static class Test { public Test() { diff --git a/test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java b/test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java index 6b4e28297fd04..28e62147bdfde 100644 --- a/test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java +++ b/test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java @@ -33,6 +33,8 @@ private static void test() { consume(0, c5 -> {Object o = c5/*getTypeMirror:DECLARED:java.lang.Integer*/;}); convertVarArgs(0, c6 -> {Object o = c6/*getTypeMirror:DECLARED:java.lang.Integer*/;}, 1, 2, 3, 4); consumeVarArgs(0, c7 -> {Object o = c7/*getTypeMirror:DECLARED:java.lang.Integer*/;}, 1, 2, 3, 4); + convertVarArgs2(0, c8 -> {Object o = c8/*getTypeMirror:DECLARED:java.lang.Integer*/;}, c8 -> {Object o = c8/*getTypeMirror:DECLARED:java.lang.Integer*/;}); + consumeVarArgs2(0, c9 -> {Object o = c9/*getTypeMirror:DECLARED:java.lang.Integer*/;}, c9 -> {Object o = c9/*getTypeMirror:DECLARED:java.lang.Integer*/;}); } public R convert(T t, Function f, int i) { return null; @@ -44,6 +46,11 @@ public R convertVarArgs(T t, Function c, int... i) { } public void consumeVarArgs(T t, Consumer c, int... i) { } + public R convertVarArgs2(T t, Function... c) { + return null; + } + public void consumeVarArgs2(T t, Consumer... c) { + } public static class Test { public static Test of(T t) { return new Test<>(); From 988f85be87778dbbd1ce7b9e65212a912bd2de6c Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Fri, 11 Sep 2020 14:57:26 +0200 Subject: [PATCH 3/3] Moving the TestGetTypeMirrorReference test into a separate folder. --- .../TestGetTypeMirrorReference.java | 16 +++++++++++++++- .../TestGetTypeMirrorReferenceData.java | 0 2 files changed, 15 insertions(+), 1 deletion(-) rename test/langtools/tools/javac/api/{ => lambdaErrorRecovery}/TestGetTypeMirrorReference.java (92%) rename test/langtools/tools/javac/api/{ => lambdaErrorRecovery}/TestGetTypeMirrorReferenceData.java (100%) diff --git a/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java b/test/langtools/tools/javac/api/lambdaErrorRecovery/TestGetTypeMirrorReference.java similarity index 92% rename from test/langtools/tools/javac/api/TestGetTypeMirrorReference.java rename to test/langtools/tools/javac/api/lambdaErrorRecovery/TestGetTypeMirrorReference.java index 5347169658c69..741835a0a4fb9 100644 --- a/test/langtools/tools/javac/api/TestGetTypeMirrorReference.java +++ b/test/langtools/tools/javac/api/lambdaErrorRecovery/TestGetTypeMirrorReference.java @@ -49,10 +49,24 @@ import javax.tools.StandardJavaFileManager; import javax.tools.ToolProvider; +/* + * This test verifies proper error recovery for method invocations which need + * type inference, and have lambdas as arguments. + * + * The test will read the adjacent TestGetTypeMirrorReferenceData.java, parse and + * attribute it, and call Trees.getTypeMirror on each place marked, in a block + * comment, with: + * getTypeMirror:: + * The actual retrieved TypeMirror will be checked against the provided description, + * verifying the return value of TypeMirror.getKind and TypeMirror.toString(). + * + * The AST for TestGetTypeMirrorReferenceData.java will also be printed using + * Tree.toString(), and compared to the expected AST form. + */ public class TestGetTypeMirrorReference { private static final String JDK_VERSION = - Integer.toString(Runtime.getRuntime().version().feature()); + Integer.toString(Runtime.version().feature()); public static void main(String... args) throws IOException { analyze("TestGetTypeMirrorReferenceData.java", diff --git a/test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java b/test/langtools/tools/javac/api/lambdaErrorRecovery/TestGetTypeMirrorReferenceData.java similarity index 100% rename from test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java rename to test/langtools/tools/javac/api/lambdaErrorRecovery/TestGetTypeMirrorReferenceData.java