Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v56]
On Wed, 12 Apr 2023 15:15:09 GMT, Chen Liang wrote: >> Jim Laskey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Change MAX_INDY_CONCAT_ARG_SLOTS to be updatable. > > src/java.base/share/classes/java/lang/runtime/StringTemplateImplFactory.java > line 112: > >> 110: } >> 111: interpolateMH = MethodHandles.filterArguments(interpolateMH, 0, >> components); >> 112: mt = MethodType.methodType(String.class, >> StringTemplateImpl.class); > > This MethodType can be stored in a static final field than created every time > on the fly. Don't know if JIT compiler can inline this statement. Same fore > that `List.class, StringTemplateImpl.class` type below. Changing - PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1165433226
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v56]
On Wed, 12 Apr 2023 15:09:50 GMT, Chen Liang wrote: >> Jim Laskey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Change MAX_INDY_CONCAT_ARG_SLOTS to be updatable. > > src/java.base/share/classes/java/lang/runtime/TemplateRuntime.java line 109: > >> 107: * @param lookup method lookup >> 108: * @param namemethod name >> 109: * @param typemethod type > > Maybe mention that the name is ignored, and the type must be convertible from > `(String[], Object[])StringTemplate`? The specification says "fragments list" > and "values list", which are more accurately described as arrays. > > Also, does "large" string template mean there are more than 250~ fragments > that the bootstrap method arguments can't fit, or does it mean dynamic number > of fragments? Might be worth mentioning that as well. Updating comments - PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1165418717
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v56]
On Sat, 8 Apr 2023 15:51:36 GMT, Jim Laskey 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: > > Change MAX_INDY_CONCAT_ARG_SLOTS to be updatable. This is a remnant from when the processor was not required. - PR Comment: https://git.openjdk.org/jdk/pull/10889#issuecomment-1505712211
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v56]
On Sat, 8 Apr 2023 15:51:36 GMT, Jim Laskey 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:
>
> Change MAX_INDY_CONCAT_ARG_SLOTS to be updatable.
Just curious, in what scenario can `StringTemplateTree.getProcessor()` evaluate
to `null`, when `"{variable}"` is not a valid literal without a
`StringTemplate.RAW.` qualifier?
-
PR Comment: https://git.openjdk.org/jdk/pull/10889#issuecomment-1505686329
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v56]
On Sat, 8 Apr 2023 15:51:36 GMT, Jim Laskey 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:
>
> Change MAX_INDY_CONCAT_ARG_SLOTS to be updatable.
A few comments on the API/CSR's specification. Thanks for uploading the
Javadocs.
src/java.base/share/classes/java/lang/StringTemplate.java line 351:
> 349: * then it is returned unchanged.
> 350: */
> 351: static StringTemplate combine(StringTemplate... stringTemplates) {
Sorry for a late mention, but I believe this method and its List variant are
better named `concat` instead: `concat` indicates an explicitly ordered
operation (String, Stream, AffineTransform), while `combine` is usually
unordered (IntSummaryStatistics, CompletableFuture.thenCombine,
Collector.combiner). Using `concat` also brings `String` and `StringTemplate`
closer.
All examples come from using the search box with `concat` and `combine` in the
code review Javadoc copy:
https://cr.openjdk.org/~jlaskey/templates/docs/api/java.base/java/util/FormatProcessor.html
src/java.base/share/classes/java/lang/StringTemplate.java line 522:
> 520: * {@link String};
> 521: * {@snippet :
> 522: * Processor jsonProcessor = st -> new
> JSONObject(st.interpolate());
This isn't quite a good example; it defeats the point of string templates, that
injection attacks like `}, "another_key": {` can still happen with JSON.
src/java.base/share/classes/java/lang/runtime/StringTemplateImplFactory.java
line 112:
> 110: }
> 111: interpolateMH = MethodHandles.filterArguments(interpolateMH, 0,
> components);
> 112: mt = MethodType.methodType(String.class,
> StringTemplateImpl.class);
This MethodType can be stored in a static final field than created every time
on the fly. Don't know if JIT compiler can inline this statement. Same fore
that `List.class, StringTemplateImpl.class` type below.
src/java.base/share/classes/java/lang/runtime/TemplateRuntime.java line 109:
> 107: * @param lookup method lookup
> 108: * @param namemethod name
> 109: * @param typemethod type
Maybe mention that the name is ignored, and the type must be convertible from
`(String[], Object[])StringTemplate`? The specification says "fragments list"
and "values list", which are more accurately described as arrays.
Also, does "large" string template mean there are more than 250~ fragments that
the bootstrap method arguments can't fit, or does it mean dynamic number of
fragments? Might be worth mentioning that as well.
-
PR Review: https://git.openjdk.org/jdk/pull/10889#pullrequestreview-1381542020
PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1164313718
PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1164324857
PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1164284857
PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1164278200
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v56]
> 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: Change MAX_INDY_CONCAT_ARG_SLOTS to be updatable. - Changes: - all: https://git.openjdk.org/jdk/pull/10889/files - new: https://git.openjdk.org/jdk/pull/10889/files/62eadb84..6016deb8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10889=55 - incr: https://webrevs.openjdk.org/?repo=jdk=10889=54-55 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/10889.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/10889/head:pull/10889 PR: https://git.openjdk.org/jdk/pull/10889
