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

[Maurizio Cimadamore]

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



On 18/06/18 03:29, Vicente Romero wrote:
> Hi,
>
> Not sure about this one. I think that the type of the return 
> expression inside a lambda should be erased to the erasure of the 
> return type of the lambda type. The `currentMethod` field at 
> TransTypes is just a carrier to actually access the erasure of the 
> return type of the current method, and this access happens only inside 
> visitReturn. So I wonder if we actually should have a field in 
> TransTypes that could hold that erasure directly. At 
> TransTypes.visitMethodDef we could have:
>
> theField = types.erasure(tree.type.getReturnType());
>
> and at TransTypes.visitLambda it could be:
>
> theField = 
> types.erasure(tree.getDescriptorType(types).asMethodType().restype);
>
> of course with a better name, and then at TransTypes.visitReturn, it 
> could use the field to erase the returned expression.
>
> Thanks,
> Vicente
>
> On 06/16/2018 06:20 PM, B. Blaser wrote:
>> Hi,
>>
>> As noted in the JBS issue, the following example fails at run-time 
>> with a CCE:
>>
>>      List.of('x', 'y').stream().max((a, b) -> { return 
>> List.of(a).get(0); });
>>
>> The interesting point here is that removing 'return' makes it run 
>> successfully:
>>
>>      List.of('x', 'y').stream().max((a, b) -> List.of(a).get(0));
>>
>> The problem seems to be in 'TransType.visitReturn()'. The lambda
>> method doesn't yet exist ('currentMethod == null', see
>> 'TransType.visitLambda()') causing the return expression type to be
>> set to the erasure of 'List.get()' (= Object) which further causes the
>> type conversion problem (see
>> 'LambdaToMethod.makeLambdaStatementBody()').
>>
>> The suggested fix (here under) is to keep the original erased
>> expression type (= Character) if 'currentMethod == null'.
>>
>> Tier1 is OK.
>>
>> Any feedback is welcome,
>> Bernard
>>
>> diff -r ed8de3d0cd28
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java
>> --- 
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java
>>     Sat Jun 16 10:10:54 2018 +0100
>> +++ 
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java
>>     Sat Jun 16 21:16:33 2018 +0200
>> @@ -601,7 +601,8 @@
>>       }
>>
>>       public void visitReturn(JCReturn tree) {
>> -        tree.expr = translate(tree.expr, currentMethod != null ?
>> types.erasure(currentMethod.type).getReturnType() : null);
>> +        tree.expr = translate(tree.expr, currentMethod != null ?
>> types.erasure(currentMethod.type).getReturnType() :
>> +                tree.expr != null ? types.erasure(tree.expr.type) : 
>> null);
>>           result = tree;
>>       }
>