Skip to content

remove on an ordered map builder iterator succeeds after a remove of a different key - #334

Open
DmitryNekrasov wants to merge 1 commit into
fix-set-iterator-remove-external-modificationfrom
fix-ordered-map-iterator-remove-external-modification
Open

remove on an ordered map builder iterator succeeds after a remove of a different key#334
DmitryNekrasov wants to merge 1 commit into
fix-set-iterator-remove-external-modificationfrom
fix-ordered-map-iterator-remove-external-modification

Conversation

@DmitryNekrasov

@DmitryNekrasov DmitryNekrasov commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

remove() was the only method on the ordered map builder's links iterator without the comodification check next() carries. Unlike the two twins it never trips an assert - the builder splices both neighbours on every removal, so the chain stays consistent and the entry is just silently dropped.

Fixes #326

@DmitryNekrasov
DmitryNekrasov marked this pull request as draft August 23, 2026 21:37
@DmitryNekrasov
DmitryNekrasov force-pushed the fix-ordered-map-iterator-remove-external-modification branch from 31cf5e2 to e95f3e9 Compare August 24, 2026 08:47
@DmitryNekrasov
DmitryNekrasov requested a balanced review from Copilot August 24, 2026 08:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds fail-fast behavior to ordered-map builder iterator removal, fixing #326.

Changes:

  • Checks for concurrent modification before iterator removal.
  • Adds regression coverage for entries, keys, values, and size-neutral mutations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
core/commonMain/src/implementations/persistentOrderedMap/PersistentOrderedMapBuilderContentIterators.kt Adds the missing concurrent-modification check.
core/commonTest/src/contract/map/PersistentOrderedMapBuilderTest.kt Tests concurrent mutation scenarios across iterator views.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@DmitryNekrasov DmitryNekrasov self-assigned this Aug 24, 2026
@DmitryNekrasov
DmitryNekrasov marked this pull request as ready for review August 24, 2026 09:05
@DmitryNekrasov
DmitryNekrasov force-pushed the fix-ordered-map-iterator-remove-external-modification branch from e95f3e9 to a79e8ed Compare August 24, 2026 09:09
@DmitryNekrasov DmitryNekrasov added this to the v0.5.2 milestone Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

remove on an ordered map builder iterator succeeds after a remove of a different key

2 participants