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

[Jan Lahoda]

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