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

Reply via email to