On Thu, 24 Apr 2025 23:53:11 GMT, Luca Kellermann <d...@openjdk.org> wrote:
>> Per Minborg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Make public constuctor private > > src/java.base/share/classes/java/util/ImmutableCollections.java line 898: > >> 896: final StableValueImpl<E>[] reversed = >> ArraysSupport.reverse( >> 897: Arrays.copyOf(delegates, delegates.length)); >> 898: return StableUtil.renderElements(base, "Collection", >> reversed); > > Suggestion: > > return StableUtil.renderElements(this, "StableList", > reversed); > > All other calls use "Stable...". Other view collections (`ArrayList.SubList`, > `ReverseOrderListView`, `AbstractMap.keySet`, `AbstractMap.values`, > `HashMap.EntrySet`) use the view reference (and not the underlying > collection) for detecting self containment, so `renderElements` should use > `this` instead of `base` for `self`. The reason we are using "Collection" is that this method directly overrides `AbstractCollection` which is using "(this Collection)" for first-level circular references. I also think `base` should be replaced with `this`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2063177736