RFR: 8332230: jshell throws AssertionError when processing annotations

[Adam Sotona]

On Thu, 23 May 2024 15:55:01 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Passing snippet `k=aa:a. at a` to JShell crashes it:
> 
> jshell> k=aa:a. at a
> ...
> Caused by: java.lang.AssertionError
>         at jdk.compiler/com.sun.tools.javac.util.Assert.error(Assert.java:155)
>         at jdk.compiler/com.sun.tools.javac.util.Assert.checkNonNull(Assert.java:62)
>         at jdk.compiler/com.sun.tools.javac.comp.Annotate.fromAnnotations(Annotate.java:167)
>         at jdk.compiler/com.sun.tools.javac.comp.Annotate.lambda$annotateTypeSecondStage$5(Annotate.java:1059)
>         at jdk.compiler/com.sun.tools.javac.comp.Annotate.flush(Annotate.java:194)
>         at jdk.compiler/com.sun.tools.javac.comp.Annotate.unblockAnnotations(Annotate.java:144)
> 
> 
> There are two places where relevant work for type annotations is scheduled:
> - Attr.visitMethodDef will schedule and perform first stage of type annotation processing before processing the method's body: https://github.com/openjdk/jdk/blob/90758f6735620776fcb60da9e0e2c91a4f53aaf1/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java#L1215
> - Attr.visitAnnotatedType will schedule the second phase of type annotation processing: https://github.com/openjdk/jdk/blob/90758f6735620776fcb60da9e0e2c91a4f53aaf1/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java#L5224
> 
> There are two problems here:
> a) it may happen that `visitAnnotatedType` schedules the second phase for the given type, but noone calls `Annotate.flush()` anymore before javac finishes. `flush` is in `Attr` called at the beginning of processing of classes, methods, many, but not all variables, etc., but if there's none of them in the source after the task has been scheduled, the task will remain in the queue. This has twofold effect: 1) the model for the type is slightly broken (if we ask of the type annotations on the type, we won't get the annotation); 2) since JShell is using `JavacTaskPool`, and reuses javac tasks, the pending type annotation task may remain in the queue, and be invoked when doing reusing the `JavacTask` for unrelated processing. My proposal here is to call `Annotate.flush()` after "top-level" declarations, to make sure all annotation tasks are always finished.
> 
> b) `Attr` visits even erroneous subtrees, so annotated types inside erroneous subtrees will have the second stage scheduled; but the first stage of type annotation processing is not looking inside erroneous subtrees, so the first stage is not done for annotated types inside these subtrees. And the second stage then crashes. My proposal is to let the first...

Looks good to me.

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

Marked as reviewed by asotona (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19371#pullrequestreview-2082608446