RFR; JDK-8235338: test jdk/jshell/CompletenessTest.java fails for records

[Robert Field]

+1


On 12/6/19 3:53 AM, Jan Lahoda wrote:
> On 05. 12. 19 20:23, Robert Field wrote:
>> Maybe it is just me, but adding support for records in JShell as well 
>> as seemingly correcting the compiler's processing of records under a 
>> bug for a JShell test failure seems inappropriate. Probably could be 
>> addressed by creating a RFE that covers it.
>
> The test failure is directly caused by the incorrect support for 
> records in JShell. Basically, the only thing in the patch that does 
> not have a fairly close connection to the test is the fix for Pretty.
>
> But I am OK with splitting the reports, so I've created:
> https://bugs.openjdk.java.net/browse/JDK-8235474
> and submitted:
> https://mail.openjdk.java.net/pipermail/compiler-dev/2019-December/014014.html 
>
>
> The patch for this bug then depends on JDK-8235474 and is here:
> http://cr.openjdk.java.net/~jlahoda/8235338/webrev.01/
>
> The CSR is no longer needed for this particular bug (although it is 
> still needed for JDK-8235474). Also moving compiler-dev on CC, since 
> this is no longer a javac patch.
>
> Jan
>
>>
>> I assume the CSR needs to be approved before this is pushed. The CSR 
>> has the same naming issue.
>>
>> Otherwise looks fine.
>>
>> -Robert
>>
>>
>> On 12/5/19 7:59 AM, Jan Lahoda wrote:
>>> Hi,
>>>
>>> This is a patch to improve the behavior of jshell w.r.t. records, 
>>> including fixing the recently problem-listed CompletenessTest.java.
>>>
>>> Notes on the patch:
>>> -added jdk.jshell.Snippet.SubKind.RECORD_SUBKIND, similar to 
>>> ENUM_SUBKIND. CSR for this change for review is here:
>>> https://bugs.openjdk.java.net/browse/JDK-8235421
>>> -moved setting the record final from JavacParsert to 
>>> Check.checkFlags (as jshell was struggling with the modifier being 
>>> set in parser, and checkFlags seems to be a better place anyway)
>>> -improved JavacParser.isRecordStart to take EOF into account - this 
>>> could be moved to jshell if too dangerous, but might be good for 
>>> javac for error recovery
>>> -Pretty (JCClassDecl.toString()) should now print records as records 
>>> instead of classes.
>>>
>>> Proposed patch:
>>> http://cr.openjdk.java.net/~jlahoda/8235338/webrev.00/
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8235338
>>>
>>> How does this look?
>>>
>>> Thanks,
>>>     Jan