On Fri, 9 May 2025 12:10:38 GMT, Per Minborg <pminb...@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 <li...@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
