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

[Vicente Romero]

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