RFR: 8333749: Consolidate ConstantDesc conversion in java.base

[Claes Redestad]

On Thu, 6 Jun 2024 18:35:01 GMT, Chen Liang <liach at openjdk.org> wrote:

> In java.base, especially in bytecode generators, we have many different methods converting known valid Class and MethodType into ClassDesc and MethodTypeDesc. These conversions should be consolidated into the same utility method for the ease of future maintenance.

Looks good to me. Probably should get someone to OK changes in foreign/abi (@JornVernee perhaps?).

There are some pre-existing places where we call `ReferenceClassDescImpl.ofValidated` directly that could probably be switched over to `ConstantUtils.classDesc` for slightly nicer code. Or - if it matters - add a `referenceClassDesc` which avoids the `type.isPrimitive` test.

src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line 256:

> 254:             // the field name holding the method handle for this method
> 255:             String fieldName = "m" + count++;
> 256:             var mt = methodDesc(m.getReturnType(), JLRA.getExecutableSharedParameterTypes(m));

Suggestion:

            var md = methodDesc(m.getReturnType(), JLRA.getExecutableSharedParameterTypes(m));

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 469:

> 467:                             // o instanceof Wrapped(float)
> 468:                             cb.aload(SELECTOR_OBJ);
> 469:                             cb.instanceOf(classDesc(Wrapper.forBasicType(classLabel)

I have a patch somewhere to cache the wrapper class desc in `sun.invoke.util.Wrapper`, both as a micro-optimization and to help disambigutate the unfortunately named (my bad) `Wrapper::classDescriptor` method. Maybe we can roll that into this..?

src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java line 471:

> 469:                 case Allocate allocate           -> emitAllocBuffer(allocate);
> 470:                 case BoxAddress boxAddress       -> emitBoxAddress(boxAddress);
> 471:                 case SegmentBase _               -> emitSegmentBase();

Seem a bit too far detached from the changes at hand for a drive-by code cleanup?

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

Marked as reviewed by redestad (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19585#pullrequestreview-2102953208
PR Review Comment: https://git.openjdk.org/jdk/pull/19585#discussion_r1630049028
PR Review Comment: https://git.openjdk.org/jdk/pull/19585#discussion_r1630061889
PR Review Comment: https://git.openjdk.org/jdk/pull/19585#discussion_r1630074187