> On 17 Oct 2016, at 16:36, John Rose <john.r.r...@oracle.com> wrote: > > 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. >
Thanks, updated in place. > 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. > Another patch will follow soon for the MH/VH linking (and general j.l.invoke code) unduly wrapping Errors. Thanks, Paul. > Thanks a million! Reviewed. > > — John
