8188225: AST could be improved in presence of var types.
authorjlahoda
Fri, 06 Oct 2017 17:18:03 +0200
changeset 47318 423f5e46016e
parent 47317 62e749769358
child 47320 8b09673f7ede
8188225: AST could be improved in presence of var types. Summary: Adding a better start pos for vars; always adding a synthetic type tree to vars. Reviewed-by: mcimadamore
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java
src/jdk.jshell/share/classes/jdk/jshell/Eval.java
test/langtools/jdk/jshell/ErrorTranslationTest.java
test/langtools/tools/javac/tree/VarTree.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java	Fri Oct 06 08:37:08 2017 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java	Fri Oct 06 17:18:03 2017 +0200
@@ -1174,6 +1174,9 @@
                         v.type = chk.checkLocalVarType(tree, tree.init.type.baseType(), tree.name);
                     }
                 }
+                if (tree.isImplicitlyTyped()) {
+                    setSyntheticVariableType(tree, v.type);
+                }
             }
             result = tree.type = v.type;
         }
@@ -1348,11 +1351,7 @@
             }
             if (tree.var.isImplicitlyTyped()) {
                 Type inferredType = chk.checkLocalVarType(tree.var, elemtype, tree.var.name);
-                if (inferredType.isErroneous()) {
-                    tree.var.vartype = make.at(tree.var.vartype).Erroneous();
-                } else {
-                    tree.var.vartype = make.at(tree.var.vartype).Type(inferredType);
-                }
+                setSyntheticVariableType(tree.var, inferredType);
             }
             attribStat(tree.var, loopEnv);
             chk.checkType(tree.expr.pos(), elemtype, tree.var.sym.type);
@@ -2559,7 +2558,7 @@
                     Type argType = arityMismatch ?
                             syms.errType :
                             actuals.head;
-                    params.head.vartype = make.at(params.head).Type(argType);
+                    setSyntheticVariableType(params.head, argType);
                     params.head.sym = null;
                     actuals = actuals.isEmpty() ?
                             actuals :
@@ -4831,6 +4830,14 @@
         return types.capture(type);
     }
 
+    private void setSyntheticVariableType(JCVariableDecl tree, Type type) {
+        if (type.isErroneous()) {
+            tree.vartype = make.at(Position.NOPOS).Erroneous();
+        } else {
+            tree.vartype = make.at(Position.NOPOS).Type(type);
+        }
+    }
+
     public void validateTypeAnnotations(JCTree tree, boolean sigOnly) {
         tree.accept(new TypeAnnotationsValidator(sigOnly));
     }
@@ -5152,7 +5159,7 @@
                 that.sym.adr = 0;
             }
             if (that.vartype == null) {
-                that.vartype = make.Erroneous();
+                that.vartype = make.at(Position.NOPOS).Erroneous();
             }
             super.visitVarDef(that);
         }
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java	Fri Oct 06 08:37:08 2017 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java	Fri Oct 06 17:18:03 2017 +0200
@@ -3063,6 +3063,7 @@
         }
         else if (reqInit) syntaxError(token.pos, "expected", EQ);
         JCTree elemType = TreeInfo.innermostType(type, true);
+        int startPos = Position.NOPOS;
         if (allowLocalVariableTypeInference && elemType.hasTag(IDENT)) {
             Name typeName = ((JCIdent)elemType).name;
             if (isRestrictedLocalVarTypeName(typeName)) {
@@ -3070,6 +3071,9 @@
                     //error - 'var' and arrays
                     reportSyntaxError(pos, "var.not.allowed.array");
                 } else {
+                    startPos = TreeInfo.getStartPos(mods);
+                    if (startPos == Position.NOPOS)
+                        startPos = TreeInfo.getStartPos(type);
                     //implicit type
                     type = null;
                 }
@@ -3078,6 +3082,7 @@
         JCVariableDecl result =
             toP(F.at(pos).VarDef(mods, name, type, init));
         attach(result, dc);
+        result.startPos = startPos;
         return result;
     }
 
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java	Fri Oct 06 08:37:08 2017 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java	Fri Oct 06 17:18:03 2017 +0200
@@ -920,6 +920,8 @@
         public JCExpression init;
         /** symbol */
         public VarSymbol sym;
