Repository navigation
ContiguousSet.AsSpan() / ContiguousMap.AsSpan() return a writable Span that silently desyncs the uniqueness/key index (duplicates in a set, wrong lookups in a map) #78
Description
What's wrong
ContiguousSet<T>.AsSpan() (Containers/ContiguousSet.cs:543) and ContiguousMap<TKey,TValue>.AsSpan() (Containers/ContiguousMap.cs:565) return a mutable Span<T> / Span<Entry> over the backing items array:
public Span<T> AsSpan() => new(items, 0, Count); // ContiguousSet
public Span<Entry> AsSpan() => new(items, 0, Count); // ContiguousMapBoth types also keep a side index: uniquenessSet for the set and keyToIndex for the map. Writing through the span changes items but leaves that index untouched, so the container breaks its own invariants with no error. The doc comments give no warning. ContiguousCollection<T>.AsSpan() is fine because it has no side index.
Reproduction (verified against a local build of main)
Set
var s = new ContiguousSet<int> { 1, 2, 3 };
s.AsSpan()[0] = 2;
// items enumerate as 2,2,3 -> duplicate inside a set
s.Contains(1); // True, although 1 is no longer stored
s.Add(1); // False, so 1 cannot be re-added
s.Remove(2); // items now 2,3
s.Remove(2); // False, but 2 is still enumeratedMap
var m = new ContiguousMap<string,int> { ["a"] = 1, ["b"] = 2 };
m.AsSpan()[0] = new ContiguousMap<string,int>.Entry("b", 99);
// Keys enumerate as b,b
m["a"]; // 99
m.ContainsKey("a"); // TrueAny caller who treats AsSpan() as the fast path for in-place updates, which is what a writable span invites, corrupts the container with no error.
Suggested fix
- Make
AsSpan()onContiguousSetandContiguousMapreturnReadOnlySpan<…>, or remove it and keep onlyAsReadOnlySpan(). This is a breaking public API change, so it needs a major/minor version bump per the repo's versioning. - If in-place value mutation on the map is a use case worth keeping, add a narrow API instead, e.g.
ref TValue GetValueRefOrNullRef(TKey key)in the style ofCollectionsMarshal. It can update values without letting callers rewrite keys.
Acceptance criteria: no public API on ContiguousSet or ContiguousMap can change stored elements or keys without updating uniquenessSet / keyToIndex, and a test covers the scenario above.
Activity
matt-edmondson commentedon Sep 26, 2026
Triage
Contains/Add/Remove/indexer return wrong answers, and nothing raises an error. It stops short of High because it only triggers when a caller writes throughAsSpan(). The read-only paths andContiguousCollection<T>are not affected.ContiguousSet<T>andContiguousMap<TKey,TValue>:AsSpan()against theuniquenessSet/keyToIndexside indexes. Maintainer: @matt-edmondson.foreachsilently skips elements), which is the same kind of problem: the containers don't protect their own invariants against caller mutation. It is not the same defect, so fix it separately.ContiguousMapnear the same code, so merge that first or coordinate to avoid conflicts.Next steps: Changing
AsSpan()to returnReadOnlySpan<…>, or removing it in favour ofAsReadOnlySpan(), breaks the public API and needs a[major]bump. Decide that before any code is written. If you want to keep in-place value updates on the map, aref TValue GetValueRefOrNullRef(TKey key)-style accessor provides them without exposing keys. Turn the reproduction in the issue into the regression test the acceptance criteria ask for.Generated by Claude Code
matt-edmondson commentedon Sep 28, 2026
Decision (maintainer, 2026-09-28)
AsSpan()onContiguousSet<T>andContiguousMap<TKey,TValue>returnsReadOnlySpan<…>.ContiguousMap, in the style ofref TValue GetValueRefOrNullRef(TKey key), so values can still be updated in place without exposing keys.Next reader: implement both changes and turn the reproduction above into regression tests. Merge or coordinate with #75 first, because it touches the same
ContiguousMapcode, and release with[major].Generated by Claude Code