RFR: 8177466: Add compiler support for local variable type-inference

[Vicente Romero]

Hi,

Another great piece of work! some high level comments:

at Types:
* new visitors TypeProjection and CaptureScanner would benefit of having 
some documentation added, it should be made clear what (!upward) means 
as there is no explicit reference to the downward projection in the 
code. Instead of passing a boolean, using an enum could be beneficial 
for self documentation. Like enum Projection {UPWARD, DOWNWARD}, the 
enum could have a getInverseProjection() or getComplementProjection() 
method.
     - also probably examples of expected output for a given input could 
be added, but I'm not too strong about this though
     - also could it be beneficial to include some unit tests for them? 
I realize that they need to be public for a jtreg test to be able to 
access them but I think it will be important to check that the upward 
projection is yielding the
       expected result. I'm mostly interested in this code path: "Type 
lower = t.map(this, !upward);" at TypeProjection.mapTypeArgument()

Attr:
why at attribLazyConstantValue you do:
     - variable.sym.type = chk.checkLocalVarType(variable, itype, 
variable.name); and at visitVarDef:
    - v.type = chk.checkLocalVarType(tree, tree.init.type.baseType(), 
tree.name);  // baseType() is invoked in this case


JavacParser:
general question: can't the parser do an early detection of cases like 
"var s = () -> {};"?

Thanks,
Vicente


On 09/19/2017 11:04 AM, Maurizio Cimadamore wrote:
> Hi,
> I have put together a slightly updated webrev:
>
> * as pointed out, one diagnostic used to say '9' instead of '10' (this 
> will likely need to change again because of new release cadence, but 
> good enough for now)
> * the previous webrev contained a spurious added file (TypeHarness) 
> which was caused by a glitch in the lvti branch in the amber repo.
>
> New webrev:
>
> http://cr.openjdk.java.net/~mcimadamore/8177466_v2/
>
> New diags:
>
> http://cr.openjdk.java.net/~mcimadamore/8177466_v2/diags.html
>
> Maurizio
>
>
> On 18/09/17 17:14, Maurizio Cimadamore wrote:
>> Hi,
>> this change adds support for local variable type inference (JEP 286 
>> [1]). A webrev of the change is available here:
>>
>> http://cr.openjdk.java.net/~mcimadamore/8177466
>>
>> The patch is relatively straightforward: implicitly typed locals are 
>> modeled in a similar fashion to implicit lambda parameters: their AST 
>> node is a JCVariableDecl whose 'vartype' field is not set (e.g. null).
>>
>> There are few tricky parts to this changeset:
>>
>> 1) tweak the parser to give 'var' special meaning depending on the 
>> version number and context
>>
>> 2) Add logic in name resolution to catch bad reference to types named 
>> 'var'
>>
>> 3) add logic to map initializer type back to a suitable variable 
>> declared type
>>
>> As for (1), the parser has been extended so as to special case local 
>> variables with special name 'var', so that the type will be left out 
>> of the corresponding AST representing the variable declaration. This 
>> behavior will only affect latest source version.
>>
>> The parser has a number of extra checks to prevent 'var to be used in 
>> places where it does not belong (according to the spec draft [2]); 
>> for instance, declaring a class whose name is 'var' is rejected in 
>> the parser. As a general rule, I tried to implement all such checks 
>> in the parser, as that gives very early and precise feedback about 
>> what's wrong with the code. The changes are implemented in Parser.java.
>>
>> There are however errors which cannot be caught in the parser, and 
>> that's why (2) is needed. Basically, whenever 'var' is used in a 
>> position where it could be either a type or a package name, the 
>> parser can't simply rule that out, so we have to accept the code, and 
>> give an error if, later on, we discover that 'var' was really used in 
>> a type position (see changes in Resolve.java).
>>
>> As far as (3) is concerned, we need to 'uncapture' captured types 
>> from initializers. That means that if we have a 'var' like this:
>>
>> class Foo {
>>     void test() {
>>         var x = getClass().getSuperClass();
>>     }
>> }
>>
>> The initializer type will be something like Class<? super #CAP>, 
>> where #CAP <: Foo
>>
>> In this case, the compiler will project this type back to the less 
>> specific type Class<?>, and use that as the declared type for 'x'. 
>> This logic is defined in Types.java. As this logic is the same logic 
>> needed by jshell to render type of standalone expressions, jshell 
>> class VarTypePrinter has been removed and jshell has been rewired to 
>> point at the (now) official routine in Types. Jshell also needed 
>> several other tweaks to (i) accept 'var' and (ii) to deal with 
>> non-denotable types (intersection types and anonymous class types) 
>> that can be produced by the LVTI machinery (many thanks to Jan for 
>> doing those changes!)
>>
>>
>> As far as testing is concerned, I wrote several tests to check that 
>> the parser was behaving as expected; to check the behavior of the 
>> LVTI inference machinery, I wrote a test harness which leverages 
>> annotation on 'var' so that we can write down assertions such as:
>>
>> @InferredType("java.util.List<? extends java.lang.String>")
>> var s = extString();
>>
>>
>> Regarding compiler diagnostics, for those interested, a comprehensive 
>> list of examples of new diagnostics triggered by the LVTI compiler 
>> can be found here:
>>
>> http://cr.openjdk.java.net/~mcimadamore/8177466/lvti-diags.html
>>
>> Finally, a finder has been added to detect local variable decls whose 
>> declared type can be replaced by 'var' - to enable it, the hidden 
>> option -XDfind=local should be used.
>>
>>
>> Thanks
>> Maurizio
>>
>> [1] - http://openjdk.java.net/jeps/286
>> [2] - http://cr.openjdk.java.net/~dlsmith/local-var-inference.html
>>
>>
>