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

[Jan Lahoda]

On Mon, 14 Nov 2022 17:51:24 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 incrementally with one additional commit since the last revision:
> 
>   Wrong line separator

I've looked at the javac part, and added a few mostly minor comments.

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java line 77:

> 75: import static com.sun.tools.javac.code.TypeTag.METHOD;
> 76: import static com.sun.tools.javac.code.TypeTag.VOID;
> 77: import com.sun.tools.javac.code.Types;

This change is probably unnecessary?

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 31:

> 29: import java.util.stream.Collectors;
> 30: 
> 31: import com.sun.tools.javac.code.*;

There seem to be changes in this file that are not related to this patch (import section re-organization, addition of empty lines, but possibly also changes to `makeIndyQualifier`) - could these be, please, undone?

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransLiterals.java line 231:

> 229:         JCVariableDecl valuesVar;
> 230:         List<JCVariableDecl> fields;
> 231:         MethodInfo interpolateMethod;

Some of the field here appear to be unused? Probably `stringTemplateClass`, `fragmentsVar`, `valuesVar`, `fields`, `interpolateMethod`.

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line 700:

> 698:             S.setPrevToken(stringToken);
> 699:         }
> 700:         JCExpression t = F.at(pos).StringTemplate(processor, fragments, expressions);

Should there be something like `toP(...)` around the `F.at...`, to set the end position of the tree?

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line 1487:

> 1485:                                 if (typeArgs != null) return illegal();
> 1486:                                 t = stringTemplate(t);
> 1487:                                 typeArgs = null;

`typeArgs = null;` is unnecessary here, correct?

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line 1715:

> 1713:                     }
> 1714:                     t = stringTemplate(t);
> 1715:                     typeArgs = null;

`typeArgs = null;` unnecessary?

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line 1716:

> 1714:                     t = stringTemplate(t);
> 1715:                     typeArgs = null;
> 1716:                     // Uncomment to not allow follow on DOT

What is the reason for this comment? Seems like it should be removed?

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

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