On Thu, 6 Jun 2024 18:35:01 GMT, Chen Liang <[email protected]> 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
