On Wed, 17 Feb 2021 00:30:09 GMT, Michael Hixson <github.com+297150+michaelhix...@openjdk.org> wrote:
>> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the reference instead of creating a new immutable collection >> that wraps the existing one. > > src/java.base/share/classes/java/util/Collections.java line 1473: > >> 1471: public static <K,V> Map<K,V> unmodifiableMap(Map<? extends K, ? >> extends V> m) { >> 1472: if(m.getClass() == UnmodifiableMap.class || >> 1473: m.getClass() == ImmutableCollections.Map1.class || > > (I'm not a reviewer.) > > I think this causes a change in behavior to this silly program. > > var map1 = Map.of(1, 2); > var map2 = Collections.unmodifiableMap(map1); > > try { > System.out.println("map1 returned " + map1.entrySet().contains(null)); > } catch (NullPointerException e) { > System.out.println("map1 threw"); > } > > try { > System.out.println("map2 returned " + map2.entrySet().contains(null)); > } catch (NullPointerException e) { > System.out.println("map2 threw"); > } > > With JDK 15 the output is: >> map1 threw >> map2 returned false > > With this change I think the output will be: >> map1 threw >> map2 threw > > It seems unlikely that anyone will be bit by this, but it is a change to > behavior and it wasn't called out in the Jira issue, so I felt it was worth > mentioning. > > I think it is just this one specific case that changes -- only `Map1`, and > only `entrySet().contains(null)`. Other sub-collections like `keySet()` and > `values()` and `subList(...)` already throw on `contains(null)` for both the > `ImmutableCollections.*` implementation and the `Collections.umodifiable*` > wrapper. `MapN`'s `entrySet().contains(null)` already returns `false` for > both. This sounds like an inconsistency between `Map1` and `MapN` that should perhaps be considered a bug that needs fixing. /ping @stuart-marks ------------- PR: https://git.openjdk.java.net/jdk/pull/2596