RFR (M) JDK-8033150 (#2): invokestatic: IncompatibleClassChangeError trying to invoke static method from a parent in presence of conflicting defaults

[Lois Foltan]

On 4/11/2014 12:58 PM, Karen Kinnear wrote:
> Lois,
>
> I like the enum change. I agree with Coleen's suggestion of leaving 
> out the vmstructs change.
>
> I had one question - instanceKlass.cll uncached_lookup_method -
> would it make sense to have a local holding the mode rather than 
> overwriting the input argument of
> mode? Wouldn't that be better as a const input argument?

Hi Karen,
Thanks for the review!  Glad the enum change seems to be well liked.  I 
have completed removing the vmStructs change.  I have also taken your 
suggestion and implemented a local variable within 
InstanceKlass::uncached_lookup_method() to contain the incoming 
MethodLookupMode parameter.  So far my sanity testing on these minor 
changes is going well.

In case anyone wants to view the updated webrev, it is at 
http://cr.openjdk.java.net/~lfoltan/bug_jdk8033150.2/

Lois

>
> thanks,
> Karen
>
> On Apr 11, 2014, at 9:04 AM, Lois Foltan wrote:
>
>>
>> Please review the updated fix, webrev at:
>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8033150.1/
>>
>> This includes Keith's suggestion below.  All testing has been rerun 
>> successfully.
>>
>> Thank you,
>> Lois
>>
>>
>> On 3/31/2014 3:51 PM, Lois Foltan wrote:
>>>
>>> On 3/31/2014 2:08 PM, Keith McGuigan wrote:
>>>> Hi Lois,
>>>>
>>>> I think that looks good.  I like it much better than doing the 
>>>> static method check in the default method processing.
>>>> 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).
>>>>
>>>> So instead of:
>>>>    uncached_lookup_method(field_name, field_sig, false);
>>>> It'd be:
>>>>   uncached_lookup_method(field_name, field_sig, NORMAL); or
>>>>   uncached_lookup_method(field_name, field_sig, IGNORE_OVERPASS);
>>>>
>>>> (or something like that -- I'm no good at names).
>>>
>>> Thank you Keith.  Good suggestion.  I will implement and repost an 
>>> updated webrev for review.
>>> Lois
>>>
>>>>
>>>> --
>>>> - Keith
>>>>
>>>>
>>>> On Mon, Mar 31, 2014 at 1:25 PM, Lois Foltan 
>>>> <lois.foltan at oracle.com <mailto:lois.foltan at oracle.com>> wrote:
>>>>
>>>>     Hi,
>>>>
>>>>     Please review the following fix:
>>>>
>>>>     Webrev:
>>>>     http://cr.openjdk.java.net/~lfoltan/bug_jdk8033150/
>>>>     <http://cr.openjdk.java.net/%7Elfoltan/bug_jdk8033150/>
>>>>
>>>>     Bug: invokestatic: IncompatibleClassChangeError trying to
>>>>     invoke static method from a parent in presence of conflicting
>>>>     defaults
>>>>     https://bugs.openjdk.java.net/browse/JDK-8033150
>>>>
>>>>     Summary of fix:
>>>>     During default method processing, determine_target(), is
>>>>     responsible for making decisions on whether
>>>>     to create and add an overpass method to the current class for
>>>>     issues that have been encountered during the walk
>>>>     of the default methods.  The routine
>>>>     defaultMethods.cpp/has_matching_static() helped determine this
>>>>     decision by looking within the current class for a static
>>>>     method that should be preferred during method
>>>>     resolution over an overpass method being created.  However,
>>>>     has_matching_static() did not continue to
>>>>     look for a static method in current class' superclasses which
>>>>     JDK-8033150 exposed.
>>>>
>>>>     After looking at the code more closely, the has
>>>>     _matching_static() concept is being moved out out of default
>>>>     method processing and into method resolution processing.  The
>>>>     primary reason for this is to allow
>>>>     default method processing to match method selection where
>>>>     statics are and should be ignored.   According
>>>>     to JVMS, static methods should only be preferred over an
>>>>     overpass method at method and interface
>>>>     method resolution time.  To enable method resolution to
>>>>     potentially find a static method over an overpass
>>>>     method, a new parameter "ignore_overpass" was added to
>>>>     InstanceKlass::uncached_lookup_method().
>>>>     It has the affect of indicating to find_method_index() to
>>>>     ignore overpass methods and continue the search
>>>>     in case a static method of the same name and signature is
>>>>     present in the current class or its superclasses.
>>>>
>>>>     Tests:
>>>>         - jtreg hotspot/test/*, JDK java.lang & java.util,
>>>>     vm.quick.testlist, JCK lang & vm, defmeth tests
>>>>     - Test will be added to the defmeth test system
>>>>
>>>>     Thank you,
>>>>     Lois
>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140411/709458c0/attachment.html>