RFR JDK-8215470: Bad EnclosingMethod attribute on classes declared in lambdas

[Maurizio Cimadamore]

Looks good

Maurizio

On 18/01/2019 14:56, Vicente Romero wrote:
> Hi Maurizio,
>
> Thanks for your comments, what about now?
>
> Vicente
>
> On 1/18/19 9:43 AM, Maurizio Cimadamore wrote:
>> Hi,
>> i think when you say "javac is not in sync with the spec" that's a 
>> bit harsh. There are two possible enclosing methods - the synthetic 
>> lambda and the 'real' one. There's nothing in the spec suggesting 
>> that the enclosingMethod attribute should point one way or another.
>>
>> Also not 100% on 'minimal' as compatibility risk - while it's true 
>> that there's no official commitment one way or another, this change 
>> will result in behavioral changes with core reflection. Also, I don't 
>> think I can think of another case where EnclosingMethod is set to 
>> something so that a real classfile method is 'skipped' over.
>>
>> Maurizio
>>
>> On 18/01/2019 13:32, Vicente Romero wrote:
>>> Hi,
>>>
>>> I also need a review on the CSR: 
>>> https://bugs.openjdk.java.net/browse/JDK-8217272
>>>
>>> Thanks,
>>> Vicente
>>>
>>> On 1/17/19 1:50 PM, Vicente Romero wrote:
>>>>
>>>>
>>>> On 1/17/19 1:31 PM, Maurizio Cimadamore wrote:
>>>>> Looks good - all tests clear despite the removals?
>>>>
>>>> yes on my local, anyway I will do a test on all platforms before 
>>>> pushing
>>>>>
>>>>> Maurizio
>>>> Thanks,
>>>> Vicente
>>>>>
>>>>> On 17/01/2019 02:15, Vicente Romero wrote:
>>>>>> Hi Maurizio,
>>>>>>
>>>>>> Thanks for your comments. What about [1]?
>>>>>>
>>>>>> Vicente
>>>>>>
>>>>>> [1] 
>>>>>> http://cr.openjdk.java.net/~vromero/8215470/webrev.01/jdk.dev.patch
>>>>>>
>>>>>> On 1/16/19 4:05 PM, Maurizio Cimadamore wrote:
>>>>>>> The patch looks good - but I have some questions; sometime ago 
>>>>>>> some logic was added to LambdaToMethod to handle type variables 
>>>>>>> - see LambdaSymbolKind.TypeVar and related code.
>>>>>>>
>>>>>>> I have the vague feeling that this code was added precisely for 
>>>>>>> this reason: to fixup issues related to missing type-variable in 
>>>>>>> the enclosing method. If now you point to the correct method (as 
>>>>>>> per source code), wouldn't this code be redundant? Doing some 
>>>>>>> analysis on the code (hg annotate) I was able to get to this bug:
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8005653
>>>>>>>
>>>>>>> Looking at the synopsis, I see an inner class with an enclosing 
>>>>>>> method - and a dandling type variable reference... I think this 
>>>>>>> is overlapping with the issue you are trying to fix.
>>>>>>>
>>>>>>> Also, I think it would be nice to have a test which uses 
>>>>>>> reflection to poke at the type variables, and checks that the 
>>>>>>> runtime values of the type-variables in the local class are 
>>>>>>> indeed the same as the ones in the enclosing method.
>>>>>>>
>>>>>>> Maurizio
>>>>>>>
>>>>>>> On 16/01/2019 19:55, Vicente Romero wrote:
>>>>>>>> Please review fix for [1] at [2]. The fix is about pointing the 
>>>>>>>> enclosing method attribute of inner classes inside a lambda; to 
>>>>>>>> the original method in which the inner class was declared. 
>>>>>>>> Currently javac is pointing it to the synthetic method that 
>>>>>>>> contains the lambda implementation. Please review the CSR too,
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Vicente
>>>>>>>>
>>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8215470
>>>>>>>> [2] http://cr.openjdk.java.net/~vromero/8215470/webrev.00/
>>>>>>>> [3] https://bugs.openjdk.java.net/browse/JDK-8217272
>>>>>>
>>>>
>>>
>