RFR: 8152616: com.sun.tools.javac.tree.Pretty generates nested comments for enum
Srinivas Dama
srinivas.dama at oracle.com
Mon Feb 5 17:04:07 UTC 2018
Sorry, no more reviews are required.
Regards,
Srinivas
----- Original Message -----
From: srinivas.dama at oracle.com
To: compiler-dev at openjdk.java.net
Sent: Monday, February 5, 2018 3:09:44 PM GMT +05:30 Chennai, Kolkata, Mumbai, New Delhi
Subject: Re: RFR: 8152616: com.sun.tools.javac.tree.Pretty generates nested comments for enum
Hi,
May I have second review for this to push.
Regards,
Srinivas
----- Original Message -----
From: maurizio.cimadamore at oracle.com
To: srinivas.dama at oracle.com, compiler-dev at openjdk.java.net
Sent: Friday, February 2, 2018 2:37:00 PM GMT +05:30 Chennai, Kolkata, Mumbai, New Delhi
Subject: Re: RFR: 8152616: com.sun.tools.javac.tree.Pretty generates nested comments for enum
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/ .
> * 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/ 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/20180205/4b41f794/attachment.html>
More information about the compiler-dev
mailing list