RFR 8233920: MethodHandles::tryFinally generates illegal bytecode for long/double return types

[Jorn Vernee]

Hi Claes,

Thanks for the comments!

Updated webrev: http://cr.openjdk.java.net/~jvernee/8233920/webrev.02/

Also, thanks for sponsoring.

Jorn

On 12/11/2019 15:30, Claes Redestad wrote:
> Hi Jorn,
>
> good catch!
>
> Some minor stylistic concerns:
>
> "* = depends on whether the return type is a category 2 type, which 
> takes up 2 stack slots."
>
> While long/double are known to use two stack slots, "category 2 type" 
> isn't used much outside the specification. I'd either simplify to
> "* = depends on whether the return type takes up 1 or 2 stack slots."
> or add a reference to the definition of type categories.
>
> Similarly I think:
>
> boolean returnsCat2Type = returnType == long.class || returnType == 
> double.class;
>
> could be simplified as:
>
> boolean isDoubleWord = basicReturnType.isDoubleWord(); // 
> returnsDoubleWord?
>
> In the test, is it possible to narrow the expected exception to the
> exact type being thrown (T1?) rather than Throwable?
>
> I can sponsor the patch.
>
> /Claes
>
> On 2019-11-12 12:09, Jorn Vernee wrote:
>> Hi,
>>
>> Please review the following patch that fixes handling of long/double 
>> returns for the tryFinally MethodHandle combinator.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233920
>> Webrev: http://cr.openjdk.java.net/~jvernee/8233920/webrev.01/
>> (Testing=tier1)
>>
>> FWIW, I also added some missing close tags to the javadoc in the 
>> InvokerBytecodeGenerator class (IntelliJ was warning about this).
>>
>> As a heads-up; I'm not a committer on the jdk project, so a sponsor 
>> would be needed to push this.
>>
>> Thanks,
>> Jorn
>>