RFR 8166974: invokedynamic implementation should not wrap Errors

[John Rose]

On Oct 17, 2016, at 3:38 PM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
> 
> 
>> On 17 Oct 2016, at 15:01, Stuart Marks <stuart.marks at oracle.com> wrote:
>> 
>> Hi Paul,
>> 
>> I took a look at the jdk changes. They look good to me.
>> 
>> One section of code gave me pause, which is the throw of ClassCastException at 339 of CallSite.java, and the throw of the exception returned from wrongTargetType() at 344 of CallSite.java. This appears odd given the "rethrow any Error and wrap anything else in BSME" rule. But these throws are caught by the catch of Throwable below, which does the wrap and throw of BSME.
>> 
>> This arrangement was already present in the code.
>> 
>> Usually I wrinkle my nose at a throw that's caught by a catch clause later on, but in this case it's not obvious what would be better. Maybe a comment is warranted?
> 
> In addition to the
> 
>  // See the "Linking Exceptions" section for the invokedynamic
>  // instruction in JVMS 6.5.
> 
> I can add something like:
> 
>  // Throws a runtime exception defining the cause that is then later wrapped in BootstrapMethodError

I agree.  Maybe s/later/in the "catch (Throwable ex)" a few lines below"/, to be more specific.

I think the throw-to-catch is good here, unusually, because it funnels all BSME-wrapped
exceptions through one point.  That may make somebody's day easier when placing breakpoints.

The upgraded BootstrapMethodErrorTest.java is really, really good.

Overall, I am *delighted* to see this little mess cleaned up.  It makes indy linkage approximately as
transparent to errors as the other invocation instructions, making it a better replacement for them.

Thanks a million!  Reviewed.

— John