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

[S. Bharadwaj Yadavalli]

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/ 
> <http://cr.openjdk.java.net/%7Eacorn/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
>>
>