On Mon, 12 Jun 2023 02:41:02 GMT, ExE Boss <d...@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 14 commits: >> >> - Review changes, fixed tests >> - Merge branch 'master' into hierarchy-resolve >> - 1. Moved the default resolver to a static method, in anticipation of >> future changes >> 2. Removed SecurityManager related content >> 3. Changed ClassHierarchyInfo into an interface >> 4. Moved caching method from static to instance method >> - Merge branch 'master' into hierarchy-resolve >> - rename to ofClassLoading/ofResourceParsing >> convert the default class provider to bypass security manager restrictions >> - Merge branch 'master' into hierarchy-resolve >> - Merge branch 'master' into hierarchy-resolve >> - Test both cached and uncached resolvers >> - Update the class hierarchy resolver api as per mailing list last week >> - Merge branch 'master' into hierarchy-resolve >> - ... and 4 more: https://git.openjdk.org/jdk/compare/a1ab377d...90645b6f > > src/java.base/share/classes/jdk/internal/classfile/impl/Util.java line 103: > >> 101: } >> 102: return descOrInternalName.substring(1, >> descOrInternalName.length() - 1).replace('/', '.'); >> 103: } else { > > Note that this won’t work right if this method is called with the internal > name of a class whose FQCN starts with `L`, the correct implementation would > be: > Suggestion: > > if (descOrInternalName.startsWith("L") && > descOrInternalName.endsWith(";")) { > // descriptors of classes or interfaces > if (descOrInternalName.length() <= 2) { > throw new IllegalArgumentException(descOrInternalName); > } > return descOrInternalName.substring(1, > descOrInternalName.length() - 1).replace('/', '.'); > } else {
Honestly, this API should be changed to take a ClassDesc instead. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13082#discussion_r1226080172
