RFR 8167343: JShell: Completeness analysis infers an incomplete declaration as COMPLETE_WITH_SEMI, which is a first line of Allman style

Robert Field robert.field at oracle.com
Fri Oct 7 18:07:35 UTC 2016


On 10/07/16 08:45, ShinyaYoshida wrote:
> Hi all,
> Could you review this?
>
> webrev: http://cr.openjdk.java.net/~shinyafox/kulla/8167343/webrev.00/
> bugs: https://bugs.openjdk.java.net/browse/JDK-8167343
>
> Regards,
> shinyafox(Shinya Yoshida)

This looks good.

We need to keep in mind that the completion analysis is only a guess, 
and cannot always be correct -- we need to be careful not to infinitely 
chase perfection.
This case though is an important one -- good catch.

I have only very minor nits which you can choose to address, or not, 
without need for re-review:

169: The name is rather hard to read when used and doesn't match the 
tokens, at this level it is BRACES not LBRACE -- maybe XBRACESNEEDED

651-653:  Purely personal style.  Could be written:   expectBrace |= 
token.kind.expectBrace();

678: The style in this file would generally have the '?' and the ':' on 
their own lines.

Thumbs up to push!

Thanks,
Robert



More information about the kulla-dev mailing list