On Fri, 1 Aug 2025 15:49:57 GMT, Chen Liang <[email protected]> wrote:
> Provide a general facility for our null check APIs like
> Objects::requireNonNull or future Checks::nullCheck (void), converting the
> existing infrastructure to start tracking from a given stack site (depth
> offset) and a given stack slot (offset value).
src/hotspot/share/interpreter/bytecodeUtils.cpp line 1514:
> 1512: return true;
> 1513: }
> 1514:
Extra new line
src/java.base/share/classes/java/lang/NullPointerException.java line 75:
> 73:
> 74: /// Creates an NPE with a custom backtrace configuration.
> 75: /// The exception has no message if detailed NPE is not enabled.
Don't mix markdown comments with regular javadoc comments, it just looks
inconsistent without adding value.
Use regular // comments.
src/java.base/share/classes/java/lang/NullPointerException.java line 142:
> 140: }
> 141:
> 142: // Methods below must be called in object monitor
This kind of warning should be on every method declaration, if separated from
the method doc, it can be overlooked by future maintainers.
src/java.base/share/classes/java/lang/NullPointerException.java line 146:
> 144: private void ensureMessageComputed() {
> 145: if ((extendedMessageState & (MESSAGE_COMPUTED |
> CONSTRUCTOR_FINISHED)) == CONSTRUCTOR_FINISHED) {
> 146: int stackOffset = (extendedMessageState & STACK_OFFSET_MASK)
> >> STACK_OFFSET_SHIFT;
This would be more readable if private utility methods were added that
encapsulated the shift and masking, both for extraction and construction.
src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java line 637:
> 635: /// Stack offset is the number of non-hidden frames to skip,
> pointing to the null-checking API.
> 636: /// Search slot is the slot where the null-checked value is passed
> in.
> 637: NullPointerException extendedNullPointerException(int stackOffset,
> int searchSlot);
Method should **not** be added to SharedSecrets solely for access by tests.
Tests can use @modules to gain access.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2251763870
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2254483182
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2254486286
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2254494309
PR Review Comment: https://git.openjdk.org/jdk/pull/26600#discussion_r2254508852