RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v25]

Maurizio Cimadamore mcimadamore at openjdk.org
Wed Apr 2 09:46:08 UTC 2025


On Wed, 2 Apr 2025 09:13:32 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Per Minborg has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Add lazy toSting for StableMap::values
>>  - Make toString for reversed and sublist lazy
>
> src/java.base/share/classes/java/lang/StableValue.java line 371:
> 
>> 369: 
>> 370:     /**
>> 371:      * {@return {@code true} if the content was set to the provided {@code value},
> 
> I find this description a bit confusing -- as written it almost looks like a predicate -- e.g. test if this stable value is set with the value I'm providing. But the important thing here is the side-effect -- so I feel the javadoc for this method should be much less about what this method returns, and much more about what the method actually does. (`setOrThrow` seems to be better in this respect)

I also note that, so far, we have always referred to the thing contained in a stable value as "content" (bonus points for that -- as that's also the terminology we settled on in the JEP!). But here (and in other methods) we fall back to `value` -- which is generic, and also a bit confusing with respect to `StableValue` itself.

> src/java.base/share/classes/java/lang/StableValue.java line 416:
> 
>> 414:      * When this method returns successfully, the content is always set.
>> 415:      * <p>
>> 416:      * This method will always return the same witness value regardless if invoked by
> 
> will always return/always returns (more direct)

`witness value` -- what is that? Also, why do we say that here -- does it mean that `orElseThrow` can return different values? Overall I'm not sure what this last section wants to add that wasn't already covered at the beginning by the very clear:

>  The provided {@code supplier} is guaranteed to be invoked at most once if it completes without throwing an exception.

> src/java.base/share/classes/java/lang/StableValue.java line 461:
> 
>> 459:      * An unset stable value has no content.
>> 460:      * <p>
>> 461:      * The returned stable value is not {@link Serializable}.
> 
> I find the claim about serializable a bit odd here. Perhaps in some `implSpec`/`apiNote` at the toplevel?

Same in other factories...

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024427669
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024436050
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024448413


More information about the core-libs-dev mailing list