<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html;
      charset=windows-1252">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Looks good, thanks!</p>
    <p>Maurizio<br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 02/02/18 05:41, Srinivas Dama wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:fbcdc511-6b86-4ac2-bbde-dd6f019b6ccb@default">
      <meta http-equiv="Content-Type" content="text/html;
        charset=windows-1252">
      <meta name="Generator" content="Microsoft Word 15 (filtered
        medium)">
      <style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;
        color:black;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:#0563C1;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:#954F72;
        text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
        {mso-style-name:msonormal;
        mso-margin-top-alt:auto;
        margin-right:0in;
        mso-margin-bottom-alt:auto;
        margin-left:0in;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;
        color:black;}
span.EmailStyle19
        {mso-style-type:personal;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
span.EmailStyle20
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
      <div class="WordSection1">
        <p class="MsoNormal"><span style="color:windowtext">Hi Maurizio,<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span style="color:windowtext">Thank you
            for the comments.<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="color:windowtext">Please
            review the revised webrev at <a
              href="http://cr.openjdk.java.net/%7Esdama/8152616/webrev.01/"
              moz-do-not-send="true">http://cr.openjdk.java.net/~sdama/8152616/webrev.01/</a>
            .<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
        <p><i><span style="color:windowtext">></span>* 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:<o:p></o:p></i></p>
        <p>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.<o:p></o:p></p>
        <p>><i>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?<o:p></o:p></i></p>
        <p>Yes – there is a bug hidden here.Fixed and modified the  test
          case as well.<o:p></o:p></p>
        <p class="MsoNormal"><span style="color:windowtext">Regards,<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="color:windowtext">Srinivas<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
        <div>
          <div style="border:none;border-top:solid #E1E1E1
            1.0pt;padding:3.0pt 0in 0in 0in">
            <p class="MsoNormal"><b><span style="color:windowtext">From:</span></b><span
                style="color:windowtext"> Maurizio Cimadamore <br>
                <b>Sent:</b> Thursday, January 25, 2018 4:48 PM<br>
                <b>To:</b> Srinivas Dama
                <a class="moz-txt-link-rfc2396E" href="mailto:srinivas.dama@oracle.com"><srinivas.dama@oracle.com></a>;
                <a class="moz-txt-link-abbreviated" href="mailto:compiler-dev@openjdk.java.net">compiler-dev@openjdk.java.net</a><br>
                <b>Subject:</b> Re: RFR: 8152616:
                com.sun.tools.javac.tree.Pretty generates nested
                comments for enum<o:p></o:p></span></p>
          </div>
        </div>
        <p class="MsoNormal"><o:p> </o:p></p>
        <p>Hi Srinivas,<br>
          two comments:<o:p></o:p></p>
        <p>* 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:<o:p></o:p></p>
        <p><a
href="https://docs.oracle.com/javase/specs/jls/se9/html/jls-8.html#jls-8.9.1"
            moz-do-not-send="true">https://docs.oracle.com/javase/specs/jls/se9/html/jls-8.html#jls-8.9.1</a><o:p></o:p></p>
        <p>* 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?<o:p></o:p></p>
        <p>Cheers<br>
          Maurizio<o:p></o:p></p>
        <p class="MsoNormal"><o:p> </o:p></p>
        <div>
          <p class="MsoNormal">On 25/01/18 08:19, Srinivas Dama wrote:<o:p></o:p></p>
        </div>
        <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
          <p class="MsoNormal">Hi,<o:p></o:p></p>
          <p class="MsoNormal"> <o:p></o:p></p>
          <p class="MsoNormal">Please review <a
              href="http://cr.openjdk.java.net/%7Esdama/8152616/webrev.00/"
              moz-do-not-send="true">http://cr.openjdk.java.net/~sdama/8152616/webrev.00/</a> 
            for <a
              href="https://bugs.openjdk.java.net/browse/JDK-8152616"
              moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8152616</a><o:p></o:p></p>
          <p class="MsoNormal"> <o:p></o:p></p>
          <p class="MsoNormal">Regards,<o:p></o:p></p>
          <p class="MsoNormal">Srinivas<o:p></o:p></p>
        </blockquote>
        <p class="MsoNormal"><o:p> </o:p></p>
      </div>
    </blockquote>
    <br>
  </body>
</html>