RFR: 8308167: SequencedMap::firstEntry throws NPE when first entry has null key or value

[Brent Christian]

On Fri, 2 Jun 2023 01:12:32 GMT, Stuart Marks <smarks at openjdk.org> wrote:

> Create and use new NullableKeyValueHolder class to accommodate map entries whose key or value might be null.

Changes look good.
    
An observation:
`TreeMap` implements `SequencedMap`, and I see that its `firstEntry()` and related methods use `AbstractMap.SimpleImmutableEntry` (via `exportEntry()`), despite it being serializable. However, this is long-standing code (from 1.6, perhaps?), and `TreeMap` is itself serializable. So, leaving it as is seems the right thing to do.

src/java.base/share/classes/jdk/internal/util/NullableKeyValueHolder.java line 65:

> 63:  * Map.of might still need to reject nulls, and so would Map.ofEntries) but allowing
> 64:  * a Map.Entry itself to contain nulls seems beneficial in general. If this is done,
> 65:  * merging KeyValueHolder and NullableKeyValueHolder should be reconsidered.

I think having this background and explanation in the docs for this internal class is fine.
IMO, this information would also be useful to have in the bug report.

-------------

Marked as reviewed by bchristi (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14278#pullrequestreview-1463441861
PR Review Comment: https://git.openjdk.org/jdk/pull/14278#discussion_r1218591121