[lworld] RFR: 8237073: [lworld] Need special handling of jlO constructor invocation [v2]

[Mandy Chung]

On Fri, 16 Jul 2021 05:05:10 GMT, Srikanth Adayapalam <sadayapalam at openjdk.org> wrote:

>> Translate new Object() instantiations down to invovations of java.util.Objects.newIdentity() call with an informational message at compie time
>
> Srikanth Adayapalam 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 three additional commits since the last revision:
> 
>  - Incorporate review comments from Mandy Chung
>  - Merge branch 'lworld' into JDK-8237073
>  - 8237073: [lworld] Need special handling of jlO constructor invocation

test/jdk/jdk/dynalink/BeanLinkerTest.java line 186:

> 184:         final MethodType mt = MethodType.methodType(Object.class, Object.class, String.class);
> 185:         final CallSite cs = createCallSite(publicLookup, GET_PROPERTY, mt);
> 186:         Assert.assertEquals(cs.getTarget().invoke(new Object(), "class"), java.util.Objects.newIdentity().getClass());

The change looks okay but this may be unclear to the reader why the returned class is not Object.class.

An alternative fix is to define an explicit `TestObject` class:


diff --git a/test/jdk/jdk/dynalink/BeanLinkerTest.java b/test/jdk/jdk/dynalink/BeanLinkerTest.java
index fafc1be447f..f22aab8f026 100644
--- a/test/jdk/jdk/dynalink/BeanLinkerTest.java
+++ b/test/jdk/jdk/dynalink/BeanLinkerTest.java
@@ -179,11 +179,13 @@ public class BeanLinkerTest {
         this.linker = null;
     }
 
+    static class TestObject {}
+
     @Test(dataProvider = "flags")
     public void getPropertyTest(final boolean publicLookup) throws Throwable {
         final MethodType mt = MethodType.methodType(Object.class, Object.class, String.class);
         final CallSite cs = createCallSite(publicLookup, GET_PROPERTY, mt);
-        Assert.assertEquals(cs.getTarget().invoke(new Object(), "class"), Object.class);
+        Assert.assertEquals(cs.getTarget().invoke(new TestObject(), "class"), TestObject.class);
         Assert.assertEquals(cs.getTarget().invoke(new Date(), "class"), Date.class);
     }
 
@@ -191,14 +193,14 @@ public class BeanLinkerTest {
     public void getPropertyNegativeTest(final boolean publicLookup) throws Throwable {
         final MethodType mt = MethodType.methodType(Object.class, Object.class, String.class);
         final CallSite cs = createCallSite(publicLookup, GET_PROPERTY, mt);
-        Assert.assertNull(cs.getTarget().invoke(new Object(), "DOES_NOT_EXIST"));
+        Assert.assertNull(cs.getTarget().invoke(new TestObject(), "DOES_NOT_EXIST"));
     }
 
     @Test(dataProvider = "flags")
     public void getPropertyTest2(final boolean publicLookup) throws Throwable {
         final MethodType mt = MethodType.methodType(Object.class, Object.class);
         final CallSite cs = createCallSite(publicLookup, GET_PROPERTY, "class", mt);
-        Assert.assertEquals(cs.getTarget().invoke(new Object()), Object.class);
+        Assert.assertEquals(cs.getTarget().invoke(new TestObject()), TestObject.class);
         Assert.assertEquals(cs.getTarget().invoke(new Date()), Date.class);
     }
 
@@ -208,7 +210,7 @@ public class BeanLinkerTest {
         final CallSite cs = createCallSite(publicLookup, GET_PROPERTY, "DOES_NOT_EXIST", mt);
 
         try {
-            cs.getTarget().invoke(new Object());
+            cs.getTarget().invoke(new TestObject());
             throw new RuntimeException("Expected NoSuchDynamicMethodException");
         } catch (final Throwable th) {
             Assert.assertTrue(th instanceof NoSuchDynamicMethodException);

test/jdk/jdk/dynalink/TrustedDynamicLinkerFactoryTest.java line 196:

> 194:                 MethodHandles.publicLookup(), GET_PROPERTY, mt)));
> 195:         Assert.assertFalse(reachedPrelinkTransformer[0]);
> 196:         Assert.assertEquals(cs.getTarget().invoke(new Object(), "class"), java.util.Objects.newIdentity().getClass());

Similar to BeanLinkerTest.java, it's clearer to define a TestObject class to replace "new Object()" in this test.

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

PR: https://git.openjdk.java.net/valhalla/pull/481