6863465: javac doesn't detect circular subclass dependencies via qualified names
Summary: class inheritance circularity check should look at trees, not just symbols
Reviewed-by: jjg
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java Sat Sep 18 09:56:23 2010 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java Sat Sep 18 14:24:09 2010 -0700
@@ -60,6 +60,7 @@
private final Names names;
private final Log log;
private final Symtab syms;
+ private final Enter enter;
private final Infer infer;
private final Types types;
private final JCDiagnostic.Factory diags;
@@ -86,6 +87,7 @@
names = Names.instance(context);
log = Log.instance(context);
syms = Symtab.instance(context);
+ enter = Enter.instance(context);
infer = Infer.instance(context);
this.types = Types.instance(context);
diags = JCDiagnostic.Factory.instance(context);
@@ -1727,6 +1729,113 @@
return undef;
}
+ void checkNonCyclicDecl(JCClassDecl tree) {
+ CycleChecker cc = new CycleChecker();
+ cc.scan(tree);
+ if (!cc.errorFound && !cc.partialCheck) {
+ tree.sym.flags_field |= ACYCLIC;
+ }
+ }
+
+ class CycleChecker extends TreeScanner {
+
+ List<Symbol> seenClasses = List.nil();
+ boolean errorFound = false;
+ boolean partialCheck = false;
+
+ private void checkSymbol(DiagnosticPosition pos, Symbol sym) {
+ if (sym != null && sym.kind == TYP) {
+ Env<AttrContext> classEnv = enter.getEnv((TypeSymbol)sym);
+ if (classEnv != null) {
+ DiagnosticSource prevSource = log.currentSource();
+ try {
+ log.useSource(classEnv.toplevel.sourcefile);
+ scan(classEnv.tree);
+ }
+ finally {
+ log.useSource(prevSource.getFile());
+ }
+ } else if (sym.kind == TYP) {
+ checkClass(pos, sym, List.<JCTree>nil());
+ }
+ } else {
+ //not completed yet
+ partialCheck = true;
+ }
+ }
+
+ @Override
+ public void visitSelect(JCFieldAccess tree) {
+ super.visitSelect(tree);
+ checkSymbol(tree.pos(), tree.sym);
+ }
+
+ @Override
+ public void visitIdent(JCIdent tree) {
+ checkSymbol(tree.pos(), tree.sym);
+ }
+
+ @Override
+ public void visitTypeApply(JCTypeApply tree) {
+ scan(tree.clazz);
+ }
+
+ @Override
+ public void visitTypeArray(JCArrayTypeTree tree) {
+ scan(tree.elemtype);
+ }
+
+ @Override
+ public void visitClassDef(JCClassDecl tree) {
+ List<JCTree> supertypes = List.nil();
+ if (tree.getExtendsClause() != null) {
+ supertypes = supertypes.prepend(tree.getExtendsClause());
+ }
+ if (tree.getImplementsClause() != null) {
+ for (JCTree intf : tree.getImplementsClause()) {
+ supertypes = supertypes.prepend(intf);
+ }
+ }
+ checkClass(tree.pos(), tree.sym, supertypes);
+ }
+
+ void checkClass(DiagnosticPosition pos, Symbol c, List<JCTree> supertypes) {
+ if ((c.flags_field & ACYCLIC) != 0)
+ return;
+ if (seenClasses.contains(c)) {
+ errorFound = true;
+ noteCyclic(pos, (ClassSymbol)c);
+ } else if (!c.type.isErroneous()) {
+ try {
+ seenClasses = seenClasses.prepend(c);
+ if (c.type.tag == CLASS) {
+ if (supertypes.nonEmpty()) {
+ scan(supertypes);
+ }
+ else {
+ ClassType ct = (ClassType)c.type;
+ if (ct.supertype_field == null ||
+ ct.interfaces_field == null) {
+ //not completed yet
+ partialCheck = true;
+ return;
+ }
+ checkSymbol(pos, ct.supertype_field.tsym);
+ for (Type intf : ct.interfaces_field) {
+ checkSymbol(pos, intf.tsym);
+ }
+ }
+ if (c.owner.kind == TYP) {
+ checkSymbol(pos, c.owner);
+ }
+ }
+ } finally {
+ seenClasses = seenClasses.tail;
+ }
+ }
+ }
+ }
+
/** Check for cyclic references. Issue an error if the
* symbol of the type referred to has a LOCKED flag set.
*
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/MemberEnter.java Sat Sep 18 09:56:23 2010 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/MemberEnter.java Sat Sep 18 14:24:09 2010 -0700
@@ -927,7 +927,7 @@
tp.accept(new TypeAnnotate(baseEnv));
tree.accept(new TypeAnnotate(env));
- chk.checkNonCyclic(tree.pos(), c.type);
+ chk.checkNonCyclicDecl(tree);
attr.attribTypeVariables(tree.typarams, baseEnv);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/T6863465a.java Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,14 @@
+/**
+ * @test /nodynamiccopyright/
+ * @bug 6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @author Maurizio Cimadamore
+ * @compile/fail/ref=T6863465a.out -XDrawDiagnostics T6863465a.java
+ */
+
+class T6863465a {
+ static class a { static interface b {} }
+ static class c extends a implements z.y {}
+ static class x { static interface y {} }
+ static class z extends x implements c.b {}
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/T6863465a.out Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,2 @@
+T6863465a.java:11:12: compiler.err.cyclic.inheritance: T6863465a.c
+1 error
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/T6863465b.java Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,14 @@
+/**
+ * @test /nodynamiccopyright/
+ * @bug 6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @author Maurizio Cimadamore
+ * @compile/fail/ref=T6863465b.out -XDrawDiagnostics T6863465b.java
+ */
+
+class T6863465b {
+ static class a { static interface b { static interface d {} } }
+ static class c extends a implements z.y, z.d {}
+ static class x { static interface y {} }
+ static class z extends x implements c.b {}
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/T6863465b.out Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,2 @@
+T6863465b.java:11:12: compiler.err.cyclic.inheritance: T6863465b.c
+1 error
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/T6863465c.java Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,14 @@
+/**
+ * @test /nodynamiccopyright/
+ * @bug 6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @author Maurizio Cimadamore
+ * @compile/fail/ref=T6863465c.out -XDrawDiagnostics T6863465c.java
+ */
+
+class T6863465c {
+ static class x { static interface y {} }
+ static class z extends x implements c.b {}
+ static class a { static interface b { static interface d {} } }
+ static class c extends a implements z.y, z.d {}
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/T6863465c.out Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,3 @@
+T6863465c.java:13:47: compiler.err.cant.resolve.location: kindname.class, d, , , kindname.class, T6863465c.z
+T6863465c.java:11:12: compiler.err.cyclic.inheritance: T6863465c.z
+2 errors
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/T6863465d.java Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,14 @@
+/**
+ * @test /nodynamiccopyright/
+ * @bug 6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @author Maurizio Cimadamore
+ * @compile/fail/ref=T6863465d.out -XDrawDiagnostics T6863465d.java
+ */
+
+class T6863465d {
+ static class a { static interface b { static interface d {} } }
+ static class c extends a implements z.y, z.d {}
+ static class x { static interface y { static interface w {} } }
+ static class z extends x implements c.b, c.w {}
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/T6863465d.out Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,3 @@
+T6863465d.java:13:47: compiler.err.cant.resolve.location: kindname.class, w, , , kindname.class, T6863465d.c
+T6863465d.java:11:12: compiler.err.cyclic.inheritance: T6863465d.c
+2 errors
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6863465/TestCircularClassfile.java Sat Sep 18 14:24:09 2010 -0700
@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) 2010, 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 6863465
+ * @summary javac doesn't detect circular subclass dependencies via qualified names
+ * @run main TestCircularClassfile
+ */
+
+import java.io.*;
+import java.net.URI;
+import java.util.Arrays;
+import javax.tools.Diagnostic;
+import javax.tools.DiagnosticListener;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.ToolProvider;
+
+import com.sun.source.util.JavacTask;
+import java.util.EnumSet;
+
+public class TestCircularClassfile {
+
+ enum ClassName {
+ A("A"),
+ B("B"),
+ C("C"),
+ OBJECT("Object");
+
+ String name;
+
+ ClassName(String name) {
+ this.name = name;
+ }
+ }
+
+ static class JavaSource extends SimpleJavaFileObject {
+
+ final static String sourceStub = "class #C extends #S {}";
+
+ String source;
+
+ public JavaSource(ClassName clazz, ClassName sup) {
+ super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
+ source = sourceStub.replace("#C", clazz.name).replace("#S", sup.name);
+ }
+
+ @Override
+ public CharSequence getCharContent(boolean ignoreEncodingErrors) {
+ return source;
+ }
+ }
+
+ public static void main(String... args) throws Exception {
+ int count = 0;
+ for (ClassName clazz : EnumSet.of(ClassName.A, ClassName.B, ClassName.C)) {
+ for (ClassName sup : EnumSet.of(ClassName.A, ClassName.B, ClassName.C)) {
+ if (sup.ordinal() < clazz.ordinal()) continue;
+ check("sub_"+count++, clazz, sup);
+ }
+ }
+ }
+
+ static JavaSource[] initialSources = new JavaSource[] {
+ new JavaSource(ClassName.A, ClassName.OBJECT),
+ new JavaSource(ClassName.B, ClassName.A),
+ new JavaSource(ClassName.C, ClassName.B)
+ };
+
+ static String workDir = System.getProperty("user.dir");
+
+ static void check(String destPath, ClassName clazz, ClassName sup) throws Exception {
+ File destDir = new File(workDir, destPath); destDir.mkdir();
+ final JavaCompiler tool = ToolProvider.getSystemJavaCompiler();
+ JavacTask ct = (JavacTask)tool.getTask(null, null, null,
+ Arrays.asList("-d", destPath), null, Arrays.asList(initialSources));
+ ct.generate();
+ File fileToRemove = new File(destPath, clazz.name + ".class");
+ fileToRemove.delete();
+ JavaSource newSource = new JavaSource(clazz, sup);
+ DiagnosticChecker checker = new DiagnosticChecker();
+ ct = (JavacTask)tool.getTask(null, null, checker,
+ Arrays.asList("-cp", destPath), null, Arrays.asList(newSource));
+ ct.analyze();
+ if (!checker.errorFound) {
+ throw new AssertionError(newSource.source);
+ }
+ }
+
+ static class DiagnosticChecker implements DiagnosticListener<JavaFileObject> {
+
+ boolean errorFound = false;
+
+ public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
+ if (diagnostic.getKind() == Diagnostic.Kind.ERROR &&
+ diagnostic.getCode().equals("compiler.err.cyclic.inheritance")) {
+ errorFound = true;
+ }
+ }
+ }
+}
--- a/langtools/test/tools/javac/CyclicInheritance.out Sat Sep 18 09:56:23 2010 -0700
+++ b/langtools/test/tools/javac/CyclicInheritance.out Sat Sep 18 14:24:09 2010 -0700
@@ -4,6 +4,6 @@
CyclicInheritance.java:22:1: compiler.err.cyclic.inheritance: I11
CyclicInheritance.java:27:1: compiler.err.cyclic.inheritance: C211
CyclicInheritance.java:31:1: compiler.err.cyclic.inheritance: C212
-CyclicInheritance.java:36:27: compiler.err.report.access: C221.I, private, C221
-CyclicInheritance.java:40:24: compiler.err.report.access: C222.C, private, C222
+CyclicInheritance.java:36:1: compiler.err.cyclic.inheritance: C221
+CyclicInheritance.java:40:1: compiler.err.cyclic.inheritance: C222
8 errors
--- a/langtools/test/tools/javac/NameCollision.out Sat Sep 18 09:56:23 2010 -0700
+++ b/langtools/test/tools/javac/NameCollision.out Sat Sep 18 14:24:09 2010 -0700
@@ -1,2 +1,3 @@
NameCollision.java:13:31: compiler.err.intf.expected.here
-1 error
+NameCollision.java:13:5: compiler.err.cyclic.inheritance: NameCollision.Runnable
+2 errors