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