RFR: JDK-8237041: AssertionError while parsing unclosed class declarations

[Vicente Romero]

Hi Jan,

I'm curious that the user commented that the issue was only reproducible 
when APs were present. Wouldn't that mean that there is some data that 
is not being cleaned up after AP iterations?

Thanks,
Vicente

On 8/20/20 8:21 AM, Jan Lahoda wrote:
> Hi,
>
> Consider a simplified testcase from the report (the code is obviously 
> erroneous):
> ---
> class T {
>      String.class,
>      String.class,
>      //String.class, repeated many times
> }
>
> This crashes the parser in a safety code that is intended to prevent 
> infinite loops in the parser. This code crashes the parser when there 
> is too many errors reported on the same point in the source code.
>
> There are two somewhat independent problematic parts in the code:
> -first, consider code like this:
> ---
> class T {
>     class C1 {
>          class C2 {
>          //many more nested classes
> //but no closing '}'
> ---
>
> This is triggering the safety code, as it reports an "premature EOF" 
> error for each of the classes, when it cannot find the closing brace. 
> The proposed fix for this is to disable the safety code when the token 
> that is being handled is an EOF token. (An alternative would be to 
> remove the safety check altogether.)
>
> -for code like this:
> ---
> class T {
>      class
>      class
>      class
> }
> ---
>
> the parser will parse this code like:
> ---
> class T {
>      class {
>          class {
>              class {
> ...
> ---
>
> I.e. the AST nodes for the classes will be nested. That does not seem 
> ideal, as there are no opening braces for the classes. It seems it 
> would be better to parse like:
> ---
> class T {
>      class {}
>      class {}
>      class {}
> ---
>
> The second proposed change in JavacParser attempts to do that, i.e. 
> rather return an empty "class" body than continue parsing the class 
> content if there is no opening brace. This required some tweaks to 
> expected outputs for some tests.
>
> The proposed patch is here:
> http://cr.openjdk.java.net/~jlahoda/8237041/webrev.00/index.html
>
> JBS:
> https://bugs.openjdk.java.net/browse/JDK-8237041
>
> How does this look?
>
> Thanks,
>      Jan