RFR: 8210742: Compound var declaration splitting seems broken

[srinivas]

Hi Maurizio,

Thanks for the review.

Yes - Initially I thought of making this code common to all branches.

Below are the reasons for not doing that.

1) the reported issue from NB team is for simple 'var x' types but not 
for arrays.

2) I don't have enough data to preserve the type for arrays or not and I 
want to avoid breaking any assumption related to arrays error cases

both from NB team and javac tests.

Please let me know if I should include array case as well and refactor 
the fix.

Regards,

Srinivas



On 15/11/18 5:13 PM, Maurizio Cimadamore wrote:
>
> Hi,
> I think what you are doing is ok, but I have some comments:
>
> 1) it seems like this code:
>
>
> startPos = TreeInfo.getStartPos(mods);
> if (startPos == Position.NOPOS)
>    startPos = TreeInfo.getStartPos(type);
>
> should probably be lifted outside the 'if' - e.g. should apply to all 
> cases after the check for "isRestrictedLocalVarTypeName"
>
> 2) A similar argument applies, kind of, to setting type to 'null'. 
> With your patch we do that in 2/3 branches; the branch that is left 
> out is when you have a var declaration like this:
>
> var foo[] = ...
>
> In this case the parser will still attempt to create a var tree with 
> 'var' as its type. If we are worried about how the AST would look in 
> erroneous cases, then I think this is a problem too.
>
> Cheers
> Maurizio
>
> On 15/11/2018 10:31, srinivas wrote:
>> Hi,
>>
>> Please review http://cr.openjdk.java.net/~sdama/8210742/webrev.00/
>>
>> for https://bugs.openjdk.java.net/browse/JDK-8210742.
>>
>> Regards,
>>
>> Srinivas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20181115/5a3da3f7/attachment.html>