<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <br>
    <br>
    <div class="moz-cite-prefix">On 02/03/2018 12:37 AM, Liam
      Miller-Cushon wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAL4QsgssKbVctBNnFWBLo2rkRqjALj3E2CWRxJRDC-suqKao9A@mail.gmail.com">
      <div dir="ltr">Hi <span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Vicente,
          thanks for the review!</span>
        <div><span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br>
          </span></div>
        <div><span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">I
            inlined the golden files into the test class. I don't think
            inlining Lib.java works with the current approach of using
            `@compile --release 7/8` and reflection to access the
            parameter names, because the reflective APIs were added in
            8. I could rewrite the test to use class reading instead of
            reflection if you prefer.</span></div>
      </div>
    </blockquote>
    <br>
    yes I was thinking about class reading but I'm OK with the current
    version of the patch<br>
    <br>
    <blockquote type="cite"
cite="mid:CAL4QsgssKbVctBNnFWBLo2rkRqjALj3E2CWRxJRDC-suqKao9A@mail.gmail.com">
      <div dir="ltr">
        <div>
          <div><br>
          </div>
          <div>Here's the updated webrev: <span style="font-size:12.8px"><a
href="http://cr.openjdk.java.net/%7Ecushon/8190452/webrev.01/"
                class="cremed" moz-do-not-send="true">http://cr.openjdk.java.net/~cushon/8190452/webrev.01/</a></span></div>
        </div>
        <div><br>
        </div>
        <div>> <span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Regarding
            your questions about what to do with the -Xlint:X options. I
            don't have any opinion on one way or the other, is there any
            reason to change them?</span></div>
        <div><span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br>
          </span></div>
        <div><span style="font-size:12.8px">I thought it was potentially
            surprising that -parameters will now be silently ignored for
            Java 7 and earlier language levels. Warning about that flag
            combination would make the new behaviour more discoverable.</span><br>
        </div>
        <div><span style="font-size:12.8px"><br>
          </span></div>
        <div><span style="font-size:12.8px">Since the bug shipped in 9
            and 10 there are some <v52 class files with
            MethodParameters in the wild, and I've seen cases where it
            broke builds using -Xlint:classfile and -Werror.</span></div>
        <div><span style="font-size:12.8px"><br>
          </span></div>
        <div><span style="font-size:12.8px"><span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">I
              don't think either of those are common problems. If you
              think we should leave the -Xlint handling as-is that
              sounds good to me.</span><br>
          </span></div>
      </div>
    </blockquote>
    <br>
    as discussed off-line with Jon, yes please add the warning, but feel
    free to do it as part of this bug or in a different one.<br>
    <br>
    Vicente<br>
    <br>
    <blockquote type="cite"
cite="mid:CAL4QsgssKbVctBNnFWBLo2rkRqjALj3E2CWRxJRDC-suqKao9A@mail.gmail.com">
      <div class="gmail_extra"><br>
        <div class="gmail_quote">On Fri, Feb 2, 2018 at 6:17 AM, Vicente
          Romero <span dir="ltr"><<a
              href="mailto:vicente.romero@oracle.com" target="_blank"
              moz-do-not-send="true">vicente.romero@oracle.com</a>></span>
          wrote:<br>
          <blockquote class="gmail_quote" style="margin:0 0 0
            .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <div text="#000000" bgcolor="#FFFFFF"> Hi Liam,<br>
              <br>
              The fix looks OK, regarding the test, I don't see the need
              for the two golden files as they can be constants in the
              test per se. In addition, the whole test could be self
              contained in only one class that compiles the Lib.java
              source. Regarding your questions about what to do with the
              -Xlint:X options. I don't have any opinion on one way or
              the other, is there any reason to change them?<br>
              <br>
              Thanks,<br>
              Vicente
              <div>
                <div class="h5"><br>
                  <br>
                  <div class="m_-3429153412801308596moz-cite-prefix">On
                    02/01/2018 01:56 PM, Liam Miller-Cushon wrote:<br>
                  </div>
                  <blockquote type="cite">
                    <div dir="ltr">Bump. I'm happy to implement either
                      of the alternatives I mentioned, but was hoping to
                      get feedback on the approach first.</div>
                    <div class="gmail_extra"><br>
                      <div class="gmail_quote">On Wed, Jan 17, 2018 at
                        11:51 AM, Liam Miller-Cushon <span dir="ltr"><<a
                            href="mailto:cushon@google.com"
                            target="_blank" moz-do-not-send="true">cushon@google.com</a>></span>
                        wrote:<br>
                        <blockquote class="gmail_quote" style="margin:0
                          0 0 .8ex;border-left:1px #ccc
                          solid;padding-left:1ex">
                          <div dir="ltr">
                            <div>Please review a fix for <span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">JDK-8190452.
                                The change causes javac to not emit </span>MethodParameters
                              attributes when targeting v51 class files.</div>
                            <div><span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br>
                              </span></div>
                            <div><span
style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">The
                                change implements the suggestion from
                                this thread:</span><br>
                            </div>
                            <div><a
href="http://mail.openjdk.java.net/pipermail/compiler-dev/2018-January/011579.html"
                                target="_blank" moz-do-not-send="true">http://mail.openjdk.java.net/p<wbr>ipermail/compiler-dev/2018-Jan<wbr>uary/011579.html</a><br>
                            </div>
                            <div><br>
                            </div>
                            <div>There are two related changes that may
                              be worth considering:</div>
                            <div>* now that -parameters will be ignored
                              when compiling with --release < 8,
                              should this combination of flags result in
                              a warning if -Xlint:options is enabled?</div>
                            <div>* since this wasn't fixed in JDK 9,
                              there are v51 class files in the wild that
                              contain unexpected MethodParameters
                              attributes. Should -Xlint:classfile be
                              relaxed to avoid warning on those?</div>
                            <div><br>
                            </div>
                            <div>bug: <a
                                href="https://bugs.openjdk.java.net/browse/JDK-8190452"
                                target="_blank" moz-do-not-send="true">https://bugs.openjdk.java<wbr>.net/browse/JDK-8190452</a><br>
                            </div>
                            <div>webrev: <a
                                href="http://cr.openjdk.java.net/%7Ecushon/8190452/webrev.00/"
                                target="_blank" moz-do-not-send="true">http://cr.openjdk.java<wbr>.net/~cushon/8190452/webrev.<wbr>00/</a></div>
                          </div>
                        </blockquote>
                      </div>
                      <br>
                    </div>
                  </blockquote>
                  <br>
                </div>
              </div>
            </div>
          </blockquote>
        </div>
        <br>
      </div>
    </blockquote>
    <br>
  </body>
</html>