RFR: 8292625: jshell crash on "var a = a" [v2]

[Bo Zhang]

On Mon, 28 Nov 2022 09:37:21 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> Good catch, but I would personally prefer if `attribExpr` would set the type (as it does under normal circumstances), presumably here?
>> 
>> https://github.com/openjdk/jdk/blob/5c3345404d850cf01d9629b48015f1783a32bfc0/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java#L4651
>> 
>> Also, might be nice to have a (core javac) test that would not rely on JShell, if possible.
>
>> @lahodaj Done moving the assignment. However, this was not a problem with `javac`, probably due to different code path:
>> 
>> ```
>> # zhb @ zhb in ~/Projects/jdk on git:jdk-8292625 x [7:03:45] C:1
>> $ cat Test.java
>> public class Test {
>>   public static void main() {
>>     var a = a;
>>   }
>> }
>> 
>> # zhb @ zhb in ~/Projects/jdk on git:jdk-8292625 x [7:03:48]
>> $ /Library/Java/JavaVirtualMachines/temurin-19.jdk/Contents/Home/bin/javac Test.java
>> Test.java:3: error: cannot infer type for local variable a
>>     var a = a;
>>         ^
>>   (cannot use 'var' on self-referencing variable)
>> 1 error
>> printing javac parameters to: /Users/zhb/Projects/jdk/javac.20221122_070353.args
>> ```
>> 
>> That's why I put the test into jshell tests.
> 
> Thanks for moving the assignment. For tests, the test in JShell is fine, but there are other ways to the attributes in addition to that, like actually checking the type is assigned in the AST:
> 
> diff --git a/test/langtools/tools/javac/attr/AttrRecoveryTest.java b/test/langtools/tools/javac/attr/AttrRecoveryTest.java
> index b364cc28001..f5589a8093f 100644
> --- a/test/langtools/tools/javac/attr/AttrRecoveryTest.java
> +++ b/test/langtools/tools/javac/attr/AttrRecoveryTest.java
> @@ -36,6 +36,7 @@
>  import com.sun.source.tree.AnnotationTree;
>  import com.sun.source.tree.CompilationUnitTree;
>  import com.sun.source.tree.ErroneousTree;
> +import com.sun.source.tree.VariableTree;
>  import com.sun.source.util.TaskEvent;
>  import com.sun.source.util.TaskListener;
>  import com.sun.source.util.TreePath;
> @@ -45,7 +46,9 @@ import java.nio.file.Files;
>  import java.nio.file.Path;
>  import java.nio.file.Paths;
>  import java.util.List;
> +import java.util.concurrent.atomic.AtomicInteger;
>  import javax.lang.model.element.Element;
> +import javax.lang.model.type.TypeMirror;
>  
>  import toolbox.TestRunner;
>  import toolbox.JavacTask;
> @@ -162,4 +165,61 @@ public class AttrRecoveryTest extends TestRunner {
>              throw new AssertionError();
>          }
>      }
> +
> +    @Test
> +    public void testVarAssignment2Self(Path base) throws Exception {
> +        Path current = base;
> +        Path src = current.resolve("src");
> +        Path classes = current.resolve("classes");
> +        tb.writeJavaFiles(src,
> +                          """
> +                          public class Test {
> +                              void t() {
> +                                  var v = v;
> +                              }
> +                          }
> +                          """);
> +
> +        Files.createDirectories(classes);
> +
> +        AtomicInteger seenVariables = new AtomicInteger();
> +        TreePathScanner<Void, Trees> checkTypes = new TreePathScanner<>() {
> +            @Override
> +            public Void visitVariable(VariableTree node, Trees trees) {
> +                if (node.getName().contentEquals("v")) {
> +                    TypeMirror type = trees.getTypeMirror(getCurrentPath());
> +                    if (type == null) {
> +                        throw new AssertionError("Unexpected null type!");
> +                    }
> +                    seenVariables.incrementAndGet();
> +                }
> +                return super.visitVariable(node, trees);
> +            }
> +        };
> +
> +        new JavacTask(tb)
> +            .options("-XDrawDiagnostics")
> +            .outdir(classes)
> +            .files(tb.findJavaFiles(src))
> +            .callback(t -> {
> +                t.addTaskListener(new TaskListener() {
> +                    CompilationUnitTree parsed;
> +                    @Override
> +                    public void finished(TaskEvent e) {
> +                        switch (e.getKind()) {
> +                            case PARSE -> parsed = e.getCompilationUnit();
> +                            case COMPILATION ->
> +                                checkTypes.scan(parsed, Trees.instance(t));
> +                        }
> +                    }
> +                });
> +            })
> +            .run(Task.Expect.FAIL)
> +            .writeAll()
> +            .getOutputLines(Task.OutputKind.DIRECT);
> +
> +        if (seenVariables.get() != 1) {
> +            throw new AssertionError("Didn't see enough variables: " + seenVariables);
> +        }
> +    }
>  }
> 
> 
> I think the advantage of having the test in the javac tests (in addition to the JShell test) is that if someone modifies javac, and only runs javac tests, it would still fail.

Thanks @lahodaj . Can you please sponsor this one?

-------------

PR: https://git.openjdk.org/jdk/pull/11263