On Fri, 29 Apr 2022 03:00:40 GMT, Stuart Marks <sma...@openjdk.org> wrote:

>> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch 
>> in the bug report that breaks `IdentityHashMap` now causes several cases in 
>> this new test to fail. There's more that could be done, but the new tests 
>> cover most of the core functions of `IdentityHashMap`. Unfortunately it 
>> seems difficult to merge this with the existing, comprehensive Collections 
>> tests (e.g., MOAT.java) because those tests implicity rely on 
>> `equals()`-based contract instead of the special-purpose `==`-based contract 
>> used by `IdentityHashMap`.
> Stuart Marks has updated the pull request incrementally with two additional 
> commits since the last revision:
>  - Assertions over return values. Some refinement of equals() testing.
>  - Add comment about Map.Entry identity not guaranteed.

Hello Stuart, I had a look at the updates. This looks good to me, thank you for 
the changes. Just a couple of comments/questions that I've included at relevant 

test/jdk/java/util/IdentityHashMap/Basic.java line 50:

> 48: // that a map's entrySet contains exactly the expected mappings. In most 
> cases, keys and
> 49: // values should be compared by identity, not equality. However, the 
> identities of Map.Entry
> 50: // instances from an IdentityHashSet are not guaranteed; the keys and 
> values they contain

I think I understand what you are saying here, but it took me a few reads to 
understand it. More so because of `IdentityHashSet` here, which I think is a 
So what's being stated here is that you cannot do something like:

IdentityHashMap m = new IdentityHashMap();
var e1 = m.entrySet();
var e2 = m.entrySet();

assertSame(e1, e2); // this isn't guaranteed to pass

Did I understand this right?

test/jdk/java/util/IdentityHashMap/Basic.java line 500:

> 498:         Box newKey = new Box(k1a);
> 499:         Box newVal = new Box(v1a);
> 500:         Box r = map.computeIfAbsent(newKey, k -> { called[0] = true; 
> return newVal; });

More of a curiosity than a review comment - I see that various places in this 
PR use a boolean array with one element instead of just a boolean type. Is that 
a personal coding preference or is there something more to it?


PR: https://git.openjdk.java.net/jdk/pull/8354

Reply via email to