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

[Jim Laskey]

On Fri, 28 Oct 2022 19:12:02 GMT, Rémi Forax <forax at openjdk.org> wrote:

>> 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/template/StringTemplate.java line 307:
> 
>> 305:             Objects.requireNonNull(fragment, "fragments elements must be non-null");
>> 306:         }
>> 307:         fragments = Collections.unmodifiableList(new ArrayList<>(fragments));
> 
> I think using `List.copyOf()` is more efficient that `Collections.unmodifiableList(new ArrayList<>(...))` because there is no copy if the list is already created with List.of().
> 
> Edit:
> fragments should use List.copyOf() but i suppose that a value inside values can be null, so you can not use List.copyOf() for the values.
> 
> There a little secret, the ImmutableList  has a special flag to represent an unmodifiable list that can access null, this implementation is used by `stream.toList()`, i think you should use a shared secret access to have have access to this implementation instead of relying on `Collections.unmodifiableList(new ArrayList<>(...))`.
> 
> @stuart-marks, do you think it's a good idea to use the null allowed ImmutableList here ?

Changing as recommended

> src/java.base/share/classes/java/lang/template/StringTemplate.java line 354:
> 
>> 352:      * @implNote The result of interpolation is not interned.
>> 353:      */
>> 354:     public static final StringProcessor STR = st -> st.interpolate();
> 
> Should be `StringTemplate::interpolate`.

Yep

> src/java.base/share/classes/java/lang/template/TemplateProcessor.java line 38:
> 
>> 36:  * that do not throw checked exceptions. For example:
>> 37:  * {@snippet :
>> 38:  * TemplateProcessor<String> processor = st -> {
> 
> This is a good example of why having both way to describe a template processor of string, TemplateProcessor<String and `StringPorcessor` is a bad idea.

Noted

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

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