8230105: -XDfind=diamond crashes
authorjlahoda
Fri, 30 Aug 2019 12:24:16 +0200
changeset 57963 ed7eb20871c5
parent 57962 4b436b5d1630
child 57964 6bee0a3d2a3a
8230105: -XDfind=diamond crashes Summary: Avoiding side-effects in Analyzer's speculative attribution. Reviewed-by: mcimadamore, vromero
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
test/langtools/tools/javac/analyzer/AnalyzerMandatoryWarnings.java
test/langtools/tools/javac/analyzer/AnalyzerMandatoryWarnings.out
test/langtools/tools/javac/analyzer/AnalyzerNotQuiteSpeculative.java
test/langtools/tools/javac/analyzer/AnalyzerNotQuiteSpeculative.out
test/langtools/tools/javac/analyzer/DoNoRunAnalyzersWhenException.java
test/langtools/tools/javac/analyzer/StuckLambdas.java
test/langtools/tools/javac/analyzer/StuckLambdas.out
--- 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<AttrContext> copyEnvIfNeeded(JCTree tree, Env<AttrContext> env) {
         if (!analyzerModes.isEmpty() &&
-                !env.info.isSpeculative &&
+                !env.info.attributionMode.isSpeculative &&
                 TreeInfo.isStatement(tree) &&
                 !tree.hasTag(LABELLED)) {
             Env<AttrContext> 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());
         }
--- 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);
--- 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<AttrContext> env) {
         Env<AttrContext> 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<ArgumentAttr.LocalCacheContext> 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
--- 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;
--- 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<AttrContext> 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<AttrContext> 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);
     }
 
     <Z> JCTree attribSpeculative(JCTree tree, Env<AttrContext> env, ResultInfo resultInfo, TreeCopier<Z> deferredCopier,
-                                 Function<JCTree, DeferredDiagnosticHandler> diagHandlerCreator,
+                                 Function<JCTree, DeferredDiagnosticHandler> diagHandlerCreator, AttributionMode attributionMode,
                                  LocalCacheContext localCache) {
         final JCTree newTree = deferredCopier.copy(tree);
         Env<AttrContext> 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;
+    }
 }
--- 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()) {
--- /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 {}
--- /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
--- /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<Base>(c1).set(c2));
+        t(new C<Base>(b).set(c2));
+    }
+
+    public static class Base {}
+    public static class Subclass1 extends Base {}
+    public static class Subclass2 extends Base {}
+    public class C<T extends Base> {
+        public C(T t) {}
+        public C<T> set(T t) { return this; }
+    }
+    <T extends Base> void t(C<? extends Base> l) {}
+}
--- /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
--- /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<JavaSource> 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<Attr>) 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;
+        }
+
+    }
+
+}
--- /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<K, V> {
+        Stream<V> getValues();
+    }
+
+    abstract <K, V> N<K, V> c();
+
+    abstract <T, K, V, M extends N<K, V>> Collector<T, ?, M> f(
+            Function<? super T, ? extends K> k,
+            Function<? super T, ? extends Stream<? extends V>> v,
+            Supplier<M> multimapSupplier);
+
+    void m(Map<String, N<?, ?>> 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));
+        });
+    }
+}
+
--- /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