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

[Jan Lahoda]

On 24. 08. 20 21:11, Vicente Romero wrote:
> 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?

It happens even without an annotation processor, but without an 
annotation processor, the errors are reported immediately and the 
exception is concealed from the user by:
https://github.com/openjdk/jdk/blob/4895a19dd195ff01cc67411e1f113ce77ddfe418/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Main.java#L348

When annotation processors are used, the errors are stashed on side, and 
then the parser crashes before the errors are actually reported, and 
hence the error is shown to the user.

Jan

> 
> 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
>