+1 On Jul 2, 2014, at 9:16 AM, A. Sundararajan <[email protected]> wrote:
> Updated review at http://cr.openjdk.java.net/~sundar/8043232/webrev.01/ > > Please review > > Thanks > -Sundar > > On Wednesday 02 July 2014 02:04 PM, Attila Szegedi wrote: >> Two remarks: >> >> 1. in DynamicMethodLinker.java, you could factor out the >> >> return new >> GuardedInvocation(MethodHandles.dropArguments(invocation, 0, >> desc.getMethodType().parameterType(0)), >> Guards.getIdentityGuard(receiver)); >> >> part that is appearing in both if and else branch to be outside of the if, >> guarded with if(invocation == null) (maybe adding an assert invocation != >> null in the constructor (else) branch if it must not be null in that case >> (as you aren't testing for it being null there). >> >> 2. OverloadedDynamicMethod could save the boolean flag by returning >> methods.getFirst().isConstructor(). Maybe assert in the >> OverloadedDynamicMethod constructor that all elements of "methods" return >> the same value for isConstructor(). >> >> Other than these, +1. >> >> Attila. >> >> On Jul 2, 2014, at 7:34 AM, A. Sundararajan >> <[email protected]> wrote: >> >>> Please review http://cr.openjdk.java.net/~sundar/8043232/webrev.00/ for >>> https://bugs.openjdk.java.net/browse/JDK-8043232 >>> >>> Thanks, >>> -Sundar >
