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
>> 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.