<i18n dev> RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v8]

[Jorn Vernee]

On Wed, 2 Nov 2022 17:49:58 GMT, Jim Laskey <jlaskey at 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 22 additional commits since the last revision:
> 
>  - Merge branch 'master' into 8285932
>  - Add @SafeVarargs declarations
>  - Move template bootstrap
>  - Requested changes #2
>  - Requested changes
>  - Remove .orig file
>  - Update TemplateRuntime::combine
>  - Move StringConcatItem to FormatConcatItem
>  - Tabs to spaces
>  - Force processor before template string expression
>  - ... and 12 more: https://git.openjdk.org/jdk/compare/7a2dcaba...6cea084b

src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 118:

> 116:         List<Class<?>> result = new ArrayList<>();
> 117:         Class<?> tsClass = st.getClass();
> 118:         if (tsClass.isSynthetic()) {

This check seems pretty ad-hoc... What happens if a synthetic class is passed that declares some `x0` field that doesn't correspond to the type of the value? i.e. someone could generating an implementation of `StringTemplate`, slap `ACC_SYNTHETIC` on it, and it seems that could have the potential to break this code.

I suggest generating an override of `valueTypes` in the synthetic class emitted by javac instead.

src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 160:

> 158:                 for (int i = 0; ; i++) {
> 159:                     Field field = tsClass.getDeclaredField("x" + i);
> 160:                     MethodHandle mh = JLIA.unreflectField(field, false);

This by-passes any access checks by using `IMPL_LOOKUP`.

This seems problematic. e.g. there's a class that has some `x0` field that I can't access, but the class is not `final`. I generate an impl of `StringTemplate` that extends that class (with `ACC_SYNTHETIC`), and call `valueAccessors` to get a getter that bypasses all access checks.

I think the synthetic class generated by javac should generate an override of this method as well.

src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 175:

> 173:             int size = st.values().size();
> 174:             for (int index = 0; index < size; index++) {
> 175:                 result.add(MethodHandles.insertArguments(GET_VALUE_MH, 0, index));

The returned method handle takes a `StringTemplate` as an argument, but doesn't guard against the fact that a different string template instance can be passed than the one used here, which can lead to errors when invoking the method handle (e.g. out of bounds access, or cast failures when the value types are different).

I was gonna suggest using `List::get` as a method handle instead, and then binding the `values()` list and the index to that. That would drop the `StringTemplate` param, which I suppose is important for `javac` generated code? Could add a `dropArguments` for that, but then the argument would always be ignored.

Maybe alternatively, the `getValue` method could do a check that the `StringTemplate` instance passed to it is the same instance as the one used here.

I don't know...

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

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