feat(primitives): add SortedSpan — merge-based set algebra over sorted spans - #342

Coverage

Re: the four low-confidence findings on the merge mismatch branches

The second review round raised no inline comments but its body carries four suppressed findings, all making the same claim: MergeIntersect and MergeCount should call SkipEqual in their x < y / y < x branches — as MergeUnion and MergeExcept do — to "reduce the number of comparisons for duplicate-heavy inputs".

It is a performance claim, so I measured it rather than argued it. Both variants, interleaved round by round with the leading variant alternated and medians taken over 41 rounds, so warm-up and cache ordering cancel (a naive back-to-back run reverses its own verdict when you swap the order — which is how this nearly went the other way):

So: a wash on the two shapes the suggestion targets, and a 66% regression on the one where the mismatch branch matters most.

The reason the premise does not hold is that the two conditions pull apart. On duplicate-heavy input both sides contain nearly every value, so almost every step takes the equality branch — which already skips runs — and the mismatch branch barely executes; there is nothing there to save. The shape that lives entirely in the mismatch branch is the distinct/disjoint one, and there SkipEqual's extra equality test per element can never succeed, so it is pure added cost.

The apparent inconsistency with MergeUnion / MergeExcept is also not one: those two emit in the mismatch branch, so they must skip the run or they would write the same value twice. MergeIntersect / MergeCount emit nothing there, so they carry no such obligation.

Not changing the code. I have added the measurement and the reasoning as a comment on the merge section in 8416ada, so the next reader — human or reviewer — does not have to rediscover it.

Both suppressed findings from the third round were right, and they are fixed in 882e20b.

AssertSorted tested span[i] < span[i - 1], which is false in both directions for a NaN pair — so a span containing NaN sailed straight past the assertion whose entire job is to catch precondition violations, and past the one violation the XML docs specifically name as how a floating-point span breaks the contract. It now tests !(span[i - 1] <= span[i]), which catches a descending pair and an unordered one alike, and the message reads "is not ordered after" rather than "is smaller" so it stays accurate for both.

Still Debug-only — the method is [Conditional("DEBUG")], so Release elides the call and its O(n) scan exactly as before, and there is no test driving the failing path because a tripped Debug.Assert tears down the test host (which is also why the method carries [ExcludeFromCodeCoverage]).

Re-verified locally after the change: 5289 tests pass and SortedSpan still measures 100% line and 100% branch coverage.

Round four: one taken, one measured and declined

CHANGELOG length — fair, fixed in a8c3b43. The four bullets carried the full pitch plus every supporting detail, and release.yml extracts the whole version section verbatim as the release body — a section this repo has overrun before. Same facts, ~300 characters shorter, still one bullet per rollout facet so the change stays reconstructible from the changelog alone.

Debug.Assert allocating its message on the passing path — not correct, no change. The claim is that the interpolated message is built even when the assertion holds. It is not: since .NET 6 Debug.Assert has an interpolated-string-handler overload, and the compiler binds to it, so the holes are never evaluated unless the condition is false.

Verified rather than asserted — a net8.0 Debug build, matching the floor TFM:

interpolation hole evaluated 0 time(s) on a PASSING assert
allocated over 100k passing asserts: 0 bytes
overload: Boolean, String
overload: Boolean, AssertInterpolatedStringHandler&

The probe method inside the interpolation hole is called zero times and 100,000 passing asserts allocate zero bytes, because overload resolution picks Assert(bool, ref AssertInterpolatedStringHandler) over Assert(bool, string). The element -1 detail in the same comment is moot for the same reason — that string is never constructed. And in Release none of it exists at all: AssertSorted is [Conditional("DEBUG")], so the call and its O(n) scan are elided at the call site.

Round five was right on all three counts — same finding stated three times — and it is fixed in aa6477a.

destination aliasing an input was genuinely undocumented, and the failure mode is the nasty kind: the merge writes its result while it is still reading both sources, so an overlapping buffer can overwrite elements that have not been consumed yet and return a wrong answer with no error — exactly the silent-wrongness the sortedness precondition is written up so carefully to prevent. A caller could reasonably have read the destination-sizing note and assumed in-place use was supported.

The requirement is now stated in three places (the type remarks alongside the ordering precondition, every destination parameter, and the "Contract and special cases" list in docs/api/utilities.md) and, more usefully, enforced the same way ordering is: a Debug-only AssertNoOverlap over the BCL MemoryExtensions.Overlaps, inside a [Conditional("DEBUG")] helper so Release elides the call and the check entirely.

Verified after the change: 5289 tests pass, the fuzz target passes 3,000 cases, and SortedSpan still measures 100% line and 100% branch — no existing test, fuzz case, benchmark or AOT smoke path was relying on an aliasing destination.

Benchmarks

43 regressions ⚠️ vs main, 43 improvements ✅ (rows past ±10% beyond noise).

Highlights