RFR: 8358772: Template-Framework Library: Primitive Types

Christian Hagedorn chagedorn at openjdk.org
Thu Jun 12 11:04:36 UTC 2025 

On Fri, 6 Jun 2025 13:25:48 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> I would like to add primitive type support to the template framework library.
> 
> In follow-up work, we will use these types in random expression generation - but they can also already be useful on their own now.
> 
> I encountered an issue with some methods that return `Token` from the `TemplateFramework`, such as `Hook.insert` and `addDataName`. Since `Token` was package private, this class could not be used in some places where automatic type inference is required. I now refactored the code, so that the `Token` is just an empty interface, and all the methods are moved to a separate class `TokenParser`.
> 
> Original experiments from here: https://github.com/openjdk/jdk/pull/23418

Nice additions to the library! Lot of minor comments, otherwise, looks good.

test/hotspot/jtreg/compiler/lib/template_framework/Token.java line 29:

> 27: import java.util.ArrayList;
> 28: import java.util.List;
> 29: 

The imports can now be removed (cannot make a suggestion, it's hidden).

test/hotspot/jtreg/compiler/lib/template_framework/library/CodeGenerationDataNameType.java line 35:

> 33:  * additional functionality for code generation. These types with their extended
> 34:  * functionality can be used with many other code generation facilities in the
> 35:  * lbrary, such as generating random {@code Expression}s.

Suggestion:

 * library, such as generating random {@code Expression}s.

test/hotspot/jtreg/compiler/lib/template_framework/library/CodeGenerationDataNameType.java line 45:

> 43:      * @return A random constant value.
> 44:      */
> 45:     public Object con();

You can remove all the public identifies since the methods are implicitly public.

test/hotspot/jtreg/compiler/lib/template_framework/library/CodeGenerationDataNameType.java line 106:

> 104:      * List of all {@link PrimitiveType}s.
> 105:      */
> 106:     public static final List<PrimitiveType> PRIMITIVE_TYPES = List.of(

Same with the constants: They are implicitly public static final.

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 52:

> 50:     private static enum Kind { BYTE, SHORT, CHAR, INT, LONG, FLOAT, DOUBLE, BOOLEAN };
> 51: 
> 52:     // We have one static instance each, so we do not have duplicat instances.

Suggestion:

    // We have one static instance each, so we do not have duplicated instances.

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 76:

> 74:     @Override
> 75:     public String name() {
> 76:         return switch(kind) {

Suggestion:

        return switch (kind) {

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 94:

> 92: 
> 93:     public Object con() {
> 94:         return switch(kind) {

Suggestion:

        return switch (kind) {

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 102:

> 100:             case FLOAT   -> GEN_FLOAT.next();
> 101:             case DOUBLE  -> GEN_DOUBLE.next();
> 102:             case BOOLEAN -> RANDOM.nextInt() % 2 == 0;

You can use:
Suggestion:

            case BOOLEAN -> RANDOM.nextBoolean();

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 113:

> 111:      */
> 112:     public int byteSize() {
> 113:         return switch(kind) {

Suggestion:

        return switch (kind) {

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 120:

> 118:             case LONG    -> 8;
> 119:             case FLOAT   -> 4;
> 120:             case DOUBLE  -> 8;

Up to you if you want to merge cases like that for more compactness:
Suggestion:

            case BYTE    -> 1;
            case SHORT, CHAR -> 2;
            case INT, FLOAT -> 4;
            case LONG, DOUBLE -> 8;

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 131:

> 129:      */
> 130:     public String boxedTypeName() {
> 131:         return switch(kind) {

Suggestion:

        return switch (kind) {

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 149:

> 147:      */
> 148:     public boolean isFloating() {
> 149:         return switch(kind) {

Suggestion:

        return switch (kind) {

test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java line 157:

> 155:             case FLOAT   -> true;
> 156:             case DOUBLE  -> true;
> 157:             case BOOLEAN -> false;

Same here, up to you:
Suggestion:

            case BYTE, SHORT, CHAR, INT, LONG, BOOLEAN -> false;
            case FLOAT, DOUBLE -> true;

test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestPrimitiveTypes.java line 42:

> 40: 
> 41: import compiler.lib.compile_framework.*;
> 42: import compiler.lib.template_framework.DataName;

Unused:
Suggestion:

test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestPrimitiveTypes.java line 50:

> 48: import static compiler.lib.template_framework.Template.$;
> 49: import static compiler.lib.template_framework.Template.addDataName;
> 50: import static compiler.lib.template_framework.Template.dataNames;

Unused:
Suggestion:

test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestPrimitiveTypes.java line 79:

> 77:     public static String generate() {
> 78:         // Generate a list of test methods.
> 79:         Map<String,TemplateToken> tests = new HashMap<>();

Suggestion:

        Map<String, TemplateToken> tests = new HashMap<>();

test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestPrimitiveTypes.java line 166:

> 164:                     CodeGenerationDataNameType.PRIMITIVE_TYPES.stream().map(type ->
> 165:                         sampleTemplate.asToken(type)
> 166:                     ).toList()

Can be simplified to:
Suggestion:

                    CodeGenerationDataNameType.PRIMITIVE_TYPES.stream().map(sampleTemplate::asToken).toList()

-------------

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25672#pullrequestreview-2920519548
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142332525
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142333995
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142335015
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142336679
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142341330
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142344920
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142345127
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142343704
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142345326
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142347722
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142349338
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142350538
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142351955
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142353227
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142353655
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142356649
PR Review Comment: https://git.openjdk.org/jdk/pull/25672#discussion_r2142359970

More information about the hotspot-compiler-dev mailing list