Please re-review 1 file: Re: 8027229 code review

[Karen Kinnear]

Bharadwaj,

Great suggestion - sorry I just pushed the change, so I will fix the extra space next time I am in there -
or one of us will.

Thank you for the review and for the test case.

thanks,
Karen

On Nov 13, 2013, at 10:32 AM, S. Bharadwaj Yadavalli wrote:

> Thanks for considering the test case and addressing it, Karen.
> 
> Changes look good to me.
> 
> You may want to consider removing the additional space between original_methods and -> in the hunk of defaultMethods.cpp patch.
> @@ -1074,11 +1081,13 @@
>    // Replace klass methods with new merged lists
>    klass->set_methods(merged_methods);
>    klass->set_initial_method_idnum(new_size);
>  
>    ClassLoaderData* cld = klass->class_loader_data();
> +  if (original_methods ->length() > 0) {
>    MetadataFactory::free_array(cld, original_methods);
> +  }
> 
> Sorry to be picky :-)
> 
> Thanks,
> 
> Bharadwaj
> 
> On 11/12/2013 10:27 PM, Karen Kinnear wrote:
>> Bharadwaj et al,
>> 
>> One more very short round of code reviews please folks.
>> 
>> Thank you for the testcase. I broke this with the fix for 8027304, so the code reviews for 8027229 are
>> still valid.
>> 
>> I did add the fix for this to the updated webrev below.
>> The only changes relative to the initial webrev are: 
>> 1. klassVtable.cpp - I removed the extra <CR> before the {
>> 
>> 2. defaultMethods.cpp
>>    - added the suggested comments, and removed extra space
>>    - code fix in lines: 403, 408, 411
>> 
>> All small tests have passed, and the longer ones are in progress.
>> Goal is to check this in tomorrow please.
>> 
>> thanks,
>> Karen
>> 
>> webrev: http://cr.openjdk.java.net/~acorn/8027229.2/webrev/
>> bug: http://bugs.openjdk.java.net/browse/JDK-8027229
>> 
>> Added support for default method inheritance logic for interfaces.
>> Removed interface methods from interface vtables.
>> Better cleanup of 8027304 as well.
>> 
>> specific tests:
>> 1. jdk jtreg: FDSeparateCompilation, DefaultMethodsTest
>>   including: testSuperConflict (fixed to match specification)
>> 2. vmtestbase defmeth 
>>   including: SuperCallTest:testSuperConflict (fixed to match specification)
>> 3. jtreg java.util.streams
>> 4. jck lang, vm
>> 5. nsk vm.quick, vm.mlvm
>> 6. invoke tests
>> 7. jtreg hotspot/test/runtime, hotspot/test/compiler/jsr292
>> 8. Bharadwaj's new test example
>> 
>> 
>> On Nov 12, 2013, at 5:47 PM, S. Bharadwaj Yadavalli wrote:
>> 
>>> Hi Karen,
>>> 
>>> I changed testSuperConflict_ICCE_or_AME_or_none/L/L.java as follows
>>> 
>>> interface L  { 
>>> 
>>>      //default int m() { return 101; }
>>>      abstract public int m();
>>> }
>>> 
>>> and ran sh run.sh to get
>>> 
>>> Exception in thread "main" java.lang.AbstractMethodError: J.m()I
>>>     at I.m(I.java:3)
>>>     at IV_C.m(IV_C.java:3)
>>>     at IV_C.main(IV_C.java:6)
>>> 
>>> Just to verify, here is the info about L.class
>>> 
>>> $ javap  cpath/L.class 
>>> Compiled from "L.java"
>>> interface L {
>>>   public abstract int m();
>>> }
>>> 
>>> I believe the change I made to L.java reflects the case where one of the methods is abstract and the other default.
>>> 
>>> Apologies if this (rightly or wrongly) delays your checkin.
>>> 
>>> Bharadwaj
>>> 
>> 
>