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

Vicente Romero vicente.romero at oracle.com
Thu Sep 21 00:06:20 UTC 2017 


On 09/20/2017 06:54 PM, Maurizio Cimadamore wrote:
>
>
> On 20/09/17 22:26, Vicente Romero wrote:
>>
>>
>> On 09/20/2017 05:14 PM, Maurizio Cimadamore wrote:
>>> Thanks for the comments - some responses below
>>>
>>>
>>> On 20/09/17 20:36, Vicente Romero wrote:
>>>> 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.
>>> Good idea, I'll play with it a bit
>>>>     - 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()
>>> I think a TypeHarness test would be a natural way to unit test this. 
>>> Not sure though if this needs to be done now or as a follow up
>>
>> right it doesn't have to be right now, it can wait.
>>
>>> - after all, the test harness for LVTI is already providing some 
>>> sort of unit testing of this part, look at this:
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/8177466_v2/test/langtools/tools/javac/lvti/harness/NonDenotableTest.java.html 
>>>
>>>>
>>>> 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
>>>>
>>> As to why there's a baseType in visitVarDef, I believe it is because 
>>> we can get into that code path if a 'var' is not final but its 
>>> initializer is a constant - e.g. :
>>>
>>> var s = "Hello!"
>>>
>>> In this case we don't want the type of 's' to be constant.
>>>
>>> Regarding attribLazyConstantValue, I think it's probably better to 
>>> use baseType() there too. But I will have to double check if lack of 
>>> baseType is creating issues.
>> ok thanks for the clarification, yes I think that we need to 
>> investigate if you need to use baseType() for lazy evaluation too 
>> which should be triggered by:
>>
>> final var s = "Hello!" right?
> Yes, lazy evaluation would happen for final vars.
>>>>
>>>> JavacParser:
>>>> general question: can't the parser do an early detection of cases 
>>>> like "var s = () -> {};"?
>>> I think it's maybe doable, but it gets convoluted if you have casts 
>>> and other things before the lambda.
>>
>> right but if there is a cast then there is no issue as there is a 
>> target, we need to discard only the cases when a "raw" lambda or 
>> method reference is used to initialize a variable with no explicit 
>> type. Again if you consider that code won't benefit from this then 
>> let's let it as it is. But it could be worthy trying to discard ill 
>> formed expressions as soon as possible
> You are right - a cast is the dumbest example I could have come up 
> with :-)
>
> but think of this:
>
> var s = (cond) ? "Hello!" : () -> { }
>
> Again, this is all doable, we could do lookahead, or we could look at 
> the parsed expression after the fact and rule it out as there's a 
> lambda in the middle - but it seemed simpler to simply special case it 
> in the Attr visitors. That way, no matter how deeply nested the lambda 
> is, you will get a consistent error message.

right, yes I think that you are right and Attr should be the best place, 
it was just too tempting to put it in the parser :)

>
> Maurizio

Vicente

>>
>>>>
>>>> Thanks,
>>>> Vicente
>>
>> 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
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

More information about the compiler-dev mailing list