On Wed, 22 Apr 2026 00:34:08 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. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 10 additional commits since > the last revision: > > - Revision after CSR review > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/deprecate-modifier-tostring > - Fix failing test, typos > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/deprecate-modifier-tostring > - Missed comment > - Revisions > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/deprecate-modifier-tostring > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/deprecate-modifier-tostring > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/deprecate-modifier-tostring > - Deprecate Modifier.toString I think this is mostly looks good, just some minor word smithing on the docs change. src/java.base/share/classes/java/lang/reflect/Modifier.java line 38: > 36: * Modifier interpretation is context-sensitive: for example, the bit > checked by > 37: * {@link #isSynchronized(int) isSynchronized} only represents the {@code > 38: * synchronized} modifier on methods, so a {@code true} return on a field The first paragraph uses "flags", the second paragraph switches to "bits", a term that is never used again. The referenced AccessFlags or javax.lang.model.element.Modifier doesn't speak of bits. So I'm wondering if it would be better to say "the modifier checked by ..." rather than "bit checked". src/java.base/share/classes/java/lang/reflect/Modifier.java line 449: > 447: * @deprecated > 448: * This method exists solely to support the now-deprecated > 449: * {@link #toString(int) Modifier::toString(int)} method. What would think of changing the first sentence to "This method was originally created to the support ..". The second sentence is good, sends the reader to the new API. ------------- PR Review: https://git.openjdk.org/jdk/pull/30093#pullrequestreview-4154900547 PR Review Comment: https://git.openjdk.org/jdk/pull/30093#discussion_r3124154391 PR Review Comment: https://git.openjdk.org/jdk/pull/30093#discussion_r3124164436
