RFR: 8310242: Clarify the name parameter to Class::forName [v6]

David Holmes dholmes at openjdk.org
Thu Jun 22 00:24:11 UTC 2023


On Wed, 21 Jun 2023 22:02:16 GMT, Mandy Chung <mchung at 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


More information about the core-libs-dev mailing list