# HG changeset patch # User jlahoda # Date 1567160656 -7200 # Node ID ed7eb20871c55447996d4af24746dc3011f485a1 # Parent 4b436b5d16305485de2f31c81c22b38fddd44cf3 8230105: -XDfind=diamond crashes Summary: Avoiding side-effects in Analyzer's speculative attribution. Reviewed-by: mcimadamore, vromero diff -r 4b436b5d1630 -r ed7eb20871c5 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java Fri Aug 30 10:44:06 2019 +0200 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java Fri Aug 30 12:24:16 2019 +0200 @@ -26,11 +26,12 @@ package com.sun.tools.javac.comp; import java.util.ArrayDeque; +import java.util.Arrays; import java.util.EnumSet; import java.util.HashMap; import java.util.Map; import java.util.Queue; -import java.util.function.Predicate; +import java.util.stream.Collectors; import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.NewClassTree; @@ -42,6 +43,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; import com.sun.tools.javac.comp.ArgumentAttr.LocalCacheContext; +import com.sun.tools.javac.comp.DeferredAttr.AttributionMode; import com.sun.tools.javac.resources.CompilerProperties.Warnings; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.JCTree.JCBlock; @@ -446,7 +448,7 @@ */ Env copyEnvIfNeeded(JCTree tree, Env env) { if (!analyzerModes.isEmpty() && - !env.info.isSpeculative && + !env.info.attributionMode.isSpeculative && TreeInfo.isStatement(tree) && !tree.hasTag(LABELLED)) { Env analyzeEnv = @@ -562,10 +564,14 @@ //TODO: to further refine the analysis, try all rewriting combinations deferredAttr.attribSpeculative(treeToAnalyze, rewriting.env, attr.statInfo, new TreeRewriter(rewriting), - t -> rewriting.diagHandler(), argumentAttr.withLocalCacheContext()); + t -> rewriting.diagHandler(), AttributionMode.ANALYZER, argumentAttr.withLocalCacheContext()); rewriting.analyzer.process(rewriting.oldTree, rewriting.replacement, rewriting.erroneous); } catch (Throwable ex) { - Assert.error("Analyzer error when processing: " + rewriting.originalTree); + Assert.error("Analyzer error when processing: " + + rewriting.originalTree + ":" + ex.toString() + "\n" + + Arrays.stream(ex.getStackTrace()) + .map(se -> se.toString()) + .collect(Collectors.joining("\n"))); } finally { log.useSource(prevSource.getFile()); localCacheContext.leave(); @@ -629,6 +635,11 @@ } @Override + public void visitLambda(JCLambda tree) { + //do nothing (prevents seeing same stuff in lambda expression twice) + } + + @Override public void visitSwitch(JCSwitch tree) { scan(tree.getExpression()); } diff -r 4b436b5d1630 -r ed7eb20871c5 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java Fri Aug 30 10:44:06 2019 +0200 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java Fri Aug 30 12:24:16 2019 +0200 @@ -136,7 +136,7 @@ */ void setResult(JCExpression tree, Type type) { result = type; - if (env.info.isSpeculative) { + if (env.info.attributionMode == DeferredAttr.AttributionMode.SPECULATIVE) { //if we are in a speculative branch we can save the type in the tree itself //as there's no risk of polluting the original tree. tree.type = result; @@ -365,7 +365,7 @@ speculativeTypes.put(resultInfo, t); return t; } else { - if (!env.info.isSpeculative) { + if (!env.info.attributionMode.isSpeculative) { argumentTypeCache.remove(new UniquePos(dt.tree)); } return deferredAttr.basicCompleter.complete(dt, resultInfo, deferredAttrContext); diff -r 4b436b5d1630 -r ed7eb20871c5 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Fri Aug 30 10:44:06 2019 +0200 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Fri Aug 30 12:24:16 2019 +0200 @@ -735,11 +735,9 @@ */ public Type attribStat(JCTree tree, Env env) { Env analyzeEnv = analyzer.copyEnvIfNeeded(tree, env); - try { - return attribTree(tree, env, statInfo); - } finally { - analyzer.analyzeIfNeeded(tree, analyzeEnv); - } + Type result = attribTree(tree, env, statInfo); + analyzer.analyzeIfNeeded(tree, analyzeEnv); + return result; } /** Attribute a list of expressions, returning a list of types. @@ -933,7 +931,7 @@ public void visitClassDef(JCClassDecl tree) { Optional localCacheContext = - Optional.ofNullable(env.info.isSpeculative ? + Optional.ofNullable(env.info.attributionMode.isSpeculative ? argumentAttr.withLocalCacheContext() : null); try { // Local and anonymous classes have not been entered yet, so we need to @@ -3232,7 +3230,7 @@ return; } - if (!env.info.isSpeculative && that.getMode() == JCMemberReference.ReferenceMode.NEW) { + if (!env.info.attributionMode.isSpeculative && that.getMode() == JCMemberReference.ReferenceMode.NEW) { Type enclosingType = exprType.getEnclosingType(); if (enclosingType != null && enclosingType.hasTag(CLASS)) { // Check for the existence of an apropriate outer instance diff -r 4b436b5d1630 -r ed7eb20871c5 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java Fri Aug 30 10:44:06 2019 +0200 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java Fri Aug 30 12:24:16 2019 +0200 @@ -29,6 +29,7 @@ import com.sun.tools.javac.util.*; import com.sun.tools.javac.code.*; import com.sun.tools.javac.code.Scope.WriteableScope; +import com.sun.tools.javac.comp.DeferredAttr.AttributionMode; /** Contains information specific to the attribute and enter * passes, to be used in place of the generic field in environments. @@ -67,7 +68,7 @@ /** Is this a speculative attribution environment? */ - boolean isSpeculative = false; + AttributionMode attributionMode = AttributionMode.FULL; /** * Is this an attribution environment for an anonymous class instantiated using <> ? @@ -133,7 +134,7 @@ info.defaultSuperCallSite = defaultSuperCallSite; info.isSerializable = isSerializable; info.isLambda = isLambda; - info.isSpeculative = isSpeculative; + info.attributionMode = attributionMode; info.isAnonymousDiamond = isAnonymousDiamond; info.isNewClass = isNewClass; info.preferredTreeForDiagnostics = preferredTreeForDiagnostics; diff -r 4b436b5d1630 -r ed7eb20871c5 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java Fri Aug 30 10:44:06 2019 +0200 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java Fri Aug 30 12:24:16 2019 +0200 @@ -481,25 +481,28 @@ */ JCTree attribSpeculative(JCTree tree, Env env, ResultInfo resultInfo) { return attribSpeculative(tree, env, resultInfo, treeCopier, - (newTree)->new DeferredAttrDiagHandler(log, newTree), null); + (newTree)->new DeferredAttrDiagHandler(log, newTree), AttributionMode.SPECULATIVE, null); } JCTree attribSpeculative(JCTree tree, Env env, ResultInfo resultInfo, LocalCacheContext localCache) { return attribSpeculative(tree, env, resultInfo, treeCopier, - (newTree)->new DeferredAttrDiagHandler(log, newTree), localCache); + (newTree)->new DeferredAttrDiagHandler(log, newTree), AttributionMode.SPECULATIVE, localCache); } JCTree attribSpeculative(JCTree tree, Env env, ResultInfo resultInfo, TreeCopier deferredCopier, - Function diagHandlerCreator, + Function diagHandlerCreator, AttributionMode attributionMode, LocalCacheContext localCache) { final JCTree newTree = deferredCopier.copy(tree); Env speculativeEnv = env.dup(newTree, env.info.dup(env.info.scope.dupUnshared(env.info.scope.owner))); - speculativeEnv.info.isSpeculative = true; + speculativeEnv.info.attributionMode = attributionMode; Log.DeferredDiagnosticHandler deferredDiagnosticHandler = diagHandlerCreator.apply(newTree); + int nwarnings = log.nwarnings; + log.nwarnings = 0; try { attr.attribTree(newTree, speculativeEnv, resultInfo); return newTree; } finally { + log.nwarnings += nwarnings; enter.unenter(env.toplevel, newTree); log.popDiagnosticHandler(deferredDiagnosticHandler); if (localCache != null) { @@ -1286,4 +1289,26 @@ } } } + + /** + * Mode of attribution (used in AttrContext). + */ + enum AttributionMode { + /**Normal, non-speculative, attribution.*/ + FULL(false), + /**Speculative attribution on behalf of an Analyzer.*/ + ANALYZER(true), + /**Speculative attribution.*/ + SPECULATIVE(true); + + AttributionMode(boolean isSpeculative) { + this.isSpeculative = isSpeculative; + } + + boolean isSpeculative() { + return isSpeculative; + } + + final boolean isSpeculative; + } } diff -r 4b436b5d1630 -r ed7eb20871c5 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java Fri Aug 30 10:44:06 2019 +0200 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java Fri Aug 30 12:24:16 2019 +0200 @@ -2391,7 +2391,7 @@ if (kind.contains(KindSelector.TYP)) { RecoveryLoadClass recoveryLoadClass = allowModules && !kind.contains(KindSelector.PCK) && - !pck.exists() && !env.info.isSpeculative ? + !pck.exists() && !env.info.attributionMode.isSpeculative ? doRecoveryLoadClass : noRecovery; Symbol sym = loadClass(env, fullname, recoveryLoadClass); if (sym.exists()) { diff -r 4b436b5d1630 -r ed7eb20871c5 test/langtools/tools/javac/analyzer/AnalyzerMandatoryWarnings.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/analyzer/AnalyzerMandatoryWarnings.java Fri Aug 30 12:24:16 2019 +0200 @@ -0,0 +1,16 @@ +/** + * @test + * @bug 8230105 + * @summary Verify analyzers work reasonably in combination with mandatory warnings + * @compile/ref=AnalyzerMandatoryWarnings.out -Xlint:deprecation -XDrawDiagnostics -Xmaxwarns 1 -XDfind=lambda AnalyzerMandatoryWarnings.java + */ +public class AnalyzerMandatoryWarnings { + private void test() { + Runnable r = new Runnable() { + public void run() { + Depr r; + } + }; + } +} +@Deprecated class Depr {} diff -r 4b436b5d1630 -r ed7eb20871c5 test/langtools/tools/javac/analyzer/AnalyzerMandatoryWarnings.out --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/analyzer/AnalyzerMandatoryWarnings.out Fri Aug 30 12:24:16 2019 +0200 @@ -0,0 +1,2 @@ +AnalyzerMandatoryWarnings.java:11:17: compiler.warn.has.been.deprecated: Depr, compiler.misc.unnamed.package +1 warning diff -r 4b436b5d1630 -r ed7eb20871c5 test/langtools/tools/javac/analyzer/AnalyzerNotQuiteSpeculative.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/analyzer/AnalyzerNotQuiteSpeculative.java Fri Aug 30 12:24:16 2019 +0200 @@ -0,0 +1,25 @@ +/** + * @test /nodynamiccopyright/ + * @bug 8230105 + * @summary Ensuring speculative analysis on behalf of Analyzers works reasonably. + * @compile/ref=AnalyzerNotQuiteSpeculative.out -XDfind=diamond -XDrawDiagnostics AnalyzerNotQuiteSpeculative.java + */ +public class AnalyzerNotQuiteSpeculative { + private void test() { + Subclass1 c1 = null; + Subclass2 c2 = null; + Base b = null; + + t(new C(c1).set(c2)); + t(new C(b).set(c2)); + } + + public static class Base {} + public static class Subclass1 extends Base {} + public static class Subclass2 extends Base {} + public class C { + public C(T t) {} + public C set(T t) { return this; } + } + void t(C l) {} +} diff -r 4b436b5d1630 -r ed7eb20871c5 test/langtools/tools/javac/analyzer/AnalyzerNotQuiteSpeculative.out --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/analyzer/AnalyzerNotQuiteSpeculative.out Fri Aug 30 12:24:16 2019 +0200 @@ -0,0 +1,2 @@ +AnalyzerNotQuiteSpeculative.java:14:16: compiler.warn.diamond.redundant.args +1 warning diff -r 4b436b5d1630 -r ed7eb20871c5 test/langtools/tools/javac/analyzer/DoNoRunAnalyzersWhenException.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/analyzer/DoNoRunAnalyzersWhenException.java Fri Aug 30 12:24:16 2019 +0200 @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2015, 2019, 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 8230105 + * @summary Do not run Analyzers when Attr crashes with an exception. + * @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 + */ + +import java.io.IOException; +import java.net.URI; +import java.util.Arrays; +import java.util.List; +import javax.tools.JavaFileObject; +import javax.tools.SimpleJavaFileObject; + +import com.sun.tools.javac.api.JavacTaskImpl; +import com.sun.tools.javac.api.JavacTool; +import com.sun.tools.javac.comp.Attr; +import com.sun.tools.javac.tree.JCTree.JCVariableDecl; +import com.sun.tools.javac.util.Context; +import com.sun.tools.javac.util.Context.Factory; + +public class DoNoRunAnalyzersWhenException { + + public static void main(String... args) throws Exception { + new DoNoRunAnalyzersWhenException().run(); + } + + void run() throws IOException { + JavacTool tool = JavacTool.create(); + JavaSource source = new JavaSource("class Test {" + + " { String STOP = \"\"; }" + + "}"); + Context context = new Context(); + CrashingAttr.preRegister(context); + List inputs = Arrays.asList(source); + JavacTaskImpl task = + (JavacTaskImpl) tool.getTask(null, null, null, List.of("-XDfind=all"), null, inputs, context); + try { + task.analyze(null); + throw new AssertionError("Expected exception not seen."); + } catch (StopException ex) { + //ok + } + } + + static class CrashingAttr extends Attr { + + static void preRegister(Context context) { + context.put(attrKey, (Factory) c -> new CrashingAttr(c)); + } + + CrashingAttr(Context context) { + super(context); + } + + @Override public void visitVarDef(JCVariableDecl tree) { + if (tree.name.contentEquals("STOP")) + throw new StopException(); + super.visitVarDef(tree); + } + } + + static class StopException extends NullPointerException {} + + class JavaSource extends SimpleJavaFileObject { + + String source; + + JavaSource(String source) { + super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE); + this.source = source; + } + + @Override + public CharSequence getCharContent(boolean ignoreEncodingErrors) { + return source; + } + + } + +} diff -r 4b436b5d1630 -r ed7eb20871c5 test/langtools/tools/javac/analyzer/StuckLambdas.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/analyzer/StuckLambdas.java Fri Aug 30 12:24:16 2019 +0200 @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2017, 2019, 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 8230105 + * @summary Verify the analyzers work reasonably for stuck lambdas + * @compile/ref=StuckLambdas.out -XDfind=local -XDrawDiagnostics StuckLambdas.java + */ + +import java.util.Map; +import java.util.concurrent.ExecutorService; +import java.util.function.*; +import java.util.stream.*; + +abstract class X { + public interface N { + Stream getValues(); + } + + abstract N c(); + + abstract > Collector f( + Function k, + Function> v, + Supplier multimapSupplier); + + void m(Map> c, ExecutorService s) { + s.submit(() -> { + String s1 = ""; + return c.entrySet() + .parallelStream() + .collect(f(Map.Entry::getKey, e -> {String s2 = ""; return e.getValue().getValues();}, this::c)); + }); + } +} + diff -r 4b436b5d1630 -r ed7eb20871c5 test/langtools/tools/javac/analyzer/StuckLambdas.out --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/analyzer/StuckLambdas.out Fri Aug 30 12:24:16 2019 +0200 @@ -0,0 +1,3 @@ +StuckLambdas.java:50:20: compiler.warn.local.redundant.type +StuckLambdas.java:53:64: compiler.warn.local.redundant.type +2 warnings