[lworld] RFR: 8247569: [lworld] Align with spec for <vnew> methods

Mandy Chung mchung at openjdk.org
Tue Oct 18 17:38:15 UTC 2022


On Wed, 12 Oct 2022 08:44:41 GMT, David Simms <dsimms at openjdk.org> wrote:

> Merged javac and VM work together, includes VM support (JDK-8294224)

src/java.base/share/classes/java/lang/invoke/MemberName.java line 527:

> 525:     }
> 526:     /** Query whether this member is a constructor. */
> 527:     boolean isConstructor(String name) {

`findConstructor` only finds `<init>`.  To get `<vnew>` method handle, it just uses `Lookup::findStatic` as it's a static method.   This `isConstructor` method would cause confusion.  I can clean this up as a follow-up.

src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 2765:

> 2763:                 throw new NoSuchMethodException("Constructors must have void return type: " + refc.getName());
> 2764:             }
> 2765:             String name = refc.isValue() ? "<vnew>" : "<init>";

`findConstructor` should not find `<vnew>` which is just a static method.

src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 3704:

> 3702:             return (name.startsWith("<") &&
> 3703:             ((name.equals("<init>") && refKind != REF_newInvokeSpecial) ||
> 3704:             ((name.equals("<vnew>") || name.equals("<clinit>")) && refKind != REF_invokeStatic)));

This method should only check for `<vnew>`.   `<clinit>` cannot be looked up.

src/java.base/share/classes/jdk/internal/reflect/MethodAccessorGenerator.java line 95:

> 93:         boolean isStaticFactory = declaringClass.isValue();
> 94:         return (ConstructorAccessor) generate(declaringClass,
> 95:                                               isStaticFactory ? "<vnew>" : "<init>",

Core reflection does not use this method accessor generator by default.   I don't expect this change is needed.  Are you seeing any test failure without this change and other relevant change in jdk.internal.reflect?

test/jdk/valhalla/valuetypes/StaticFactoryMethodHandleTest.java line 85:

> 83:     @Test
> 84:     public void testNoArgStaticFactory() throws Throwable {
> 85:         // test default vnew init factory

Suggestion:

        // test default static vnew factory

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

PR: https://git.openjdk.org/valhalla/pull/788



More information about the valhalla-dev mailing list