8295024: Cyclic constructor error is non-deterministic and inconsistent

Reviewed-by: vromero
This commit is contained in:
Archie L. Cobbs 2022-10-14 13:52:50 +00:00 committed by Vicente Romero
parent 1efa93e602
commit 786ce1c27b
3 changed files with 103 additions and 22 deletions
src/jdk.compiler/share/classes/com/sun/tools/javac
test/langtools/tools/javac/Diagnostics/8295024

@ -3689,7 +3689,8 @@ public class Check {
* constructors.
*/
void checkCyclicConstructors(JCClassDecl tree) {
Map<Symbol,Symbol> callMap = new HashMap<>();
// use LinkedHashMap so we generate errors deterministically
Map<Symbol,Symbol> callMap = new LinkedHashMap<>();
// enter each constructor this-call into the map
for (List<JCTree> l = tree.defs; l.nonEmpty(); l = l.tail) {
@ -3718,7 +3719,7 @@ public class Check {
Map<Symbol,Symbol> callMap) {
if (ctor != null && (ctor.flags_field & ACYCLIC) == 0) {
if ((ctor.flags_field & LOCKED) != 0) {
log.error(TreeInfo.diagnosticPositionFor(ctor, tree),
log.error(TreeInfo.diagnosticPositionFor(ctor, tree, false, t -> t.hasTag(IDENT)),
Errors.RecursiveCtorInvocation);
} else {
ctor.flags_field |= LOCKED;

@ -48,6 +48,7 @@ import static com.sun.tools.javac.tree.JCTree.Tag.SYNCHRONIZED;
import javax.tools.JavaFileObject;
import java.util.function.Predicate;
import java.util.function.ToIntFunction;
import static com.sun.tools.javac.tree.JCTree.JCOperatorExpression.OperandPos.LEFT;
@ -710,21 +711,26 @@ public class TreeInfo {
}
public static DiagnosticPosition diagnosticPositionFor(final Symbol sym, final JCTree tree, boolean returnNullIfNotFound) {
return diagnosticPositionFor(sym, tree, returnNullIfNotFound, null);
}
public static DiagnosticPosition diagnosticPositionFor(final Symbol sym, final JCTree tree, boolean returnNullIfNotFound,
Predicate<? super JCTree> filter) {
class DiagScanner extends DeclScanner {
DiagScanner(Symbol sym) {
super(sym);
DiagScanner(Symbol sym, Predicate<? super JCTree> filter) {
super(sym, filter);
}
public void visitIdent(JCIdent that) {
if (that.sym == sym) result = that;
else super.visitIdent(that);
if (!checkMatch(that, that.sym))
super.visitIdent(that);
}
public void visitSelect(JCFieldAccess that) {
if (that.sym == sym) result = that;
else super.visitSelect(that);
if (!checkMatch(that, that.sym))
super.visitSelect(that);
}
}
DiagScanner s = new DiagScanner(sym);
DiagScanner s = new DiagScanner(sym, filter);
tree.accept(s);
JCTree decl = s.result;
if (decl == null && returnNullIfNotFound) { return null; }
@ -737,9 +743,14 @@ public class TreeInfo {
private static class DeclScanner extends TreeScanner {
final Symbol sym;
final Predicate<? super JCTree> filter;
DeclScanner(final Symbol sym) {
this(sym, null);
}
DeclScanner(final Symbol sym, Predicate<? super JCTree> filter) {
this.sym = sym;
this.filter = filter;
}
JCTree result = null;
@ -748,32 +759,40 @@ public class TreeInfo {
tree.accept(this);
}
public void visitTopLevel(JCCompilationUnit that) {
if (that.packge == sym) result = that;
else super.visitTopLevel(that);
if (!checkMatch(that, that.packge))
super.visitTopLevel(that);
}
public void visitModuleDef(JCModuleDecl that) {
if (that.sym == sym) result = that;
checkMatch(that, that.sym);
// no need to scan within module declaration
}
public void visitPackageDef(JCPackageDecl that) {
if (that.packge == sym) result = that;
else super.visitPackageDef(that);
if (!checkMatch(that, that.packge))
super.visitPackageDef(that);
}
public void visitClassDef(JCClassDecl that) {
if (that.sym == sym) result = that;
else super.visitClassDef(that);
if (!checkMatch(that, that.sym))
super.visitClassDef(that);
}
public void visitMethodDef(JCMethodDecl that) {
if (that.sym == sym) result = that;
else super.visitMethodDef(that);
if (!checkMatch(that, that.sym))
super.visitMethodDef(that);
}
public void visitVarDef(JCVariableDecl that) {
if (that.sym == sym) result = that;
else super.visitVarDef(that);
if (!checkMatch(that, that.sym))
super.visitVarDef(that);
}
public void visitTypeParameter(JCTypeParameter that) {
if (that.type != null && that.type.tsym == sym) result = that;
else super.visitTypeParameter(that);
if (that.type == null || !checkMatch(that, that.type.tsym))
super.visitTypeParameter(that);
}
protected boolean checkMatch(JCTree that, Symbol thatSym) {
if (thatSym == this.sym && (filter == null || filter.test(that))) {
result = that;
return true;
}
return false;
}
}

@ -0,0 +1,61 @@
/**
* @test /nodynamiccopyright/
* @bug 8295024
* @summary Cyclic constructor error is non-deterministic and inconsistent
*/
import java.io.*;
import java.net.*;
import java.util.*;
import java.util.stream.*;
import javax.tools.*;
public class T8295024 {
private static final int NUM_RUNS = 10;
private static final String EXPECTED_ERROR = """
Cyclic.java:12:9: compiler.err.recursive.ctor.invocation
1 error
""";
private static final String SOURCE = """
public class Cyclic {
public Cyclic(int x) {
this((float)x);
}
public Cyclic(float x) {
this((long)x);
}
public Cyclic(long x) {
this((double)x);
}
public Cyclic(double x) {
this((int)x);
// ^ error should be reported here every time
}
}
""";
private static final SimpleJavaFileObject FILE = new SimpleJavaFileObject(
URI.create("string:///Cyclic.java"), JavaFileObject.Kind.SOURCE) {
@Override
public String getCharContent(boolean ignoreEncodingErrors) {
return SOURCE;
}
};
public static void main(String[] args) throws Exception {
// Compile program NUM_RUNS times
final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
final StringWriter output = new StringWriter();
final Iterable<String> options = Collections.singleton("-XDrawDiagnostics");
final Iterable<SimpleJavaFileObject> files = Collections.singleton(FILE);
for (int i = 0; i < NUM_RUNS; i++)
compiler.getTask(output, null, null, options, null, files).call();
// Verify consistent error report each time
final String expected = IntStream.range(0, NUM_RUNS)
.mapToObj(i -> EXPECTED_ERROR)
.collect(Collectors.joining(""));
final String actual = output.toString().replaceAll("\\r", "");
assert expected.equals(actual) : "EXPECTED:\n" + expected + "ACTUAL:\n" + actual;
}
}