RFR: 8152616: com.sun.tools.javac.tree.Pretty generates nested comments for enum

[Maurizio Cimadamore]

Looks good, thanks!

Maurizio


On 02/02/18 05:41, Srinivas Dama wrote:
>
> Hi Maurizio,
>
> Thank you for the comments.
>
> Please review the revised webrev at 
> http://cr.openjdk.java.net/~sdama/8152616/webrev.01/ 
> <http://cr.openjdk.java.net/%7Esdama/8152616/webrev.01/> .
>
> />* why the logic for printing the enum constant constructor type 
> arguments? It is not possible to specify explicit constructor 
> arguments on enum constants. (Funnily, javac attempts to parse them, 
> but the attempt fails because the parser mode is wrong, so an >error 
> is generated). The spec has no place for them in the grammar:/
>
> Yes- no need to print enum constant constructor type arguments, 
> earlier I have taken this from existing  “visitNewClass” method which 
> is doing so but missed to remove that.
>
> >/I believe your end of block comment is misplaced, and it skips enum 
> constructor arguments - that is, I see that '*/' is put AFTER the 
> constructor arguments and before the '{' if any. Now, that doesn't 
> affect your test, because the constants in the test don't >have 
> constructor arguments, but I believe there's bug lurking in here?/
>
> Yes – there is a bug hidden here.Fixed and modified the  test case as 
> well.
>
> Regards,
>
> Srinivas
>
> *From:*Maurizio Cimadamore
> *Sent:* Thursday, January 25, 2018 4:48 PM
> *To:* Srinivas Dama <srinivas.dama at oracle.com>; 
> compiler-dev at openjdk.java.net
> *Subject:* Re: RFR: 8152616: com.sun.tools.javac.tree.Pretty generates 
> nested comments for enum
>
> Hi Srinivas,
> two comments:
>
> * why the logic for printing the enum constant constructor type 
> arguments? It is not possible to specify explicit constructor 
> arguments on enum constants. (Funnily, javac attempts to parse them, 
> but the attempt fails because the parser mode is wrong, so an error is 
> generated). The spec has no place for them in the grammar:
>
> https://docs.oracle.com/javase/specs/jls/se9/html/jls-8.html#jls-8.9.1
>
> * I believe your end of block comment is misplaced, and it skips enum 
> constructor arguments - that is, I see that '*/' is put AFTER the 
> constructor arguments and before the '{' if any. Now, that doesn't 
> affect your test, because the constants in the test don't have 
> constructor arguments, but I believe there's bug lurking in here?
>
> Cheers
> Maurizio
>
> On 25/01/18 08:19, Srinivas Dama wrote:
>
>     Hi,
>
>     Please review http://cr.openjdk.java.net/~sdama/8152616/webrev.00/
>     <http://cr.openjdk.java.net/%7Esdama/8152616/webrev.00/> for
>     https://bugs.openjdk.java.net/browse/JDK-8152616
>
>     Regards,
>
>     Srinivas
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180202/0a2309cd/attachment.html>