What's wrong
Several array-backed containers enumerate with a bare for (int i = 0; i < Count; i++) yield return items[i]; and keep no version counter. Changing the collection mid-enumeration therefore never throws InvalidOperationException, which the IEnumerator<T> contract expects. Instead, elements are silently skipped or visited twice.
Affected enumerators:
ContiguousCollection.GetEnumerator (ContiguousCollection.cs ~342)ContiguousSet.GetEnumerator (~325)ContiguousMap.GetEnumerator (~506), and its Keys/Values collections (~683, ~737)OrderedMap (~399, ~499, ~551)InsertionOrderMap (~383, ~441, ~486): these index items[i] directly, so the inner List<T> never gets the chance to throwRingBuffer (~284)
The List<T>-backed siblings (InsertionOrderCollection, InsertionOrderSet, OrderedCollection, OrderedSet) do throw, so behaviour is inconsistent across the library.
Repro
var c = new ContiguousCollection<int> { 1, 2, 3, 4 };
foreach (var x in c) c.Remove(x);
- Actual: no exception, and
c still contains 2, 4.
- Expected:
InvalidOperationException, which is what InsertionOrderCollection throws for the same code.
ContiguousSet, ContiguousMap, OrderedMap and InsertionOrderMap behave the same way (for the maps, remove kv.Key). I confirmed all five in a scratch console app against the current main. This is the underlying cause of the #65 ExceptWith(self) symptom, which was fixed locally rather than at the enumerator.
Why it matters
A "remove while iterating" mistake produces wrong data with no error, and only for some container types. Code that works (by throwing) on one container gives quietly wrong results after switching to its faster contiguous sibling.
Suggested fix / acceptance criteria
- Add a private
int version to each affected type and increment it on every structural or value change: Add, Insert, Remove, RemoveAt, Clear, the indexer set, and for RingBuffer PushBack/Resize/Resample.
- Each enumerator, including the map
Keys/Values views, captures the version at its start and throws InvalidOperationException when it changes.
- Add tests matching the existing List-backed behaviour for each type.
RingBuffer is commonly written to in real time, so adding the check there is a behaviour change; keep it out of scope if that is preferred.
What's wrong
Several array-backed containers enumerate with a bare
for (int i = 0; i < Count; i++) yield return items[i];and keep no version counter. Changing the collection mid-enumeration therefore never throwsInvalidOperationException, which theIEnumerator<T>contract expects. Instead, elements are silently skipped or visited twice.Affected enumerators:
ContiguousCollection.GetEnumerator(ContiguousCollection.cs~342)ContiguousSet.GetEnumerator(~325)ContiguousMap.GetEnumerator(~506), and itsKeys/Valuescollections (~683, ~737)OrderedMap(~399, ~499, ~551)InsertionOrderMap(~383, ~441, ~486): these indexitems[i]directly, so the innerList<T>never gets the chance to throwRingBuffer(~284)The
List<T>-backed siblings (InsertionOrderCollection,InsertionOrderSet,OrderedCollection,OrderedSet) do throw, so behaviour is inconsistent across the library.Repro
cstill contains2, 4.InvalidOperationException, which is whatInsertionOrderCollectionthrows for the same code.ContiguousSet,ContiguousMap,OrderedMapandInsertionOrderMapbehave the same way (for the maps, removekv.Key). I confirmed all five in a scratch console app against the currentmain. This is the underlying cause of the #65ExceptWith(self)symptom, which was fixed locally rather than at the enumerator.Why it matters
A "remove while iterating" mistake produces wrong data with no error, and only for some container types. Code that works (by throwing) on one container gives quietly wrong results after switching to its faster contiguous sibling.
Suggested fix / acceptance criteria
int versionto each affected type and increment it on every structural or value change: Add, Insert, Remove, RemoveAt, Clear, the indexer set, and forRingBufferPushBack/Resize/Resample.Keys/Valuesviews, captures the version at its start and throwsInvalidOperationExceptionwhen it changes.RingBufferis commonly written to in real time, so adding the check there is a behaviour change; keep it out of scope if that is preferred.