On Wed, 21 Jun 2023 22:02:16 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> This PR clarifies the spec of the 3-arg Class::forName regarding the format >> of the name for an array type which is of the form: one or more of "[" + >> binary name of the element type + ";'. > > Mandy Chung 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 eight additional > commits since the last revision: > > - improve the specification of forName > - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242 > - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242 > - Review comment. Add a test > - missing 'L' for the array class name > - review comment > - Clarify for an array class of primitive type > - 8310242: Clarify the name parameter to Class::forName A few small suggestions, otherwise looks good. Thanks. src/java.base/share/classes/java/lang/Class.java line 444: > 442: * <p> To obtain {@code Class} object associated with an array class, > 443: * the name consists of one or more {@code '['} representing the > depth > 444: * of the array class, followed by the element type as encoded in Suggestion + * <p> To obtain the {@code Class} object associated with an array class, + * the name consists of one or more {@code '['} representing the depth + * of the array nesting, followed by the element type as encoded in src/java.base/share/classes/java/lang/Class.java line 451: > 449: * Class<?> threadClass = Class.forName("java.lang.Thread", false, > currentLoader); > 450: * Class<?> stringArrayClass = Class.forName("[Ljava.lang.String;", > false, currentLoader); > 451: * Class<?> intArrayClass = Class.forName("[[[I", false, > currentLoader); The variable name doesn't match the actual class loaded. It might be clearer to add a trailing comment `// Class of int[][][]` ? test/jdk/java/lang/Class/forName/ForNameNames.java line 52: > 50: @ParameterizedTest > 51: @MethodSource("testCases") > 52: void testForName(String cn, Class<?> expected) throws > ClassNotFoundException { Should this also test that `c.getName().equals(cn)`? ------------- Changes requested by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14528#pullrequestreview-1492109276 PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237843279 PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237843986 PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237846267
