RFR: 8177466: Add compiler support for local variable type-inference
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Wed Sep 20 21:14:40 UTC 2017
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 - 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.
>
> 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.
>
> 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
>>>
>>>
>>
>
More information about the compiler-dev
mailing list