On Fri, 26 Apr 2024 17:57:52 GMT, Mandy Chung <[email protected]> wrote:
>> Claes Redestad has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Copyrights and other comments from @JornVernee
>
> src/java.base/share/classes/sun/invoke/util/Wrapper.java line 384:
>
>> 382:
>> 383: /** A nominal descriptor of the primitive type */
>> 384: public ClassDesc primitiveClassDescriptor() { return
>> primitiveTypeDesc; }
>
> As this method is named as `primitiveClassDescriptor`, I expect this should
> either throw or return null for `OBJECT` and even `VOID` wrapper. Or should
> be named "classDescriptor".
While I agree it may seem non-sensical to define `CD_Object` for `Object`,
there's precedent in that `Wrapper.primitiveType` is `Object.class` - so for
consistency I wired it up the same way. For the usage patterns introduced with
this patch we'll never call `primitiveClassDescriptor()` on `Wrapper.OBJECT`,
though - and `Wrapper.forPrimitiveType` will throw an `IAE` if you try - so we
can always revisit this.
`void.class` is the primitive type for `Void.class`, allowed as a return type
and recognized by `Wrapper.forPrimitiveType`. There are checks in for example
`ConstantUtils.parseMethodDescriptor` which disallows `void.class` anywhere but
as a return type.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18971#discussion_r1581404597
