Skip to content

Make filtered NavigableMap views return snapshot entries from pollFirstEntry(), etc. (and similarly for ForwardingNavigableMap.standard*Entry). - #8699

Merged
copybara-service[bot] merged 1 commit into
masterfrom
test_987445650
Sep 26, 2026

Conversation

@copybara-service

@copybara-service copybara-service Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Make filtered NavigableMap views return snapshot entries from pollFirstEntry(), etc. (and similarly for ForwardingNavigableMap.standard*Entry).

And update NavigableMapTestSuiteBuilder to require immutable entries from those methods.

For simplicity, I'll focus on the NavigableMap case in the description below:

Maps.filterKeys, filterValues and filterEntries on a NavigableMap returned the backing map's live entries from their navigation methods. This caused two problems:

Removing an Entry from the backing map might mutate the Entry

pollFirstEntry() and pollLastEntry() removed the entry through the backing entrySet iterator and then returned that same entry object.

When TreeMap removes a node with two children, it copies the successor's key and value into that node, so the returned entry could show a different mapping.

For example:

  • Maps.filterKeys(TreeMap{1=one, 2=two, 3=three}, k -> k != 1) .pollFirstEntry() returned 3=three instead of 2=two.
  • Draining filterKeys(TreeMap{0..9}, odd) with pollFirstEntry() returned keys [2, 4, 6, 7, 9] instead of [1, 3, 5, 7, 9].

Navigation methods are supposed to return snapshots

firstEntry(), lastEntry(), ceilingEntry(), floorEntry(), higherEntry() and lowerEntry() returned live entries whose setValue wrote through to the backing map. This is bad for two reasons:

  • NavigableMap specifies that these methods return snapshot entries that don't support setValue.
  • Even if they were to support setValue, maps that are filtered by value (or by the full entry contents) should check that the predicate allows the new value.

Now, all these methods return snapshots.

But we continue to override entrySet() in a way that allows its entries to support setValue (with the predicate check).

Finally, I've moved Iterables.removeFirstMatching from Iterables to Sets. That is now the only location that it's used from, since it is no longer used from Maps. Along the way, I renamed it to "pollFirstMatching" to more closely match the methods that it's used from.

Fixes #8692

I filed b/566253750 for a similar question in SortedMultiset.

RELNOTES=n/a

@copybara-service copybara-service Bot changed the title Make filtered NavigableMap views return snapshot entries from pollFirstEntry(), etc. Make filtered NavigableMap views return snapshot entries from pollFirstEntry(), etc. (and similarly for ForwardingNavigableMap.standard*Entry). Sep 25, 2026
…FirstEntry()`, etc. (and similarly for `ForwardingNavigableMap.standard*Entry`).

And update `NavigableMapTestSuiteBuilder` to require immutable entries from those methods.

For simplicity, I'll focus on the `NavigableMap` case in the description below:

`Maps.filterKeys`, `filterValues` and `filterEntries` on a `NavigableMap` returned the backing map's live entries from their navigation methods. This caused two problems:

### Removing an `Entry` from the backing map might mutate the `Entry`

`pollFirstEntry()` and `pollLastEntry()` removed the entry through the backing `entrySet` iterator and then returned that same entry object.

When `TreeMap` removes a node with two children, it copies the successor's key and value into that node, so the returned entry could show a different mapping.

For example:

- `Maps.filterKeys(TreeMap{1=one, 2=two, 3=three}, k -> k != 1) .pollFirstEntry()` returned `3=three` instead of `2=two`.
- Draining `filterKeys(TreeMap{0..9}, odd)` with `pollFirstEntry()` returned keys [2, 4, 6, 7, 9] instead of [1, 3, 5, 7, 9].

### Navigation methods are supposed to return snapshots

`firstEntry()`, `lastEntry()`, `ceilingEntry()`, `floorEntry()`, `higherEntry()` and `lowerEntry()` returned live entries whose `setValue` wrote through to the backing map. This is bad for two reasons:

- [`NavigableMap`](https://docs.oracle.com/en/java/javase/27/docs/api/java.base/java/util/NavigableMap.html) specifies that these methods return snapshot entries that don't support `setValue`.
- Even if they were to support `setValue`, maps that are filtered by value (or by the full entry contents) should check that the predicate allows the new value.

---

Now, all these methods return snapshots.

But we continue to override `entrySet()` in a way that allows its entries to support `setValue` (with the predicate check).

Finally, I've moved `Iterables.removeFirstMatching` from `Iterables` to `Sets`. That is now the only location that it's used from, since it is no longer used from `Maps`. Along the way, I renamed it to "`pollFirstMatching`" to more closely match the methods that it's used from.

Fixes #8692

I filed b/566253750 for a similar question in `SortedMultiset`.

RELNOTES=n/a
PiperOrigin-RevId: 988966779
@copybara-service
copybara-service Bot merged commit a279b88 into master Sep 26, 2026
3 checks passed
@copybara-service
copybara-service Bot deleted the test_987445650 branch September 26, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants