[code-reflection] RFR: Committing a Concatenation transformation to StringBuilder

[Paul Sandoz]

On Thu, 13 Jun 2024 20:00:29 GMT, Ian Graves <igraves at openjdk.org> wrote:

> A transformation that converts String Concatenation to StringBuilder for uses in lowering to Bytecode.

src/java.base/share/classes/java/lang/reflect/code/transformations/LinearConcatTransform.java line 34:

> 32: import java.lang.reflect.code.type.MethodRef;
> 33: 
> 34: public class LinearConcatTransform {

Suggest a rename to `LinearStringContextTransformer` and merge with `ContactTransform` (no need for an inner class). Rather than create a new package place in the analysis package.

src/java.base/share/classes/java/lang/reflect/code/transformations/LinearConcatTransform.java line 68:

> 66:         private static StringAndBuilder stringBuild(Block.Builder builder, Value left, Value right) {
> 67:             var newB = stringBuilder(builder, left, right);
> 68:             var toStringMR = MethodRef.method(SBC_TYPE, "toString", JavaType.J_L_STRING);

Declare as static final.

src/java.base/share/classes/java/lang/reflect/code/transformations/LinearConcatTransform.java line 79:

> 77: 
> 78:             sb = builder.apply(newBuilder);
> 79:             var leftMethodDesc = MethodRef.method(SBC_TYPE, "append", SBC_TYPE, left.type());

Refactor this ref and op construction into a method e.g., `append(sb, left.type())` and then inline in the `builder.op` call

test/jdk/java/lang/reflect/code/TestStringConcatTransform.java line 112:

> 110: 
> 111:     @Test(dataProvider = "getClassMethods")
> 112:     public void testSSAModelTransform(@NoInjection Method method) {

Also transform model as in `model.transform(new LinearConcatTransform.ConcatTransform())` i.e. before SSA too.

test/jdk/java/lang/reflect/code/TestStringConcatTransform.java line 199:

> 197:                 return block;
> 198:             }
> 199:         });

Use `OpTransformer.LOWERING_TRANSFORMER`

test/jdk/java/lang/reflect/code/TestStringConcatTransform.java line 224:

> 222:     }
> 223: 
> 224:     static CoreOp.FuncOp lower(CoreOp.FuncOp f) {

Not used. I recommend you look at the code analyze problems in the IDE, it will catch stuff like this.

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

PR Review Comment: https://git.openjdk.org/babylon/pull/135#discussion_r1638902294
PR Review Comment: https://git.openjdk.org/babylon/pull/135#discussion_r1638901943
PR Review Comment: https://git.openjdk.org/babylon/pull/135#discussion_r1638935940
PR Review Comment: https://git.openjdk.org/babylon/pull/135#discussion_r1638947225
PR Review Comment: https://git.openjdk.org/babylon/pull/135#discussion_r1638940348
PR Review Comment: https://git.openjdk.org/babylon/pull/135#discussion_r1638942943