RFR: 8356080: Address post-integration comments for Stable Values [v2]

[Chen Liang]

On Fri, 9 May 2025 12:10:38 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR proposes to address comments in the initial PR for Stable Values, which were deferred until after integration.
>> 
>> Unfortunately, this PR shows the total commit history for stable values.
>
> Per Minborg has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 384 commits:
> 
>  - Fix an issue with toString on nested constructs
>  - Merge branch 'master' into jep502-followup
>  - Merge branch 'master' into jep502-followup
>  - Update src/java.base/share/classes/java/lang/StableValue.java
>    
>    Co-authored-by: Chen Liang <liach at openjdk.org>
>  - Simplify furhter
>  - Address comments in PR
>  - Merge master
>  - Remove unused method and add comment
>  - Address comments
>  - Merge branch 'master' into jep502-followup
>  - ... and 374 more: https://git.openjdk.org/jdk/compare/900b3ff7...3eebd504

Looks good, some bikeshedding

src/java.base/share/classes/java/util/ImmutableCollections.java line 140:

> 138:                 }
> 139:                 public <E> List<E> stableList(int size, IntFunction<? extends E> mapper) {
> 140:                     // A stable list is not Serializable so, we cannot return `List.of()` if `size == 0`

Suggestion:

                    // A stable list is not Serializable, so we cannot return `List.of()` if `size == 0`

src/java.base/share/classes/java/util/ImmutableCollections.java line 144:

> 142:                 }
> 143:                 public <K, V> Map<K, V> stableMap(Set<K> keys, Function<? super K, ? extends V> mapper) {
> 144:                     // A stable map is not Serializable so, we cannot return `Map.of()` if `keys.isEmpty()`

Suggestion:

                    // A stable map is not Serializable, so we cannot return `Map.of()` if `keys.isEmpty()`

bikeshedding.

src/java.base/share/classes/java/util/ImmutableCollections.java line 879:

> 877:         public List<E> subList(int fromIndex, int toIndex) {
> 878:             final int size = size();
> 879:             subListRangeCheck(fromIndex, toIndex, size);

Suggestion:

            subListRangeCheck(fromIndex, toIndex, size());

Redundant variable.

src/java.base/share/classes/java/util/ImmutableCollections.java line 1715:

> 1713: 
> 1714:                 // For @ValueBased
> 1715:                 static private <K, V> LazyMapIterator<K, V> of(StableMapEntrySet<K, V> outer) {

Suggestion:

                private static <K, V> LazyMapIterator<K, V> of(StableMapEntrySet<K, V> outer) {

src/java.base/share/classes/java/util/ReverseOrderListView.java line 46:

> 44:     final List<E> base;
> 45:     @Stable
> 46:     final boolean modifiable;

Unfortunately, stable on boolean only works for true - wish we can get trusted or strict final soon :)

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

PR Review: https://git.openjdk.org/jdk/pull/25004#pullrequestreview-2828636051
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2081846330
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2081847091
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2081851704
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2081858151
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2081860387