JDK 10/11 RFR of JDK-8189146: Have use of "var" in 9 and earlier source versions issue a warning

[Maurizio Cimadamore]

On 11/01/18 22:52, joe darcy wrote:
> Hi Maurizio,
>
> On 1/9/2018 4:39 PM, Maurizio Cimadamore wrote:
>> Thanks for the update. This is close, but compared to the existing 
>> golden file, as you noticed, there are missing warnings, esp. when 
>> 'var' is used as an array element type.
>>
>> I believe the issue is with this code:
>>
>> if (Feature.LOCAL_VARIABLE_TYPE_INFERENCE.allowedInSource(source) && 
>> elemType.hasTag(IDENT)) {
>> 2998             Name typeName = ((JCIdent)elemType).name;
>> 2999             if (isRestrictedLocalVarTypeName(typeName, pos)) {
>> 3000                 if (type.hasTag(TYPEARRAY)) {
>> 3001                     //error - 'var' and arrays
>> 3002                     reportSyntaxError(pos, 
>> "var.not.allowed.array");
>> 3003                 } else {
>> 3004                     startPos = TreeInfo.getStartPos(mods);
>> 3005                     if (startPos == Position.NOPOS)
>> 3006                         startPos = TreeInfo.getStartPos(type);
>> 3007                     //implicit type
>> 3008                     type = null;
>> 3009                 }
>> 3010             }
>> 3011         }
>>
>>
>> Here, if LVTI is disabled, the 'isRestrictedLocalVarTypeName' call 
>> would not even take place, hence the missing warnings. I suggest to 
>> shuffle as follows:
>>
>> if (elemType.hasTag(IDENT)) {
>> 2998             Name typeName = ((JCIdent)elemType).name;
>> 2999             if (isRestrictedLocalVarTypeName(typeName, pos)) {
>> 3000                 if (type.hasTag(TYPEARRAY)) {
>> 3001                     //error - 'var' and arrays
>> 3002                     reportSyntaxError(pos, 
>> "var.not.allowed.array");
>> 3003                 } else {
>> 3004                     startPos = TreeInfo.getStartPos(mods);
>> 3005                     if (startPos == Position.NOPOS)
>> 3006                         startPos = TreeInfo.getStartPos(type);
>> 3007                     //implicit type
>> 3008                     type = null;
>> 3009                 }
>> 3010             }
>> 3011         }
>
> When I tried that out, there were more warnings; too many actually, 
> there were double warnings issued in some cases:
>
> /test/langtools/tools/javac/lvti/ParserTest.java:38: warning: as of 
> release 10, 'var' is a restricted local variable type and cannot be 
> used for type declarations or as the element type of an array
>     var x = null; //illegal
>     ^
> /test/langtools/tools/javac/lvti/ParserTest.java:38: warning: as of 
> release 10, 'var' is a restricted local variable type and cannot be 
> used for type declarations or as the element type of an array
>     var x = null; //illegal
>         ^
> and
>
> /home/darcy/JDK/10/hg/test/langtools/tools/javac/lvti/ParserTest.java:49: 
> warning: as of release 10, 'var' is a restricted local variable type 
> and cannot be used for type declarations or as the element type of an 
> array
>         var x9 = null, y = null; //illegal
>             ^
> /home/darcy/JDK/10/hg/test/langtools/tools/javac/lvti/ParserTest.java:49: 
> warning: as of release 10, 'var' is a restricted local variable type 
> and cannot be used for type declarations or as the element type of an 
> array
>         var x9 = null, y = null; //illegal
>                        ^
I will think about this a bit more - this (or something very similar) 
should work.

Maurizio
>
> Stepping back a bit, if "var" were added as a full new keyword then 
> the check for a use of "var" could occur within the ident() method, as 
> is done for underscore. However, since "var" is allowed its 
> traditional use in some contexts, the error/warning checks need to be 
> dependent on the syntactic context. For pre-10, only a syntactic check 
> needs occur, e.g. a construct like "var x = 5, y = 6" does not merit a 
> warning about y having a var type; although that check is needed in 10 
> and later. Therefore I moved a check out to the method for the 
> containing grammar production:
>
>     http://cr.openjdk.java.net/~darcy/8189146.3/
>
> I also changed the wording of the warning message slightly to be more 
> inclusive of the situations it was covering. (More precisely worded 
> warning could be given for different situations, but I don't think 
> that is strictly necessary.)
>
> One "var" warning/error check occurs now in typeName, which catches 
> class/interface/enum/annotation/type var declaration usage, and needs 
> to occur in some way for at least certain idents occurring in variable 
> declarations, including in try-with-resources.
>
> I made some updates to the ParserTest to better cover 
> try-with-resources and took a somewhat different tack to attempt to 
> get the right set of warnings issues. Unfortunately, there were 
> duplicates here as well:
>
> /test/langtools/tools/javac/lvti/ParserTest.java:38: warning: as of 
> release 10, 'var' is a restricted local variable type and cannot be 
> used for type declarations or as the element type of an array
>     var x = null; //illegal
>     ^
> /test/langtools/tools/javac/lvti/ParserTest.java:38: warning: as of 
> release 10, 'var' is a restricted local variable type and cannot be 
> used for type declarations or as the element type of an array
>     var x = null; //illegal
>     ^
>
> /test/langtools/tools/javac/lvti/ParserTest.java:50: warning: as of 
> release 10, 'var' is a restricted local variable type and cannot be 
> used for type declarations or as the element type of an array
>         final @DA var x10 = m(); //ok
>                   ^
> /test/langtools/tools/javac/lvti/ParserTest.java:50: warning: as of 
> release 10, 'var' is a restricted local variable type and cannot be 
> used for type declarations or as the element type of an array
>         final @DA var x10 = m(); //ok
>                   ^
> /test/langtools/tools/javac/lvti/ParserTest.java:51: warning: as of 
> release 10, 'var' is a restricted local variable type and cannot be 
> used for type declarations or as the element type of an array
>         @DA final var x11 = m(); //ok
>                   ^
> /test/langtools/tools/javac/lvti/ParserTest.java:51: warning: as of 
> release 10, 'var' is a restricted local variable type and cannot be 
> used for type declarations or as the element type of an array
>         @DA final var x11 = m(); //ok
>
> Hopefully any additional modifications needed will be minor!
>
> Thanks,
>
> -Joe
>