On Tue, 19 Nov 2024 09:40:25 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> Chen Liang has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains six commits: >> >> - Further facelift >> - Merge branch 'master' of https://github.com/openjdk/jdk into >> docs/cf-instruction >> - Merge branch 'master' of https://github.com/openjdk/jdk into >> docs/cf-instruction >> - Typos, long lines >> - Labels >> - Wip instr > > src/java.base/share/classes/java/lang/classfile/instruction/CharacterRange.java > line 62: > >> 60: * <p> >> 61: * Physically, a character range has the same structure; it is modeled >> by a >> 62: * {@link CharacterRangeInfo}. > > I'm not sure how to explain "Physically" and "it is modeled by". > If "Physically" means specific byte serialization, then it is an entry in a > CRT attribute with BCIs instead of labels. > > "it is modeled by a CharacterRangeInfo" seems to me very confusing as this > class (CharacterRange) is the primary model class. > > Maybe we should explain the duplicity of some classfile structures accessible > as attributes internal raw structures (CharacterRangeInfo) versus preferred > primary models as labeled pseudo instructions (CharacterRange). Done. I have simplified the record model and added dedicated info about CRI being independent from Code so it's used in the attribute. Same for LineNumber etc. > src/java.base/share/classes/java/lang/classfile/instruction/ConstantInstruction.java > line 58: > >> 56: * <p> >> 57: * Physically, a constant-load instruction is polymorphic; nested >> interfaces in >> 58: * this interface model different constant instructions as records. > > I'm not sure what the sentence "nested interfaces in this interface model > different constant instructions as records" gives to users and on the other > side what obligation does it mean from the JCK perspective? Removed this confusing and meaningless sentence. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21625#discussion_r1851092390 PR Review Comment: https://git.openjdk.org/jdk/pull/21625#discussion_r1851092682
