On Fri, 28 Oct 2022 16:33:11 GMT, Maurizio Cimadamore <mcimadam...@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
