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

[Vicente Romero]

I see, thanks for the clarification, looks good to me,

Vicente

On 8/25/20 3:57 AM, Jan Lahoda wrote:
> 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
>>