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

Reply via email to