RFR soon: Lambda: 8011311: support for private interface methods
Karen Kinnear
karen.kinnear at oracle.com
Mon Sep 30 18:46:03 PDT 2013
Coleen,
Thank you for the review. Love your suggestion - added the following:
// b) check if the class of the resolved_klass is a superclass
// (not supertype in order to exclude interface classes) of the current class.
// This check is not performed for super.invoke for interface methods
// in super interfaces.
I too wondered about duplicates in the tracing except I also couldn't find the simple refactoring since we don't have all
the same args at each time. I will think about that for future though.
thanks very much,
Karen
On Sep 30, 2013, at 3:49 PM, Coleen Phillimore wrote:
>
> Karen,
> The code looks good, I only had a couple of minor comments.
>
> This comment is a bit subtle and contradicts itself (unless you see what the change is, which you won't if you're not looking at the webrev).
>
> // b) check if the class of the requested methodref is a superclass of the current class (superclass relation is not reflexive!)
> 897 // note: JDK8 change, this used to be class of the resolved method
> 898 // is a superclass of the current class
> 899 current_klass->is_subclass_of(resolved_klass()) &&
> 900 current_klass() != resolved_klass() &&
>
> How about:
> b) check if the resolved class is a superclass (not supertype because this excludes interface classes) of the current classes. This check is not performed for super.invoke for interface methods in super interfaces.
>
> Or something factual which might not be my suggested sentence. It seems sort of obvious that superclass relation isn't reflexive!
>
> I like the new tracing code. There appears to be some duplication but I can't suggest a simple refactoring so it's fine.
>
> Coleen
>
>
> On 09/27/2013 08:36 PM, Karen Kinnear wrote:
>>
>> Please review soon - the goal is to check this in Monday or Tuesday.
>>
>> http://cr.openjdk.java.net/~acorn/8011311.8014013.01/webrev/ <http://cr.openjdk.java.net/%7Eacorn/8011311.8014013.01/webrev/>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8011311
>>
>> Based on JVMS update for default method support for lambda, version 0.6.3 and bug JDK-8025604:
>> http://cr.openjdk.java.net/~dlsmith/jsr335-0.6.3/ <http://cr.openjdk.java.net/%7Edlsmith/jsr335-0.6.3/>
>> Part J: Java Virtual Machine
>>
>> Summary of changes
>> 1. added private interface method support
>> - which can only be invoked via invokespecial directly on the interface method reference containing the private method
>> 2. invokevirtual and invokeinterface throw IncompatibleclassChangeError if they attempt to reference a private interface method
>> 3. conflicting default methods throw ICCE rather than AbstractMethodError
>> 4. invokespecial superinterface.method no longer performs additional check for shadowing of superinterfaces
>> 5. added some debug tracing
>>
>>
>> Note: vmsqe defmeth tests: PrivateMethodsTest updated to cover these cases.
>>
>> Testing: with JDK-8012557 to actually generate internal private interface methods (javac won't generate them)
>> 1. specific test for 8011311
>> 2. defmeth PrivateMethodsTest
>> 3. reflection invocation test
>> (16th and subsequent reflection success depends on an upcoming change to the jdk repository)
>> 4. internal TypeTest.java - in conjunction with JDK-8012557 changes
>> 5. 8025475 sample test - depends on upcoming javac/metafactory changes in JDK-8012557
>> 6. 8014013 intfbug
>> 7. invokespecial, invokevirtual, invokeinterface tests (filed bug for recently broken issue: JDK-8025592)
>> 8. lambda DefaultMethodsTest - 3 tests need updating for AME->ICCE spec change
>> 9. jtreg java.util, java.lang
>> 10. jck lang
>> 11. jck vm:
>> need to file a new JCK bug for 8 tests which need to change due to spec update, resolveIntMethod, resolveMethod in vm.constantpool
>> for private and static methods in interfaces, relative to spec bug JDK-8025604
>> 12. vm.quick.testlist
>> 13. vm.mlvm.testlist
>> 14. jprt in progress
>>
>> thanks,
>> Karen
>>
>
More information about the hotspot-runtime-dev
mailing list