[code-reflection] RFR: Model String Template [v3]

Wed Feb 28 17:47:06 UTC 2024

On Wed, 28 Feb 2024 02:45:26 GMT, Mourad Abbay <mabbay at openjdk.org> wrote:

>> Model String Template
>
> Mourad Abbay has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update model of String Template

I recommend you enhance the tests to include richer expressions in some of the templates e.g. adjust test for `f3`. Also, include a test using the formatting processor (which should not change the model, but is useful generally as a reminder for when we add lowering and tests in a subsequent PR).

src/java.base/share/classes/java/lang/reflect/code/op/ExtendedOps.java line 3150:

> 3148: 
> 3149:         private final Value processorValue;
> 3150:         private final List<Value> literalsValues;

These two fields are redundant, as we can always refer to the operands directly.

Probably better to refer to the literals as fragments like they are referred in the compiler and i presume the language specification.

If you wish add methods called `processor` and `fragments` that return the operands.

src/java.base/share/classes/java/lang/reflect/code/op/ExtendedOps.java line 3151:

> 3149:         private final Value processorValue;
> 3150:         private final List<Value> literalsValues;
> 3151:         private final List<Body> expressionsBodies;

Just call it `expressions`.

src/java.base/share/classes/java/lang/reflect/code/op/ExtendedOps.java line 3171:

> 3169: 
> 3170:         public StringTemplateOp(Value processorValue, List<Value> literalsValues, List<Body.Builder> expressionsBodies) {
> 3171:             super(NAME, makeOperandsList(processorValue, literalsValues));

Add a comment before that to say we can use statements before super (JEP 447) when the compiler can depend on 22 features.

src/java.base/share/classes/java/lang/reflect/code/op/ExtendedOps.java line 3195:

> 3193:             TypeDefinition processorReturnType = processorValue.type().toTypeDefinition().arguments().get(0);
> 3194:             return CoreTypeFactory.JAVA_TYPE_FACTORY.constructType(processorReturnType);
> 3195:         }

That's a clever way to do it, but for now i recommend passing in the result type to the factory and constructor.

The other, and more preferred, way to operate on types is to check if the processor value's type is an instance of `JavaType` and then access as that type to obtain the argument type. We will probably revisit this whole area after we have decided how to model Java types.

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

PR Review: https://git.openjdk.org/babylon/pull/30#pullrequestreview-1906870127
PR Review Comment: https://git.openjdk.org/babylon/pull/30#discussion_r1506336785
PR Review Comment: https://git.openjdk.org/babylon/pull/30#discussion_r1506340751
PR Review Comment: https://git.openjdk.org/babylon/pull/30#discussion_r1506337766
PR Review Comment: https://git.openjdk.org/babylon/pull/30#discussion_r1506339301