On Mon, 30 Nov 2020 17:28:52 GMT, Alan Bateman <al...@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
