On Fri, 28 Oct 2022 19:12:02 GMT, Rémi Forax <fo...@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
