On Fri, 26 Apr 2024 20:35:44 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> 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. > > I know it'll never call `primitiveClassDescriptor()` besides primitive types. > While `void.class.isPrimitive()` returns true, `void` is not a primitive > type and called out explicitly in the spec. > > My point is about the behavior does not match the method name. It may be > better to rename it to `classDescriptor` instead. It's okay to clean up in > your other performance fix (perhaps #18945). Updated #18945, will try ignoring the pre-existing `Wrapper.primitiveType` inconsistencies. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18971#discussion_r1581580674
