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

[Claes Redestad]

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
>