On Oct 17, 2016, at 3:38 PM, Paul Sandoz <paul.san...@oracle.com> wrote:
>> On 17 Oct 2016, at 15:01, Stuart Marks <stuart.ma...@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 
exceptions through one point.  That may make somebody's day easier when placing 

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

Reply via email to