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

[Srinivas Dama]

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 HYPERLINK "http://cr.openjdk.java.net/%7Esdama/8152616/webrev.00/"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/20180201/f28e226f/attachment-0001.html>