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

[Rémi Forax]

On Fri, 28 Oct 2022 20:01:41 GMT, Jim Laskey <jlaskey at openjdk.org> wrote:

>> 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
>
> Disagree.

As i said above, i consider this thread as resolved

>> 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
>
> Use cases are very different.  The first one produces a `MethodHandle` that has multiple inputs, The second one produces a `MethodHandle` that can only have one input.

yes, sorry for the noise.

>> 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
>
> The defensive copies are done by the callers.

In that case, i wonder if not not better to move that record inside another class, closer to where the callers are

>> src/java.base/share/classes/java/lang/template/StringProcessor.java line 45:
>> 
>>> 43: @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES)
>>> 44: @FunctionalInterface
>>> 45: public interface StringProcessor extends TemplateProcessor<String> {}
>> 
>> The name should be `StringTemplateProcessor`.
>> And i'm not sure it's useful to have a specialized version for String, TemplateProcessor<String> is not an issue given that most of the time people will implement it, so writing `implements StringTemplateProcessor` instead of `implements TemplateProcessor<String>` does not seem to offer enough bangs for bucks.
>> 
>> see TemplateProcessor
>
> Wrong use case. Think `StringProcessor upper = st -> st.interpolate().toUpperCase();`

Is it that different from`TemplateProcessor<String> upper = st -> st.interpolate().toUpperCase();` ?

People are really used to use <> with the functional interfaces of java.util.function, and you avoid the "two ways" to express the same thing.

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

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