On Thu, 5 Mar 2026 17:54:14 GMT, Chen Liang <[email protected]> wrote:
> In project valhalla development, we discovered that `Modifier.toString`
> becomes more problematic than helpful: there's now a mix-and-match of
> modifiers from access flags and other class file sources, for example,
> classes now can have ACC_IDENTITY, which clashes with ACC_SYNCHRONIZED, and
> the correct modifier to reflect, "value", must be deduced by the users
> manually.
>
> With fewer bits available for the access flags, it becomes more apparent that
> future source modifiers will no longer be reflected by `Modifier.toString`,
> and future access flags are more likely to be reflected incorrectly. Thus, we
> should dissuade users from this API in the long run.
>
>> This method tries to describe the source modifiers from an access flags
>> value. Since the introduction of this API, new source modifiers are
>> often represented by `class` file constructs other than access
>> flags, and access flags values in different `class` file
>> structures have different interpretations. As a result, the source
>> modifiers reported by this API may be incomplete or incorrect.
>>
>> The source modifiers of a declaration should be reconstructed manually,
>> by examining its reflective object. In addition, the reflective object
>> methods that provide user-friendly text representations, such as
>> `Class#toGenericString()`, render the source modifiers.
>>
>> The access flags of a declaration, with the correct interpretation, can
>> be obtained from the `accessFlags()` methods on the reflective
>> objects, such as `Class#accessFlags()`.
>>
>> To print an access flags value for debug output, consider using the
>> format ``%04x` instead of this method; this method omits all class
>> file access flags without a corresponding source modifier.
Hopefully these review comments are useful. Feel free to only consider them as
suggestions.
src/java.base/share/classes/java/lang/reflect/Modifier.java line 47:
> 45: * These assumptions are no longer valid:
> 46: * <ol>
> 47: * <li>Java language modifiers like method modifier {@code default} does
> not
Maybe grammar issue: This refers to the plural "modifiers"
Suggestion:
* <li>Java language modifiers like method modifier {@code default} do not
src/java.base/share/classes/java/lang/reflect/Modifier.java line 53:
> 51: * {@code ACC_BRIDGE} in interfaces</li>
> 52: * </ol>
> 53: * To mitigate, {@code Modifier} class only include source modifiers that
Maybe grammar issue
Suggestion:
* To mitigate, the {@code Modifier} class only includes source modifiers that
src/java.base/share/classes/java/lang/reflect/Modifier.java line 278:
> 276: * <p>
> 277: * To print an access flags value for debug output, consider using
> the
> 278: * format {@code %04x} instead of this method; this method omits all
> class
Maybe this should say "hex format" or similar to make it immediately obvious
what this pattern does?
Suggestion:
* hex format {@code %04x} instead of this method; this method omits all
class
src/java.base/share/classes/java/lang/reflect/Modifier.java line 481:
> 479: *
> 480: * @deprecated
> 481: * This method is intended to be used in conjunction with {@link
> #toString()
Link to wrong `toString` method? `toString()` is not deprecated only
`toString(int)`?
Suggestion:
* This method is intended to be used in conjunction with {@link
#toString(int)
(same also below for all other occurrences)
-------------
PR Review: https://git.openjdk.org/jdk/pull/30093#pullrequestreview-3944667880
PR Review Comment: https://git.openjdk.org/jdk/pull/30093#discussion_r2931771506
PR Review Comment: https://git.openjdk.org/jdk/pull/30093#discussion_r2931774830
PR Review Comment: https://git.openjdk.org/jdk/pull/30093#discussion_r2931801467
PR Review Comment: https://git.openjdk.org/jdk/pull/30093#discussion_r2931808382
