On Thu, 24 Aug 2023 13:49:29 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Mandy Chung has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - fix whitespace >> - move retainClassRef to ClassFrameInfo as a bit set in the flags field >> - fixup the factory methods > > src/java.base/share/classes/java/lang/StackStreamFactory.java line 1096: > >> 1094: c == Constructor.class || >> 1095: MethodAccessor.class.isAssignableFrom(c) || >> 1096: ConstructorAccessor.class.isAssignableFrom(c); > > I guess LambdaForm have the hidden flag on, which is why there's no need to > include them here now? `LambdaForm` are method handle frames which are filtered by `isMethodHandleFrame` > test/jdk/java/lang/StackWalker/SanityTest.java line 120: > >> 118: sw.forEach(StackWalker.StackFrame::isNativeMethod)); >> 119: assertThrows(UnsupportedOperationException.class, () -> >> 120: >> sw.forEach(StackWalker.StackFrame::toStackTraceElement)); > > Should we check that the exception is thrown by each frame? This code will > hit the exception at the first frame and not check the others. I think not necessary since the walker returns the `ClassFrameInfo` for all frames. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15370#discussion_r1304603999 PR Review Comment: https://git.openjdk.org/jdk/pull/15370#discussion_r1304606664