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

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Wed Sep 20 22:54:38 UTC 2017 


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.

Maurizio
>
>>>
>>> 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