RFR: 8261205: AssertionError: Cannot add metadata to an intersection type [v2]

[Vicente Romero]

On Tue, 18 May 2021 21:55:18 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> Please review this PR which is adding a field to `JCVariableDecl`, the need I see for this is that when we declare a local variable using `var` as in:
>> 
>> 
>> import java.lang.annotation.ElementType;
>> import java.lang.annotation.Target;
>> 
>> @Target({ElementType.TYPE_USE, ElementType.LOCAL_VARIABLE})
>> @interface A {}
>> 
>> class Test {
>>     void t() {
>>         @A var c = g(1, 1L);
>>     }
>> 
>>     <X> X g(X a, X b) {
>>         return a;
>>     }
>> }
>> 
>> the only clue the compiler has to know that the local variable was declared using `var` is that field `JCVariableDecl::vartype` is null, but this field is set to an inferred type at some point during the compilation and from that point on there is no way to know for sure that the original AST had a `var` or not. Unfortunately there is code that keeps asking if the local variable was implicitly typed or not, after field `JCVariableDecl::vartype` has been written, and thus making uninformed decisions as in: TypeAnnotations.TypeAnnotationPositions::visitVarDef, where the compiler is implementing a portion of section: `9.7.4 Where Annotations May Appear` of the spec (JLS16). In particular the portion that defines how to deal with annotations applied to local variables declared with `var`.
>> 
>> The test case above is interesting because the same annotation works as a declaration annotation **and** as a type annotation. If it were only a type annotation then the compiler would have issued an error before getting to `TypeAnnotations.TypeAnnotationPositions::visitVarDef`, precisely at `Check::validateAnnotation`, see that `Check::getApplicableTargets` has a special case for local variables declared using `var`. But again given that the annotation in the example above is applicable as a type and as a declaration annotation, it will seep down to the metadata info associated to the local variable symbol, and that is fine. What is not kosher is that as the compiler has no clue that the AST was originally declared using `var` that it then tries to type-annotate the type of the local variable. This change is thus making sure that that information is preserved.
>> 
>> This patch is also renaming a method in `JCVariableDecl`, its previous name was `isImplicitlyTyped` which was basically checking if the `vartype` field was `null` and was renamed to `nullVarType`, `hasNullVarType` could be an option too, and a new method was added: `declaredUsingVar` which won't change its returned value during the lifetime of a JCVariableDecl AST.
>> 
>> TIA
>
> Vicente Romero has updated the pull request incrementally with one additional commit since the last revision:
> 
>   changes after offline comments

I have uploaded a new commit, after an offline comment from Maurizio, he is correct that: given that lambda parameters also have a null type, they will be classified as "declared with var". This commit gets back to the original method name `JCVarDeclaration::isImplicitlyTyped` which will be `true` for local variables declared with `var` and for parameters of implicit lambdas.

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/MemberEnter.java line 47:

> 45: import static com.sun.tools.javac.code.Kinds.Kind.*;
> 46: import static com.sun.tools.javac.code.TypeTag.TYPEVAR;
> 47: import static com.sun.tools.javac.tree.JCTree.Tag.VARDEF;

the import removal changes have been introduced by intelliJ

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

PR: https://git.openjdk.java.net/jdk/pull/4095