> On Jun 2, 2016, at 6:54 PM, Mandy Chung <mandy.ch...@oracle.com> wrote: > >> >> On Jun 2, 2016, at 4:23 PM, Brent Christian <brent.christ...@oracle.com> >> wrote: >> >> Hi, >> >> The docs for StackWalker.getCallerClass() state, "Reflection frames, >> MethodHandle and hidden frames are filtered regardless of the >> SHOW_REFLECT_FRAMES and SHOW_HIDDEN_FRAMES options." >> >> However getCallerClass() is sensitive to the SHOW_*_FRAMES options, and >> calling getCallerClass() on a StackWalker with those options can return >> classes that should be filtered out, e.g.: >> sun.reflect.NativeMethodAccessorImpl >> and >> Test$InnerClass$$Lambda$1/1018937824 >> >> StackStreamFactory.CallerClassFinder needs to skip reflection frames when >> consuming frames, a 1-line fix. I updated the test to cover this issue and >> added a lambda case, and also did some light grammar/punctuation >> improvements on the getCallerClass JavaDoc. >> >> The webrev is here: >> http://cr.openjdk.java.net/~bchristi/8152893/webrev.00/ > > Looks fine. Thanks for doing the touchup on javadoc. > > Nit: on GetCallerClassTest.java: > > 178 Class<?> c = (Class<?>) walker.getCallerClass(); > > The cast should not be needed. Any reason there? > > 184 } catch (Throwable e) { > > I suspect this copied from the reflectiveGetCallerClass. Anyway, would it be > simpler doing this: > > Runnable lambdaRunnable = () -> { > GetCallerClassTest.staticGetCallerClass(walker, this.getClass(), > expectUOE); > }; > lambdaRunnable.run();
This would add a frame that’s not what this intends to test. I wonder if you could move the lambdaRunnable to a different class that the test will fail without your fix. Mandy