+        /** explicit start pos */
+        public int startPos = Position.NOPOS;
 
         protected JCVariableDecl(JCModifiers mods,
                          Name name,
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java	Fri Oct 06 08:37:08 2017 +0200
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java	Fri Oct 06 17:18:03 2017 +0200
@@ -457,9 +457,11 @@
             }
             case VARDEF: {
                 JCVariableDecl node = (JCVariableDecl)tree;
-                if (node.mods.pos != Position.NOPOS) {
+                if (node.startPos != Position.NOPOS) {
+                    return node.startPos;
+                } else if (node.mods.pos != Position.NOPOS) {
                     return node.mods.pos;
-                } else if (node.vartype == null) {
+                } else if (node.vartype == null || node.vartype.pos == Position.NOPOS) {
                     //if there's no type (partially typed lambda parameter)
                     //simply return node position
                     return node.pos;
--- a/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Fri Oct 06 08:37:08 2017 +0200
+++ b/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Fri Oct 06 17:18:03 2017 +0200
@@ -295,6 +295,10 @@
                 Range rtype = dis.treeToRange(baseType);
                 typeWrap = Wrap.rangeWrap(compileSource, rtype);
             } else {
+                AnalyzeTask at = trialCompile(Wrap.methodWrap(compileSource));
+                if (at.hasErrors()) {
+                    return compileFailResult(at, userSource, kindOfTree(unitTree));
+                }
                 Tree init = vt.getInitializer();
                 if (init != null) {
                     Range rinit = dis.treeToRange(init);
--- a/test/langtools/jdk/jshell/ErrorTranslationTest.java	Fri Oct 06 08:37:08 2017 +0200
+++ b/test/langtools/jdk/jshell/ErrorTranslationTest.java	Fri Oct 06 17:18:03 2017 +0200
@@ -23,6 +23,7 @@
 
 /*
  * @test
+ * @bug 8188225
  * @summary Tests for shell error translation
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
@@ -59,6 +60,13 @@
         );
     }
 
+    public void testlvtiErrors() {
+        test(
+                a -> assertDiagnostic(a, "var broken = () -> {};", newExpectedDiagnostic(0, 22, 0, -1, -1, Diagnostic.Kind.ERROR)),
+                a -> assertDiagnostic(a, "void t () { var broken = () -> {}; }", newExpectedDiagnostic(12, 34, 0, -1, -1, Diagnostic.Kind.ERROR))
+        );
+    }
+
     public void testWarnings() {
         List<ReplTest> list = new ArrayList<>();
         ExpectedDiagnostic[] diagnostics = new ExpectedDiagnostic[]{
@@ -117,19 +125,16 @@
             }
             String kind = getKind(expectedDiagnostic.getKind());
             assertEquals(lines[0], kind);
-            String source;
-            String markingLine;
-            switch (expectedDiagnostic.getKind()) {
-                case ERROR:
-                case WARNING:
-                    source = lines[2];
-                    markingLine = lines[3];
-                    break;
-                default:
-                    throw new AssertionError("Unsupported diagnostic kind: " + expectedDiagnostic.getKind());
+            boolean found = false;
+            for (int i = 0; i < lines.length; i++) {
+                if (lines[i].endsWith(expectedSource)) {
+                    assertEquals(lines[i + 1], expectedMarkingLine, "Input: " + expectedSource + ", marking line: ");
+                    found = true;
+                }
             }
-            assertTrue(source.endsWith(expectedSource), "Expected: " + expectedSource + ", found: " + source);
-            assertEquals(markingLine, expectedMarkingLine, "Input: " + expectedSource + ", marking line: ");
+            if (!found) {
+                throw new AssertionError("Did not find: " + expectedSource + " in: " + s);
+            }
         };
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/tree/VarTree.java	Fri Oct 06 17:18:03 2017 +0200
@@ -0,0 +1,133 @@
+/*
+ * Copyright (c) 2017, 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 8188225
+ * @summary Check that variables of type var have a consistent model
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ */
+
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.tools.javac.api.JavacTaskImpl;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.Arrays;
+import java.util.List;
+
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.ToolProvider;
+
+import com.sun.source.tree.VariableTree;
+import com.sun.source.util.TreeScanner;
+import com.sun.source.util.Trees;
+
+public class VarTree {
+    private final JavaCompiler tool = ToolProvider.getSystemJavaCompiler();
+
+    public static void main(String... args) throws Exception {
+        VarTree test = new VarTree();
+        test.run("|var testVar = 0;| ",
+                 "int testVar = 0");
+        test.run("|var testVar = 0;| undef undef;",
+                 "int testVar = 0");
+        test.run("|final var testVar = 0;| ",
+                 "final int testVar = 0");
+        test.run("for (|var testVar| : java.util.Arrays.asList(0, 1)) {}",
+                 "java.lang.Integer testVar");
+        test.run("for (|final var testVar| : java.util.Arrays.asList(0, 1)) {}",
+                 "final java.lang.Integer testVar");
+        test.run("java.util.function.Consumer<String> c = |testVar| -> {};",
+                 "java.lang.String testVar");
+        test.run("java.util.function.Consumer<String> c = (|testVar|) -> {};",
+                 "java.lang.String testVar");
+    }
+
+    void run(String code, String expected) throws IOException {
+        String[] parts = code.split("\\|");
+
+        if (parts.length != 3) {
+            throw new IllegalStateException("Incorrect number of markers.");
+        }
+
+        String prefix = "public class Test { void test() { ";
+        String src = prefix + parts[0] + parts[1] + parts[2] + " } }";
+
+        JavacTaskImpl ct = (JavacTaskImpl) tool.getTask(null, null, d -> {},
+                                                        List.of("--should-stop:at=FLOW"),
+                                                        null, Arrays.asList(new MyFileObject(src)));
+
+        Iterable<? extends CompilationUnitTree> units = ct.parse();
+        ct.analyze();
+
+        Trees trees = Trees.instance(ct);
+
+        for (CompilationUnitTree cut : units) {
+            new TreeScanner<Void, Void>() {
+                @Override
+                public Void visitVariable(VariableTree node, Void p) {
+                    if (node.getName().contentEquals("testVar")) {
+                        if (!expected.equals(node.toString())) {
+                            throw new AssertionError("Unexpected tree: " + node.toString());
+                        }
+
+                        int start = (int) trees.getSourcePositions().getStartPosition(cut, node);
+                        int end   = (int) trees.getSourcePositions().getEndPosition(cut, node);
+
+                        String snip = src.substring(start, end);
+
+                        if (start != prefix.length() + parts[0].length() || end != prefix.length() + parts[0].length() + parts[1].length()) {
+                            throw new AssertionError("Unexpected span: " + snip);
+                        }
+
+                        int typeStart = (int) trees.getSourcePositions().getStartPosition(cut, node.getType());
+                        int typeEnd   = (int) trees.getSourcePositions().getEndPosition(cut, node.getType());
+
+                        if (typeStart != (-1) && typeEnd != (-1)) {
+                            throw new AssertionError("Unexpected type position: " + typeStart + ", " + typeEnd);
+                        }
+                    }
+                    return super.visitVariable(node, p);
+                }
+
+            }.scan(cut, null);
+        }
+    }
+    class MyFileObject extends SimpleJavaFileObject {
+
+        private String text;
+
+        public MyFileObject(String text) {
+            super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
+            this.text = text;
+        }
+
+        @Override
+        public CharSequence getCharContent(boolean ignoreEncodingErrors) {
+            return text;
+        }
+    }
+}