On Fri, 1 Aug 2025 15:49:57 GMT, Chen Liang <li...@openjdk.org> 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

Reply via email to