[code-reflection] RFR: Adding String Concatenation to Code Model

[Paul Sandoz]

On Mon, 8 Apr 2024 20:46:33 GMT, Ian Graves <igraves at openjdk.org> wrote:

> Adding String concatenation to the code model and interpreter.

I think `Triple` can be named to something more specific with associated helper methods (as described in the comment), otherwise all other comments are just cosmetic.

src/java.base/share/classes/java/lang/reflect/code/op/CoreOps.java line 2371:

> 2369:         public ConcatOp(OpDefinition def) {
> 2370:             super(def);
> 2371:             if(def.operands().size() != 2) {

Suggestion:

            if (def.operands().size() != 2) {

test/jdk/java/lang/reflect/code/TestConcat.java line 160:

> 158: 
> 159: 
> 160:     record Triple(Class<?> first, Class<?> second, String third) {

Rename to `TestMethodData`?, where the third argument can be moved to the first and renamed to `methodName`? Then you can add functionality on this record to obtain the method from its components and to create the args array.

test/jdk/java/lang/reflect/code/TestConcat.java line 191:

> 189:                 double.class, char.class, boolean.class, Object.class);
> 190: 
> 191: 

Suggestion:

test/jdk/java/lang/reflect/code/TestConcat.java line 194:

> 192: 
> 193:         //Types from types concatenated to strings left-to-right and right-to-left
> 194:         Stream<Triple> s1 = types.stream().map(t -> new Triple(t,String.class, testName(t, 1)));

Suggestion:

        Stream<Triple> s1 = types.stream().map(t -> new Triple(t, String.class, testName(t, 1)));

test/jdk/java/lang/reflect/code/TestConcat.java line 211:

> 209: 
> 210:         return args;
> 211: 

Suggestion:

test/jdk/java/lang/reflect/code/TestConcat.java line 217:

> 215:     public static void testRun(Triple t) {
> 216:         try {
> 217: 

Suggestion:

test/jdk/java/lang/reflect/code/TestConcat.java line 226:

> 224: 
> 225:             Assert.assertEquals(res1, res2);
> 226: 

Suggestion:

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

PR Review: https://git.openjdk.org/babylon/pull/47#pullrequestreview-1987392601
PR Review Comment: https://git.openjdk.org/babylon/pull/47#discussion_r1556424343
PR Review Comment: https://git.openjdk.org/babylon/pull/47#discussion_r1556432410
PR Review Comment: https://git.openjdk.org/babylon/pull/47#discussion_r1556427468
PR Review Comment: https://git.openjdk.org/babylon/pull/47#discussion_r1556427177
PR Review Comment: https://git.openjdk.org/babylon/pull/47#discussion_r1556428272
PR Review Comment: https://git.openjdk.org/babylon/pull/47#discussion_r1556426001
PR Review Comment: https://git.openjdk.org/babylon/pull/47#discussion_r1556426354