On Thu, 20 Nov 2025 15:37:04 GMT, Jorn Vernee <[email protected]> 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