<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <div class="moz-cite-prefix">On 4/11/14, 10:09 AM, Lois Foltan
      wrote:<br>
    </div>
    <blockquote cite="mid:5347F7B2.5030704@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      <br>
      <div class="moz-cite-prefix">On 4/11/2014 10:02 AM, Coleen
        Phillimore wrote:<br>
      </div>
      <blockquote cite="mid:5347F5FB.20309@oracle.com" type="cite">
        <meta content="text/html; charset=ISO-8859-1"
          http-equiv="Content-Type">
        <br>
        This code looks good.  The MethodLookupMode enum is a nice
        improvement.  It's a good warning of the complexity of this
        code.<br>
        Did something part of the build complain about MethodLookupMode
        not being in vmStructs?<br>
        I'd prefer the serviceability agent not be tempted to duplicate
        the method lookup code in the JVM, so not have this change.<br>
      </blockquote>
      <br>
      Hi Coleen,<br>
      <br>
      Thanks for the review.  The vmStructs.cpp change was a cautionary
      move on my part to include the new enum MethodLookupMode.  The
      build did not complain without it.  From your comments sounds like
      it would be better to leave this change to vmStructs.cpp out.  I
      can do that and run through some minor testing.  Are you okay with
      me going forward with this change and not sending it out for
      another round of review?<br>
    </blockquote>
    <br>
    Yes, I'm fine with the change if you revert the change to
    vmStructs.   I don't need another round of review.  If vmStructs
    actually needed this new type, then there'd be changes in the
    serviceability agent required.  I'm glad that isn't the case.<br>
    <br>
    Thanks!<br>
    Coleen<br>
    <br>
    <blockquote cite="mid:5347F7B2.5030704@oracle.com" type="cite"> <br>
      Lois<br>
      <br>
      <br>
      <blockquote cite="mid:5347F5FB.20309@oracle.com" type="cite"> <br>
        Thanks,<br>
        Coleen<br>
        <br>
        <div class="moz-cite-prefix">On 4/11/14, 9:04 AM, Lois Foltan
          wrote:<br>
        </div>
        <blockquote cite="mid:5347E84D.60304@oracle.com" type="cite">
          <meta content="text/html; charset=ISO-8859-1"
            http-equiv="Content-Type">
          <br>
          Please review the updated fix, webrev at:<br>
              <a moz-do-not-send="true" class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Elfoltan/bug_jdk8033150.1/">http://cr.openjdk.java.net/~lfoltan/bug_jdk8033150.1/</a><br>
          <br>
          This includes Keith's suggestion below.  All testing has been
          rerun successfully.<br>
          <br>
          Thank you,<br>
          Lois<br>
          <br>
          <br>
          <div class="moz-cite-prefix">On 3/31/2014 3:51 PM, Lois Foltan
            wrote:<br>
          </div>
          <blockquote cite="mid:5339C72A.8040405@oracle.com" type="cite">
            <meta content="text/html; charset=ISO-8859-1"
              http-equiv="Content-Type">
            <br>
            <div class="moz-cite-prefix">On 3/31/2014 2:08 PM, Keith
              McGuigan wrote:<br>
            </div>
            <blockquote
cite="mid:CADNkbOHpMzCTt8V+OV01kZ2RLFKGkGD5++SAKB092=06y0ykTA@mail.gmail.com"
              type="cite">
              <div dir="ltr">Hi Lois, 
                <div><br>
                </div>
                <div>I think that looks good.  I like it much better
                  than doing the static method check in the default
                  method processing.</div>
                <div>My only suggestion is that it would be nice to
                  encode new parameter to uncached_lookup_method to be
                  some sort of enum instead of a boolean, so that it is
                  obvious from the call-site what the behavior should be
                  (having just "false" in the parameter list requires
                  you to reference back to the declaration to figure out
                  what that "false" means).</div>
                <div><br>
                </div>
                So instead of: <br>
                   uncached_lookup_method(field_name, field_sig, false);
                <div>It'd be:</div>
                <div>  uncached_lookup_method(field_name, field_sig,
                  NORMAL); or </div>
                <div>  uncached_lookup_method(field_name, field_sig,
                  IGNORE_OVERPASS);   </div>
                <div><br>
                </div>
                <div>(or something like that -- I'm no good at names).</div>
              </div>
            </blockquote>
            <br>
            Thank you Keith.  Good suggestion.  I will implement and
            repost an updated webrev for review.<br>
            Lois<br>
            <br>
            <blockquote
cite="mid:CADNkbOHpMzCTt8V+OV01kZ2RLFKGkGD5++SAKB092=06y0ykTA@mail.gmail.com"
              type="cite">
              <div dir="ltr">
                <div><br>
                </div>
                <div>--</div>
                <div>- Keith </div>
              </div>
              <div class="gmail_extra"><br>
                <br>
                <div class="gmail_quote">On Mon, Mar 31, 2014 at 1:25
                  PM, Lois Foltan <span dir="ltr"><<a
                      moz-do-not-send="true"
                      href="mailto:lois.foltan@oracle.com"
                      target="_blank">lois.foltan@oracle.com</a>></span>
                  wrote:<br>
                  <blockquote class="gmail_quote" style="margin:0 0 0
                    .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
                    <br>
                    Please review the following fix:<br>
                    <br>
                    Webrev:<br>
                        <a moz-do-not-send="true"
                      href="http://cr.openjdk.java.net/%7Elfoltan/bug_jdk8033150/"
                      target="_blank">http://cr.openjdk.java.net/~lfoltan/bug_jdk8033150/</a><br>
                    <br>
                    Bug: invokestatic: IncompatibleClassChangeError
                    trying to invoke static method from a parent in
                    presence of conflicting defaults<br>
                    <a moz-do-not-send="true"
                      href="https://bugs.openjdk.java.net/browse/JDK-8033150"
                      target="_blank">https://bugs.openjdk.java.net/browse/JDK-8033150</a><br>
                    <br>
                    Summary of fix:<br>
                    During default method processing,
                    determine_target(), is responsible for making
                    decisions on whether<br>
                    to create and add an overpass method to the current
                    class for issues that have been encountered during
                    the walk<br>
                    of the default methods.  The routine
                    defaultMethods.cpp/has_matching_static() helped
                    determine this<br>
                    decision by looking within the current class for a
                    static method that should be preferred during method<br>
                    resolution over an overpass method being created.
                     However, has_matching_static() did not continue to<br>
                    look for a static method in current class'
                    superclasses which JDK-8033150 exposed.<br>
                    <br>
                    After looking at the code more closely, the has
                    _matching_static() concept is being moved out out of
                    default<br>
                    method processing and into method resolution
                    processing.  The primary reason for this is to allow<br>
                    default method processing to match method selection
                    where statics are and should be ignored.   According<br>
                    to JVMS, static methods should only be preferred
                    over an overpass method at method and interface<br>
                    method resolution time.  To enable method resolution
                    to potentially find a static method over an overpass<br>
                    method, a new parameter "ignore_overpass" was added
                    to InstanceKlass::uncached_lookup_method().<br>
                    It has the affect of indicating to
                    find_method_index() to ignore overpass methods and
                    continue the search<br>
                    in case a static method of the same name and
                    signature is present in the current class or its
                    superclasses.<br>
                    <br>
                    Tests:<br>
                        - jtreg hotspot/test/*, JDK java.lang &
                    java.util, vm.quick.testlist, JCK lang & vm,
                    defmeth tests<br>
                    - Test will be added to the defmeth test system<br>
                    <br>
                    Thank you,<br>
                    Lois<br>
                  </blockquote>
                </div>
                <br>
              </div>
            </blockquote>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>