RFR: JDK-8203338: Unboxing in return from lambda miscompiled to throw ClassCastException

[B. Blaser]

I've attached the patch along with the following test cases:
* lambda without 'return' (coercion + unboxing + widening primitive)
* lambda with 'return' (idem)
* method reference converted to lambda (coercion)
* method with 'return' (idem)

Any comment (tier1 is OK)?
Bernard

On 19 June 2018 at 22:37, Vicente Romero <vicente.romero at oracle.com> wrote:
> Hi,
>
> I agree with Maurizio that another test covering method references would be
> nice. Apart from that I like the patch, thanks for taking the time to test
> this other approach.
>
> Vicente
>
>
> On 06/19/2018 04:02 PM, Maurizio Cimadamore wrote:
>>
>> I was not 100% sure that removing all coerce logic from LambdaToMethod is
>> gonna work - LambdaToMethod is actively rewriting method reference
>> expression as lambdas - and these synthetic lambdas are not seen by
>> TransTypes - see here:
>>
>>
>> http://hg.openjdk.java.net/jdk/jdk/file/tip/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java#l903
>>
>> But luckily, it seems L2M already handles these from inside the
>> MemberReferenceToLambda class:
>>
>>
>> http://hg.openjdk.java.net/jdk/jdk/file/tip/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java#l1051
>>
>> So that should be ok? But I'd be more comfortable if the tests included a
>> case that stressed the method reference path - e.g. an example of a method
>> reference that requires a synthetic cast to be compliant with the target
>> type - just to make sure.
>>
>> Maurizio
>>
>>
>>
>> On 19/06/18 19:31, B. Blaser wrote:
>>>
>>> On 18 June 2018 at 23:09, B. Blaser <bsrbnd at gmail.com> wrote:
>>>>
>>>> On 18 June 2018 at 22:12, Maurizio Cimadamore
>>>> <maurizio.cimadamore at oracle.com> wrote:
>>>>>
>>>>> I think that we should use the (erased) return type of the functional
>>>>> descriptor as the target of the translation. In both the
>>>>> expression/statement case.
>>>>>
>>>>> For statement, why not assigning currentMethod to the full method
>>>>> descriptor
>>>>> type? Or, if we really use currentMethod just for visitReturn, maybe
>>>>> just
>>>>> storing the expected return (as Vicente suggests) is ok?
>>>>>
>>>>> Maurizio
>>>>
>>>> I think this should probably work too but it might have more
>>>> side-effects than the fix I suggested.
>>>> I'll take a look and provide another patch with this variant if all goes
>>>> well...
>>>>
>>>> Bernard
>>>
>>> I've attached the variant you both suggested.
>>> Any comment is welcome (langtools:tier1 is OK)!
>>>
>>> Bernard
>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jdk8203338c.patch
Type: text/x-patch
Size: 5735 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180620/fbf852bf/jdk8203338c.patch>