On Mon, 8 Jul 2024 20:39:38 GMT, Chen Liang <[email protected]> wrote:
>> Liam Miller-Cushon has updated the pull request with a new target base due
>> to a merge or a rebase. The incremental webrev excludes the unrelated
>> changes brought in by the merge/rebase. The pull request contains six
>> additional commits since the last revision:
>>
>> - Merge remote-tracking branch 'origin/master' into
>> JDK-8328821-make-clear-consistent
>> - Check m.entrySet().hashCode() in MOAT
>> - Merge remote-tracking branch 'origin/master' into
>> JDK-8328821-make-clear-consistent
>> - Use AbstractImmutableSet
>> - Throw UOE for all Map.of().entrySet() mutator methods
>> - 8328821: Make the ImmutableCollections clear() call consistent
>>
>> Without overriding clear(), a call to it in an empty map would
>> just return, as iterator.hasNext() would be false. However if
>> calling Map.of().clear() throws an exception. To make the
>> behavior of Map.of().entrySet().clear() consistent, we need to
>> have an implementation of clear() for the entry set that throws
>> as well.
>
> test/jdk/java/util/Map/MapFactories.java line 505:
>
>> 503:
>> 504: @Test(expectedExceptions=UnsupportedOperationException.class)
>> 505: public void immutableEntrySetAddAllDisallowed() {
>
> Looking back at MOAT, do you think we should add these into MOAT?
> https://github.com/openjdk/jdk/blob/598af2e51464be089b64da4024e62865c2c6ec72/test/jdk/java/util/Collection/MOAT.java#L594-L619
>
> We just need to add calls to `testMapMutatorsAlwaysThrow` and
> `testEmptyMapMutatorsAlwaysThrow` to check
> `test(Empty)CollMutatorsAlwaysThrow(map.entrySet());`,
> `test(Empty)CollMutatorsAlwaysThrow(map.keySet());`, and
> `test(Empty)CollMutatorsAlwaysThrow(map.values());`
`testCollMutatorsAlwaysThrow` expects a `Collection<Integer>` (not e.g. a
`Collection<Entry<Integer, Integer>>`). MOAT could be refactored to handle that
case. Do you think that's worth it, or have thoughts about what the cleanest
way to do that would be?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18522#discussion_r1670899976
