<i18n dev> RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]

[Rémi Forax]

On Fri, 28 Oct 2022 17:57:30 GMT, Jim Laskey <jlaskey at openjdk.org> wrote:

>> Enhance the Java programming language with string templates, which are similar to string literals but contain embedded expressions. A string template is interpreted at run time by replacing each expression with the result of evaluating that expression, possibly after further validation and transformation. This is a [preview language feature and API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update TemplateRuntime::combine

src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1042:

> 1040:      * The number of fragments must be one more that the number of ptypes.
> 1041:      * The total number of slots used by the ptypes must be less than or equal
> 1042:      * to MAX_INDY_CONCAT_ARG_SLOTS.

see my comment about making MAX_INDY_CONCAT_ARG_SLOTS public

src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1060:

> 1058:             throws StringConcatException
> 1059:     {
> 1060:         Objects.requireNonNull(fragments, "fragments is null");

I think you need to do some defensive copy here

  ptypes = List.copyOf(pTypes);

to avoid the types and fragments to be changed at the same time they are checked.

src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1177:

> 1175:      */
> 1176:     @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES)
> 1177:     public static List<MethodHandle> makeConcatWithTemplateCluster(

I think instead of having two public methods and the user having to choose between them, it's better to have the implementations private and on public methods that calls the correct implementation if maxSlots > MAX_INDY_CONCAT_ARG_SLOTS or not

src/java.base/share/classes/java/lang/template/ProcessorLinkage.java line 51:

> 49:     /**
> 50:      * Construct a {@link MethodHandle} that constructs a result based on the
> 51:      * bootstrap method information.

This comment is quite obscure if you have no idea how it works.
And the information that the returned method handle has the type of the MethodType passed as parameter is missing.

src/java.base/share/classes/java/lang/template/SimpleStringTemplate.java line 38:

> 36: record SimpleStringTemplate(List<String> fragments,
> 37:                              List<Object> values
> 38: ) implements StringTemplate {}

A compact constructor doing defensive copies is missing

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

PR: https://git.openjdk.org/jdk/pull/10889