Skip to content

Remove set items from storage using the set's comparer - #63

Merged
matt-edmondson merged 2 commits into
mainfrom
claude/containers-61-set-remove-comparer
Sep 26, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
claude/containers-61-set-remove-comparer

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #61

Problem

ContiguousSet<T>.Remove and InsertionOrderSet<T>.Remove removed the item from the comparer-aware uniquenessSet, then searched the ordered backing store with default equality (Array.IndexOf / List<T>.Remove). With a custom comparer such as StringComparer.OrdinalIgnoreCase, Remove("APPLE") on a set holding "Apple" returned true but left "Apple" in storage. It still enumerated, Count stayed the same, and a later Add("apple") stored a duplicate.

Fix

Both methods now find the stored item with uniquenessSet.Comparer:

  • ContiguousSet: a linear scan with the comparer replaces Array.IndexOf
  • InsertionOrderSet: FindIndex with the comparer, then RemoveAt, replaces List<T>.Remove

Tests

I added two regression tests to each of ContiguousSetTests and InsertionOrderSetTests:

  • Remove_WithCustomComparer_RemovesItemEqualOnlyUnderComparer
  • Remove_WithCustomComparer_ThenAdd_KeepsSingleElement

With the fix reverted, all 4 fail. With it, the full suite passes (377/377, net9.0 test project on Linux).

🤖 Generated with Claude Code

https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh


Generated by Claude Code

ContiguousSet.Remove and InsertionOrderSet.Remove removed the item from the
comparer-aware uniqueness HashSet, then searched the ordered backing store
with default equality. With a custom comparer such as OrdinalIgnoreCase,
Remove("APPLE") on a set holding "Apple" dropped the hash entry but left the
stored item, so it still enumerated, Count did not change, and a later Add
stored a duplicate. Both now locate the stored item with uniquenessSet.Comparer.

Fixes #61

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh
Comment thread Containers.Test/InsertionOrderSetTests.cs Fixed
Comment thread Containers.Test/InsertionOrderSetTests.cs Fixed
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit a13cc50 into main Sep 26, 2026
12 checks passed
@matt-edmondson
matt-edmondson deleted the claude/containers-61-set-remove-comparer branch September 26, 2026 09:51
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.

ContiguousSet<T>.Remove and InsertionOrderSet<T>.Remove leave the item in storage when the set has a custom comparer, corrupting the set

2 participants