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

[Vicente Romero]

thanks
Vicente

On 1/18/19 9:58 AM, Maurizio Cimadamore wrote:
> 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
>>>>>>>
>>>>>
>>>>
>>