RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v2]

[Jim Laskey]

On Fri, 28 Oct 2022 16:33:11 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Move StringConcatItem to FormatConcatItem
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java line 631:
> 
>> 629:         stringTemplateType = enterClass("java.lang.template.StringTemplate");
>> 630:         templateRuntimeType = enterClass("java.lang.template.TemplateRuntime");
>> 631:         templateProcessorType = enterClass("java.lang.template.ValidatingProcessor");
> 
> Please give it a name that matches the corresponding class - this threw me off when looking at the type-checking code.

Renaming.

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java line 106:
> 
>> 104:     private final Attr attr;
>> 105:     private final Symtab syms;
>> 106:     private final Types types;
> 
> Why this change?

Renaming. Was residual to some context based typing (StringTemplate vs String) Jan and I experimented with.

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 4974:
> 
>> 4972:         if (processor != null) {
>> 4973:             resultType = attribTree(processor, env, new ResultInfo(KindSelector.VAL, Type.noType));
>> 4974:             resultType = chk.checkProcessorType(processor, resultType, env);
> 
> It seems that if this check is erroneous, the type that is considered as returned by the processor is just `StringTemplate`. This seems odd - if we have issues type-checking and we get StringTemplate instead of some type T that the user expects, but doesn't get (e.g. because of raw types), there could be spurious error messages generated from a type mismatch between T and StringTemplate.

Not sure where you get `StringTemplate`.  If you specify `TemplateProcessor<String>` the `resultType` will be `String`.  For example:


public class Processor implements TemplateProcessor<String> {
    @Override
    public String process(StringTemplate st) {
        return st.interpolate();
    }
}

and

public class Main {
    public static void main(String... args) throws Throwable {
        Processor processor = new Processor();
        System.out.println(processor."1234");
    }
}

works with "1234" as a result.

If you later change to


public class Processor implements TemplateProcessor<Integer> {
    @Override
    public Integer process(StringTemplate st) {
        return Integer.valueOf(st.interpolate());
    }
}


Then you get a `java.lang.ClassCastException` as you would expect.

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java.orig line 1:
> 
>> 1: /*
> 
> This file shouldn't be here

oops - We should change the `.gitignore` to include `.orig` and `.rej`

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java line 348:
> 
>> 346:                 try {
>> 347:                     chk.disablePreviewCheck = true;
>> 348:                     String autoImports = """
> 
> I see why you went down here. It is pragmatic, given we might add other stuff to the list. But it is mildly odd to see parser being called again from here, although harmless.
> 
> What worries me more is the dance around enabling/disabling preview checks. Again, I see why you went there.
> 
> As a possible alternative to disable preview checks globally, you could instead install a deferred log handler (see Log) class - so that all the diagnostics generated when following imports can be dropped on the floor. (we use this mechanism in DeferredAttr, to disable diagnostics during a deferred attribution step).

This was recommended by Jan and the procedure used in other parts of the code.

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

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