6970584: Flow.java should be more error-friendly

Summary: Added a post-attribution visitor that fixup uninitialized types/symbol in AST after erroneous attribution
Reviewed-by: jjg

langtools/src/share/classes/com/sun/tools/javac/code/Symtab.java

@@ -93,6 +93,10 @@ public class Symtab {
      */
     public final ClassSymbol errSymbol;
 
+    /** The unknown symbol.
+     */
+    public final ClassSymbol unknownSymbol;
+
     /** A value for the errType, with a originalType of noType */
     public final Type errType;
 
@@ -354,6 +358,7 @@ public class Symtab {
 
         // create the error symbols
         errSymbol = new ClassSymbol(PUBLIC|STATIC|ACYCLIC, names.any, null, rootPackage);
+        unknownSymbol = new ClassSymbol(PUBLIC|STATIC|ACYCLIC, names.fromString("<any?>"), null, rootPackage);
         errType = new ErrorType(errSymbol, Type.noType);
 
         // initialize builtin types
@@ -368,7 +373,7 @@ public class Symtab {
         initType(voidType, "void", "Void");
         initType(botType, "<nulltype>");
         initType(errType, errSymbol);
-        initType(unknownType, "<any?>");
+        initType(unknownType, unknownSymbol);
 
         // the builtin class of all arrays
         arrayClass = new ClassSymbol(PUBLIC|ACYCLIC, names.Array, noSymbol);

langtools/src/share/classes/com/sun/tools/javac/comp/Annotate.java

@@ -182,6 +182,7 @@ public class Annotate {
             if (!method.type.isErroneous())
                 buf.append(new Pair<MethodSymbol,Attribute>
                            ((MethodSymbol)method, value));
+            t.type = result;
         }
         return new Attribute.Compound(a.type, buf.toList());
     }
@@ -234,6 +235,7 @@ public class Annotate {
                                                l.head,
                                                env));
             }
+            na.type = expected;
             return new Attribute.
                 Array(expected, buf.toArray(new Attribute[buf.length()]));
         }

langtools/src/share/classes/com/sun/tools/javac/comp/Attr.java

@@ -3201,4 +3201,104 @@ public class Attr extends JCTree.Visitor {
             super.visitMethodDef(tree);
         }
     };
+
+    // <editor-fold desc="post-attribution visitor">
+
+    /**
+     * Handle missing types/symbols in an AST. This routine is useful when
+     * the compiler has encountered some errors (which might have ended up
+     * terminating attribution abruptly); if the compiler is used in fail-over
+     * mode (e.g. by an IDE) and the AST contains semantic errors, this routine
+     * prevents NPE to be progagated during subsequent compilation steps.
+     */
+    public void postAttr(Env<AttrContext> env) {
+        new PostAttrAnalyzer().scan(env.tree);
+    }
+
+    class PostAttrAnalyzer extends TreeScanner {
+
+        private void initTypeIfNeeded(JCTree that) {
+            if (that.type == null) {
+                that.type = syms.unknownType;
+            }
+        }
+
+        @Override
+        public void scan(JCTree tree) {
+            if (tree == null) return;
+            if (tree instanceof JCExpression) {
+                initTypeIfNeeded(tree);
+            }
+            super.scan(tree);
+        }
+
+        @Override
+        public void visitIdent(JCIdent that) {
+            if (that.sym == null) {
+                that.sym = syms.unknownSymbol;
+            }
+        }
+
+        @Override
+        public void visitSelect(JCFieldAccess that) {
+            if (that.sym == null) {
+                that.sym = syms.unknownSymbol;
+            }
+            super.visitSelect(that);
+        }
+
+        @Override
+        public void visitClassDef(JCClassDecl that) {
+            initTypeIfNeeded(that);
+            if (that.sym == null) {
+                that.sym = new ClassSymbol(0, that.name, that.type, syms.noSymbol);
+            }
+            super.visitClassDef(that);
+        }
+
+        @Override
+        public void visitMethodDef(JCMethodDecl that) {
+            initTypeIfNeeded(that);
+            if (that.sym == null) {
+                that.sym = new MethodSymbol(0, that.name, that.type, syms.noSymbol);
+            }
+            super.visitMethodDef(that);
+        }
+
+        @Override
+        public void visitVarDef(JCVariableDecl that) {
+            initTypeIfNeeded(that);
+            if (that.sym == null) {
+                that.sym = new VarSymbol(0, that.name, that.type, syms.noSymbol);
+                that.sym.adr = 0;
+            }
+            super.visitVarDef(that);
+        }
+
+        @Override
+        public void visitNewClass(JCNewClass that) {
+            if (that.constructor == null) {
+                that.constructor = new MethodSymbol(0, names.init, syms.unknownType, syms.noSymbol);
+            }
+            if (that.constructorType == null) {
+                that.constructorType = syms.unknownType;
+            }
+            super.visitNewClass(that);
+        }
+
+        @Override
+        public void visitBinary(JCBinary that) {
+            if (that.operator == null)
+                that.operator = new OperatorSymbol(names.empty, syms.unknownType, -1, syms.noSymbol);
+            super.visitBinary(that);
+        }
+
+        @Override
+        public void visitUnary(JCUnary that) {
+            if (that.operator == null)
+                that.operator = new OperatorSymbol(names.empty, syms.unknownType, -1, syms.noSymbol);
+            super.visitUnary(that);
+        }
+    }
+    // </editor-fold>
 }

