RFR: 8370976: Review the behavioral changes of core reflection descriptor parsing migration

[ExE Boss]

On Thu, 20 Nov 2025 15:37:04 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java line 160:
>> 
>>> 158:             | (1L << ('/' - CHECK_OFFSET))
>>> 159:             | (1L << (';' - CHECK_OFFSET))
>>> 160:             | (1L << ('[' - CHECK_OFFSET));
>> 
>> Are we sure that these are the only 4 non-identifier chars we can see in the string?
>
> Could you add a test for something like `"Ljava#/lang/Object;"`?

That is a valid **JVM** [field descriptor] (but not a denotable type in **Java**).

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

These 4 are the only characters which are forbidden from appearing in identifiers by the [JVMS § 4.2.1].

[JVMS § 4.2.1]: https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html#jvms-4.2.1
[field descriptor]: https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html#jvms-4.3.2

>> src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java line 166:
>> 
>>> 164:             int check = str.charAt(index) - CHECK_OFFSET;
>>> 165:             if ((check & -Long.SIZE) == 0 && (NON_IDENTIFIER_MASK & (1L << check)) != 0) {
>>> 166:                 break;
>> 
>> Maybe this is a little clearer:
>> Suggestion:
>> 
>>             if (check < 64 && (NON_IDENTIFIER_MASK & (1L << check)) != 0) {
>>                 break;
>
> These generate similar code (`test` vs `cmp` on x64)

No because `check` can be negative:
https://github.com/openjdk/jdk/blob/c51f542914955d0034b18be7e5d40ae97e93baca/src/java.base/share/classes/sun/invoke/util/BytecodeDescriptor.java#L164

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28079#discussion_r2546770116
PR Review Comment: https://git.openjdk.org/jdk/pull/28079#discussion_r2546753516