RFR: JDK-8248641: Trees.getScope returns incorrect results for code inside a rule case

[Vicente Romero]

yep looks good to me, thanks for making the change,

Vicente

On 7/28/20 6:29 AM, Jan Lahoda wrote:
> On 27. 07. 20 19:12, Vicente Romero wrote:
>>
>>
>> On 7/27/20 1:09 PM, Vicente Romero wrote:
>>> looks good, but shouldn't the test include cases for which the body 
>>> is not null?
>>
>> hit the send button too soon, I meant, shouldn't the test explicitly 
>> check that the body is not null for the rest of the cases?
>
> Sure, why not:
> http://cr.openjdk.java.net/~jlahoda/8248641/webrev.02/
>
> Does this look fine?
>
> Thanks,
>     Jan
>
>>>
>>> Vicente
>>>
>>> On 7/27/20 8:46 AM, Jan Lahoda wrote:
>>>> Hi,
>>>>
>>>> The existing patch is always filling out the JCCase.body, even for 
>>>> statement cases (unlike the parser, which leaves the body 
>>>> empty/null for statement cases). While I don't think there's 
>>>> anything directly broken by this, it may be better to keep 
>>>> JCCase.body null for statement cases to keep consistency with the 
>>>> normal non-copied trees.
>>>>
>>>> Updated patch:
>>>> http://cr.openjdk.java.net/~jlahoda/8248641/webrev.01/
>>>>
>>>> What do you think?
>>>>
>>>> Thanks,
>>>>     Jan
>>>>
>>>> On 07. 07. 20 21:15, Vicente Romero wrote:
>>>>> looks good,
>>>>> Vicente
>>>>>
>>>>> On 7/2/20 9:11 AM, Jan Lahoda wrote:
>>>>>> Hi,
>>>>>>
>>>>>> When calling Trees.getScope for a TreePath that is inside a rule 
>>>>>> case (i.e. "case ->"), the results are wrong/do not reflect the 
>>>>>> elements declared inside the enclosing method.
>>>>>>
>>>>>> The reason is that, for rule cases, JCCase tree is keeping 
>>>>>> "body", which represents what is in the source code. And this 
>>>>>> body is wrapped and put into "stats", which are basically the 
>>>>>> statements for an ordinary case. This means that javac can 
>>>>>> typically only work over the "stats". When TreeCopied copies the 
>>>>>> JCCase tree, it will duplicate both "stats" and "body" 
>>>>>> separately, leading to a duplication in trees. And then 
>>>>>> Attr.breakTree points to the requested tree from "body", but that 
>>>>>> one is never attributed, and so the appropriate scope is never 
>>>>>> captured.
>>>>>>
>>>>>> The proposed patch here is to only duplicate "stats", and then 
>>>>>> take appropriate "body" from "stats", leading to only one copy of 
>>>>>> the tree.
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~jlahoda/8248641/webrev.00/
>>>>>>
>>>>>> JBS:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8248641
>>>>>>
>>>>>> How does this look?
>>>>>>
>>>>>> Thanks,
>>>>>>     Jan
>>>>>
>>>
>>