8230105: -XDfind=diamond crashes

Avoiding side-effects in Analyzer's speculative attribution.

Reviewed-by: mcimadamore, vromero
This commit is contained in:
Jan Lahoda 2019-08-30 12:24:16 +02:00
parent 277ef756c4
commit 1d71dd8604
13 changed files with 267 additions and 20 deletions

View File

@ -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.Symbol.ClassSymbol;
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 @@ public class Analyzer {
*/
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 @@ public class Analyzer {
//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();
@ -628,6 +634,11 @@ public class Analyzer {
//do nothing (prevents seeing same stuff twice)
}
@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());

View File

@ -136,7 +136,7 @@ public class ArgumentAttr extends JCTree.Visitor {
*/
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 @@ public class ArgumentAttr extends JCTree.Visitor {
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);

View File

@ -735,11 +735,9 @@ public class Attr extends JCTree.Visitor {
*/
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 class Attr extends JCTree.Visitor {
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 @@ public class Attr extends JCTree.Visitor {
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

View File

@ -29,6 +29,7 @@ import com.sun.tools.javac.tree.JCTree;
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 @@ public class AttrContext {
/** 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 @@ public class AttrContext {
info.defaultSuperCallSite = defaultSuperCallSite;
info.isSerializable = isSerializable;
info.isLambda = isLambda;
info.isSpeculative = isSpeculative;
info.attributionMode = attributionMode;
info.isAnonymousDiamond = isAnonymousDiamond;
info.isNewClass = isNewClass;
info.preferredTreeForDiagnostics = preferredTreeForDiagnostics;

View File

@ -481,25 +481,28 @@ public class DeferredAttr extends JCTree.Visitor {
*/
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 @@ public class DeferredAttr extends JCTree.Visitor {
}
}
}
/**
* 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;
}
}

View File

@ -2391,7 +2391,7 @@ public class Resolve {
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()) {

View File

@ -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 {}

View File

@ -0,0 +1,2 @@
AnalyzerMandatoryWarnings.java:11:17: compiler.warn.has.been.deprecated: Depr, compiler.misc.unnamed.package
1 warning

View File

@ -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) {}
}

View File

@ -0,0 +1,2 @@
AnalyzerNotQuiteSpeculative.java:14:16: compiler.warn.diamond.redundant.args
1 warning

View File

@ -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;
}
}
}

View File

@ -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));
});
}
}

View File

@ -0,0 +1,3 @@
StuckLambdas.java:50:20: compiler.warn.local.redundant.type
StuckLambdas.java:53:64: compiler.warn.local.redundant.type
2 warnings