RFR: 8341901: Using 'var' keyword switch pattern matching causes compiler error

[Jan Lahoda]

On Thu, 17 Oct 2024 11:14:18 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> Consider code like:
>> 
>> public class T {
>>    record R(N.I i) {}
>>    int test(Object o) {
>>        return switch (o) {
>>            case R(var nested) -> 0;
>>            default -> 0;
>>        };
>>    }
>>    static class N<T> {
>>        interface I {}
>>    }
>> }
>> 
>> 
>> This fails to compile since JDK 23, due to:
>> 
>> $ javac T.java 
>> error: cannot select a static class from a parameterized type
>> 1 error
>> 
>> 
>> The reason for the error is this: the type of `nested` is inferred to `T.N.I`. This is correct. javac will then construct a synthetic AST for it, and the AST will be structurally correct as well: `T.N.I`. But a) the `Type` attached to `T.N` will be `T.N<T>` (which by itself is not correct), and b) after the synthetic AST is created, `Check.validate` is called on the type's AST, and fails, as the types is sees correspond to `T.N<T>.I`, which is illegal.
>> 
>> Note the synthetic AST is also set for local variable type inference, but the `validate` is called *before* the synthetic AST is created.
>> 
>> This PR proposes to do two things:
>> - move the `validate` call before the synthetic AST creation for `visitBindingPattern`, to mimic the behavior for `var`s.
>> - the `TreeMaker` is tweaked to inject erased types instead of parameterized types when generating qualified identifiers for classes or interfaces. This should correspond more closely to what happens when one types `T.N.I` in the source code.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java line 749:
> 
>> 747: 
>> 748:         if (sym.kind == TYP) {
>> 749:             result.setType(types.erasure(sym.type));
> 
> can we do this outside of TreeMaker? I mean TreeMaker could be used to generate trees for a phase for which erasure hasn't occurred yet. It should be the caller's option to decide if the obtained tree should be erased or not

Doing this outside of TreeMaker would be fairly difficult (as that would mean the caller would need to traverse the created tree and clean the types).

This change is not strictly necessary to solve the problem - moving the `chk.validate` is enough. But, it seems to me that type attributes in the AST that are produced by the `TreeMaker.QualIdent` do not make much sense. Specifically, passing the ClassSymbol for `java.util.List` here will produce a tree whose `.type` will be `java.util.List<E>`, where `E` is the type variable from `java.util.List`, which has no meaning outside of `java.util.List`. In general, calling `QualIdent` with a `ClassSymbol`, I don't see how any type other than the erased one would make sense.

That sad - I can easily remove this part of the patch, the AST distortion is fairly small, at least at this time.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21495#discussion_r1804717527