[lworld] RFR: 8353236: [lworld] Better documentation for Valhalla Unsafe intrinsics [v2]

[Quan Anh Mai]

On Thu, 10 Apr 2025 13:15:15 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   more explanation
>
> src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 94:
> 
>> 92:  * <p>
>> 93:  * By default, usage of all methods in this class exhibits undefined behavior,
>> 94:  * unless otherwise explicitly specified.
> 
> The "by default" and "unless otherwise specified" seems to be redundant.
> 
> Suggestion:
> 
>  * Unless otherwise explicitly specified, all methods in this class can exhibit undefined behavior.
>  * .

I decide to just remove it as each method specifies its behaviours already.

> src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 442:
> 
>> 440:      * effectively final as well as definitely assigned to with the return
>> 441:      * value of this method. The object must also be not assigned to another
>> 442:      * local variable.
> 
> Why this restriction? What assumption is violated if there is a second assignment?

There is none, but it would be easier to specify the restriction on `Unsafe::finishPrivateBuffer` as the only variable referring to the larval object is the one passed into that method. Otherwise, we must say that all variables referring to the object must not be used. This, however, sounds like a runtime constraint while what we really mean is a compile-time constraint. As a result, I think this restriction is harmless but make our life easier.

> src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 447:
> 
>> 445:      * other usage, such as loading from or returning it, is illegal. The only
>> 446:      * exception is the implicit check cast inserted by the compiler on the
>> 447:      * return value of this method. Explicit check casts are not allowed.
> 
> "not allowed"?  But there is no enforcement; unpredictable behavior yes.

Not allowed here in the sense that failure to do so will result in undefined behavior.

> src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 449:
> 
>> 447:      * return value of this method. Explicit check casts are not allowed.
>> 448:      * </ul>
>> 449:      * Illegal usage of this method exhibits undefined behavior even if the
> 
> may or can; but not always.

Illegal usage ALWAYS exhibits undefined behavior, the uncertainty here is that whether this undefined behavior manifests itself as an unexpected operation.

> src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 470:
> 
>> 468:      * <li>After the invocation of this method, the variable that holds the
>> 469:      * argument passed into this method must not be used.
>> 470:      * </ul>
> 
> I would add the exhortation that every field of the value object must have a value.

`Unsafe::makePrivateBuffer` takes a non-larval value object so all of its fields always have a value.

-------------

PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037672390
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037678291
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037679325
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037681806
PR Review Comment: https://git.openjdk.org/valhalla/pull/1425#discussion_r2037683401