On Sat, 8 Apr 2023 15:51:36 GMT, Jim Laskey <jlas...@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: > > 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<JSONObject, RuntimeException> 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 name method name > 109: * @param type method 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