langtools/src/share/classes/com/sun/tools/javac/comp/Flow.java

@@ -408,7 +408,9 @@ public class Flow extends TreeScanner {
         tree = TreeInfo.skipParens(tree);
         if (tree.getTag() == JCTree.IDENT || tree.getTag() == JCTree.SELECT) {
             Symbol sym = TreeInfo.symbol(tree);
-            letInit(tree.pos(), (VarSymbol)sym);
+            if (sym.kind == VAR) {
+                letInit(tree.pos(), (VarSymbol)sym);
+            }
         }
     }
 
@@ -481,12 +483,13 @@ public class Flow extends TreeScanner {
 
     /** Split (duplicate) inits/uninits into WhenTrue/WhenFalse sets
      */
-    void split() {
+    void split(boolean setToNull) {
         initsWhenFalse = inits.dup();
         uninitsWhenFalse = uninits.dup();
         initsWhenTrue = inits;
         uninitsWhenTrue = uninits;
-        inits = uninits = null;
+        if (setToNull)
+            inits = uninits = null;
     }
 
     /** Merge (intersect) inits/uninits from WhenTrue/WhenFalse sets.
@@ -568,9 +571,11 @@ public class Flow extends TreeScanner {
             uninitsWhenTrue = uninits;
         } else {
             scan(tree);
-            if (inits != null) split();
+            if (inits != null)
+                split(tree.type != syms.unknownType);
         }
-        inits = uninits = null;
+        if (tree.type != syms.unknownType)
+            inits = uninits = null;
     }
 
     /* ------------ Visitor methods for various sorts of trees -------------*/
@@ -1007,7 +1012,7 @@ public class Flow extends TreeScanner {
                 List.of(resource.type);
             for (Type sup : closeableSupertypes) {
                 if (types.asSuper(sup, syms.autoCloseableType.tsym) != null) {
-                    Symbol closeMethod = rs.resolveInternalMethod(tree,
+                    Symbol closeMethod = rs.resolveQualifiedMethod(tree,
                             attrEnv,
                             sup,
                             names.close,
@@ -1050,20 +1055,22 @@ public class Flow extends TreeScanner {
             List<Type> rethrownTypes = chk.diff(thrownInTry, caughtInTry);
             for (JCExpression ct : subClauses) {
                 Type exc = ct.type;
-                ctypes = ctypes.append(exc);
-                if (types.isSameType(exc, syms.objectType))
-                    continue;
-                if (chk.subset(exc, caughtInTry)) {
-                    log.error(l.head.pos(),
-                              "except.already.caught", exc);
-                } else if (!chk.isUnchecked(l.head.pos(), exc) &&
-                           exc.tsym != syms.throwableType.tsym &&
-                           exc.tsym != syms.exceptionType.tsym &&
-                           !chk.intersects(exc, thrownInTry)) {
-                    log.error(l.head.pos(),
-                              "except.never.thrown.in.try", exc);
+                if (exc != syms.unknownType) {
+                    ctypes = ctypes.append(exc);
+                    if (types.isSameType(exc, syms.objectType))
+                        continue;
+                    if (chk.subset(exc, caughtInTry)) {
+                        log.error(l.head.pos(),
+                                  "except.already.caught", exc);
+                    } else if (!chk.isUnchecked(l.head.pos(), exc) &&
+                               exc.tsym != syms.throwableType.tsym &&
+                               exc.tsym != syms.exceptionType.tsym &&
+                               !chk.intersects(exc, thrownInTry)) {
+                        log.error(l.head.pos(),
+                                  "except.never.thrown.in.try", exc);
+                    }
+                    caughtInTry = chk.incl(exc, caughtInTry);
                 }
-                caughtInTry = chk.incl(exc, caughtInTry);
             }
             inits = initsTry.dup();
             uninits = uninitsTry.dup();

langtools/src/share/classes/com/sun/tools/javac/main/JavaCompiler.java

@@ -1144,6 +1144,11 @@ public class JavaCompiler implements ClassReader.SourceCompleter {
                                   env.toplevel.sourcefile);
         try {
             attr.attribClass(env.tree.pos(), env.enclClass.sym);
+            if (errorCount() > 0 && !shouldStop(CompileState.ATTR)) {
+                //if in fail-over mode, ensure that AST expression nodes
+                //are correctly initialized (e.g. they have a type/symbol)
+                attr.postAttr(env);
+            }
             compileStates.put(env, CompileState.ATTR);
         }
         finally {

langtools/test/tools/javac/failover/FailOver01.java

+/*
+  * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver01.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver01.java
+ */
+
+class Test { { x = "" } }

langtools/test/tools/javac/failover/FailOver01.out

+FailOver01.java:10:22: compiler.err.expected: ';'
+FailOver01.java:10:16: compiler.err.cant.resolve.location: kindname.variable, x, , , kindname.class, Test
+2 errors

langtools/test/tools/javac/failover/FailOver02.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver02.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver02.java
+ */
+
+class Test implements AutoCloseable {
+    void test() {
+        try(Test t = null) {}
+    }
+}

langtools/test/tools/javac/failover/FailOver02.out

+FailOver02.java:10:1: compiler.err.does.not.override.abstract: Test, close(), java.lang.AutoCloseable
+FailOver02.java:12:9: compiler.err.cant.resolve.location.args: kindname.method, close, , , kindname.class, Test
+2 errors

langtools/test/tools/javac/failover/FailOver03.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver03.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver03.java
+ */
+
+class Test extends Test {
+   Test i;
+}

langtools/test/tools/javac/failover/FailOver03.out

+FailOver03.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver04.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver04.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver04.java
+ */
+
+class Test {
+   { new Unknown() {}; }
+}

langtools/test/tools/javac/failover/FailOver04.out

+FailOver04.java:11:10: compiler.err.cant.resolve.location: kindname.class, Unknown, , , kindname.class, Test
+1 error

langtools/test/tools/javac/failover/FailOver05.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver05.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver05.java
+ */
+
+class Test extends Test {
+   { for ( Integer x : null) {} }
+}

langtools/test/tools/javac/failover/FailOver05.out

+FailOver05.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver06.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver06.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver06.java
+ */
+
+class Test extends Test {
+    Inference x = 1;
+    { if (x == 1) { } else { } }
+}

langtools/test/tools/javac/failover/FailOver06.out

+FailOver06.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver07.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver07.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver07.java
+ */
+
+class Test extends Test {
+    Integer x = 1;
+    { do {} while (x); }
+}

langtools/test/tools/javac/failover/FailOver07.out

+FailOver07.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver08.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver08.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver08.java
+ */
+
+class Test extends Test {
+    Integer x = 1;
+    { while (x) {}; }
+}

langtools/test/tools/javac/failover/FailOver08.out

+FailOver08.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver09.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver09.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver09.java
+ */
+
+class Test extends Test {
+    Integer x = 1;
+    { for (x = 0 ; x ; x++) {}; }
+}

langtools/test/tools/javac/failover/FailOver09.out

+FailOver09.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver10.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver10.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver10.java
+ */
+
+class Test extends Test {
+
+    boolean cond;
+
+    { Object o = null; }
+}

langtools/test/tools/javac/failover/FailOver10.out

+FailOver10.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver11.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver11.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver11.java
+ */
+
+class Test extends Test {
+
+    boolean cond;
+
+    void m(Object o) {}
+
+    { m(cond ? null : null); }
+}

langtools/test/tools/javac/failover/FailOver11.out

+FailOver11.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver12.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver12.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver12.java
+ */
+
+class Test extends Test {
+
+    Integer x = 1;
+
+    { try {} catch (Exception e) {} }
+}

langtools/test/tools/javac/failover/FailOver12.out

+FailOver12.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver13.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver13.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver13.java
+ */
+
+class Test extends Test {
+
+    Integer x = 1;
+
+    { x = (Object)o; }
+}

langtools/test/tools/javac/failover/FailOver13.out

+FailOver13.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error

langtools/test/tools/javac/failover/FailOver14.java

+/*
+ * @test /nodynamiccopyright/
+ * @bug 6970584
+ * @summary Flow.java should be more error-friendly
+ * @author mcimadamore
+ *
+ * @compile/fail/ref=FailOver14.out -XDrawDiagnostics -XDshouldStopPolicy=FLOW -XDdev FailOver14.java
+ */
+
+class Test extends Test  {
+
+   { for (Integer x : !x) { } }
+
+}

langtools/test/tools/javac/failover/FailOver14.out

+FailOver14.java:10:1: compiler.err.cyclic.inheritance: Test
+1 error