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