Looks good.

Best regards,
Vladimir Ivanov

PS: I noticed you missed ACC_PRIVATE flag on the invoker class, but then erroneously put it on the method. Please, move it to class flags before pushing. No need to send updated webrev.

+ cw.visit(52, ACC_PRIVATE | ACC_SUPER, "InjectedInvoker", null, "java/lang/Object", null);
+
+             MethodVisitor mv = cw.visitMethod(ACC_STATIC, "invoke_V",


On 5/16/16 8:18 AM, shilpi.rast...@oracle.com wrote:
Thanks Vladimir and Remi.

Please review updated webrev
http://cr.openjdk.java.net/~srastogi/8149574/webrev/

Regards,
Shilpi

On 5/13/2016 7:31 PM, Vladimir Ivanov wrote:
Good point, Remi. I agree that it's redundant.

Best regards,
Vladimir Ivanov

On 5/13/16 4:51 PM, Remi Forax wrote:
Also instead of
  MethodVisitor mv = cw.visitMethod(ACC_PRIVATE | ACC_STATIC,
"invoke_V",
"(Ljava/lang/invoke/MethodHandle;[Ljava/lang/Object;)Ljava/lang/Object;",

                          null, new String[] { "java/lang/Throwable" });

because the code is never read by a java compiler (as javac), you
don't need to specify the exception
(checked exceptions are a Java the language artifact)
  MethodVisitor mv = cw.visitMethod(ACC_PRIVATE | ACC_STATIC,
"invoke_V",
"(Ljava/lang/invoke/MethodHandle;[Ljava/lang/Object;)Ljava/lang/Object;",

                          null, null);

my 2 cents,
Rémi

----- Mail original -----
De: "Vladimir Ivanov" <vladimir.x.iva...@oracle.com>
À: "shilpi rastogi" <shilpi.rast...@oracle.com>
Cc: core-libs-dev@openjdk.java.net
Envoyé: Vendredi 13 Mai 2016 15:41:33
Objet: Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of
Unsafe.defineAnonymousClass()

MethodHandle vamh = prepareForInvoker(MH_checkCallerClass);
     Object ok = bccInvoker.invokeExact(vamh, new
Object[]{hostClass, bcc});
+   assert Boolean.TRUE.equals(ok) : ok;

What I meant is to convert the whole test (inside try-catch block) into
an assert.


+            UNSAFE.ensureClassInitialized(bcc);

Do people see any reason to force invoker class init? I'd prefer to see
it goes away.

Also, as a second thought, generateInvokerTemplate() looks clearer than
invokerTemplateGenerator().

Something like the following:
   http://cr.openjdk.java.net/~vlivanov/8149574/webrev.00/

Additional cleanup: checkCallerClass() should return injected invoker
class when invoked using a method handle, so no need in 2nd argument.

Best regards,
Vladimir Ivanov

On 5/13/16 11:56 AM, shilpi.rast...@oracle.com wrote:
Thanks Paul!

Please review http://cr.openjdk.java.net/~srastogi/8149574/webrev10.0/

Regards,
Shilpi

On 5/13/2016 2:09 PM, Paul Sandoz wrote:
assert Boolean.TRUE.equals(ok) : ok;



Reply via email to