RFR JDK-8188225: AST could be improved in presence of var types.

[Maurizio Cimadamore]

Thanks for taking care of this, I have a couple of suggestion:

* the routine for generating synthetic type tree should be shared. E.g. 
you need a method that takes a type and generates something that is 
suitable for a local variable or a lambda parameter - e.g.

toSyntheticTypeTree(Type t) {
    if (t.isErroneous()) {
       return make.at(Position.NOPOS).Erroneous();
    } else {
       return make.at(Position.NOPOS).Type(t);
    }
}

* secondly, while adding the startpos tree is fine - here it seems like 
we morally would like to override the getStartPosition for the synthetic 
type trees created above, so that the former position is returned (e.g. 
that of 'var'). So, I wonder if something like this could work:

toSyntheticTypeTree(Type t, int preferredPos) {
    if (t.isErroneous()) {
       return make.at(preferredPos).Erroneous();
    } else {
       return make.at(preferredPos).Type(t);
    }
}

Then, assuming the parser creates the local variable node with the 
correct pos (which it doesn't now), I think everything should work even 
w/o a dedicated field?

Maurizio




On 02/10/17 16:35, Jan Lahoda wrote:
> Hello,
>
> JShell produces not ideal errors related to local variable type 
> inference in some cases:
> ---
> jshell> var i = () -> {};
> |  Error:
> |  incompatible types: java.lang.Object is not a functional interface
> |  var i = () -> {};
> |          ^------^
> ---
>
> Better would be:
> ---
> jshell> var i = () -> {};
> |  Error:
> |  cannot infer type for local variable i
> |    (lambda expression needs an explicit target-type)
> |  var i = () -> {};
> |  ^---------------^
> ---
>
> (which is the error produced by javac)
>
> The AST model could also be improved for local variables whose type 
> have been inferred (which also improves the jshell errors in some cases):
> - for "var i = 0;", the VariableTree.getType() will be null even after 
> attribution, but for: "for (var i : Arrays.asList(0, 1)) {}", the 
> VariableTree.getType() will be filled in by Attr. (The inferred type 
> is also filled in for lambda parameters.) This is not only 
> inconsistent, but also Attr.PostAttrAnalyzer.visitVarDef may fill in 
> the type with an ErroneousTree (with a wrong position). The proposal 
> here is to always fill in the type for consistency, and to 
> consistently use NOPOS for the synthetic type position
>
> -for "var i = 0;" SourcePositions.getStartPosition does not return a 
> proper starting position (the position of "var"), but rather the 
> position of the synthetic type, if any (which can also be 
> ErroneousTree) or the position of the variable name. The proposal here 
> is to remember the real start of the variable and return it.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8188225
> Webrev: http://cr.openjdk.java.net/~jlahoda/8188225/webrev.00/index.html
>
> What do you think?
>
> Thanks,
>     Jan