RFR: 8159746: (proxy) Support for default methods [v7]

Mandy Chung mchung at openjdk.java.net
Mon Nov 30 19:17:07 UTC 2020


On Mon, 30 Nov 2020 17:28:52 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Mandy Chung has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 25 additional commits since the last revision:
>> 
>>  - Merge branch 'master' of https://github.com/openjdk/jdk into proxy-default-method
>>  - update copyright header
>>  - clean up DefaultMethodProxy test
>>  - Performance improvement contributed by plevart
>>  - Clean up the patch.  Rename to
>>  - Merge branch 'master' of https://github.com/openjdk/jdk into proxy-default-method
>>  - move invokeDefaultMethod to InvocationHandler and throw IAE if access check fails
>>  - Merge branch 'master' of https://github.com/openjdk/jdk into proxy-default-method
>>  - Merge branch 'master' of https://github.com/openjdk/jdk into proxy-default-method
>>  - minor tweak to the spec wording and impl
>>  - ... and 15 more: https://git.openjdk.java.net/jdk/compare/1580f68b...d72d627f
>
> src/java.base/share/classes/java/lang/reflect/Proxy.java line 1227:
> 
>> 1225:         if (proxyInterfaces.contains(declaringClass))
>> 1226:             return declaringClass;
>> 1227: 
> 
> It might be beneficial for future maintainers if you could include a comment here on the search order.

What about:

+        // find the first proxy interface that inherits the default method
+        // i.e. the declaring class of the default method is a superinterface
+        // of the proxy interface

> src/java.base/share/classes/java/lang/reflect/Proxy.java line 1281:
> 
>> 1279:      * @return a lookup for proxy class of this proxy instance
>> 1280:      */
>> 1281:     private static MethodHandles.Lookup proxyClassLookup(MethodHandles.Lookup caller, Class<?> proxyClass) {
> 
> The method description could be a bit clearer. It invokes the proxy's proxyClassLookup method to get a Lookup on the proxy class ("this proxy instance" is just a bit confusing as proxyClassLookup is static).
> I guess the caller parameter isn't really needed as it could be crated in proxyClassLookup.

The caller parameter is just another level of defense.   I updated as:

     /**
-     * Returns a Lookup object for the lookup class which is the class of this
-     * proxy instance.
+     * This method invokes the proxy's proxyClassLookup method to get a
+     * Lookup on the proxy class.
      *
      * @return a lookup for proxy class of this proxy instance
      */

-------------

PR: https://git.openjdk.java.net/jdk/pull/313


More information about the core-libs-dev mailing list