From 25f7cd4c3cb14a272f6136768164ddbd39b8e83f Mon Sep 17 00:00:00 2001 From: Marius Bughiu Date: Thu, 6 Aug 2026 03:25:53 +0300 Subject: [PATCH 1/2] docs(RadixSort): scope ArgSort's aliasing exception to the overload that throws it The element on ArgSort claimed an ArgumentException when `indices` shares storage with `keys`. SortingGuard.RequireDistinctStorage short-circuits on `typeof(TLeft) == typeof(TRight)`, so for uint, ulong, long, float and double keys against an int index buffer the whole check is a JIT-time constant false and nothing is ever thrown -- verified by calling ArgSort with a reinterpreted alias, which returns normally. That is the deliberate design SortingGuard's own remarks spell out; only the doc overclaimed. It now names the int-keyed overload as the one that throws and states why a reinterpreted alias is out of contract elsewhere. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + src/Celerity.Sorting/RadixSort.cs | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 490ac70..e0bdbd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ All notable changes to Celerity are documented here. This project follows [Keep ### Fixed +- `RadixSort.ArgSort`'s XML docs promised an `ArgumentException` when `indices` shares storage with `keys`, but only the `int`-keyed overload can throw it — the aliasing check is a same-element-type test by design. The doc now says which overload it covers, and why the rest treat a reinterpreted alias as out of contract. Documentation only. - Eight documentation links pointed at anchors that do not exist: seven `CeleritySet` / `SwissSet` references in `docs/api/collections.md` and one in `CHANGELOG.md`. GitHub deletes `<`, `>` and `,` from a heading without substituting a separator, so `CeleritySet<T, THasher>` anchors as `#celeritysett-thasher`, not the `#celerityset-t-thasher` everyone writes. Closes [#339](https://github.com/marius-bughiu/Celerity/issues/339). ## [2.5.0] - 2026-08-02 diff --git a/src/Celerity.Sorting/RadixSort.cs b/src/Celerity.Sorting/RadixSort.cs index 965cf24..24cee7f 100644 --- a/src/Celerity.Sorting/RadixSort.cs +++ b/src/Celerity.Sorting/RadixSort.cs @@ -154,7 +154,13 @@ public static void SortWithScratch(Span keys, Span values, /// /// The keys to rank. Not modified. /// Receives keys.Length indices into , in ascending key order. - /// is shorter than , or shares storage with it. + /// + /// is shorter than . It is also thrown when + /// shares storage with , but only on the + /// -keyed overload: the aliasing check is a same-element-type test, and for + /// every other key type an index buffer can be made to overlap the keys only + /// by reinterpreting one buffer as another type, which is out of contract rather than checked. + /// /// /// The point of an argsort is to avoid moving a wide payload: rank once, then gather. This form /// rents three buffers, so a hot loop that already owns its scratch should copy the keys itself From 1fc280cc23ae7036324e3bf57c124997a6ac8945 Mon Sep 17 00:00:00 2001 From: Marius Bughiu Date: Fri, 7 Aug 2026 20:49:37 +0300 Subject: [PATCH 2/2] docs(RadixSort): clarify ArgSort aliasing contract --- CHANGELOG.md | 2 +- src/Celerity.Sorting/RadixSort.cs | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d89915..1f0955f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ All notable changes to Celerity are documented here. This project follows [Keep ### Fixed - `PartialSort.TopK` now throws `ArgumentException` when its `destination` overlaps its `source`, instead of silently returning a wrong answer and writing to the source it documents as untouched. Disjoint slices of one array are still accepted, matching `RadixSort` and `CountingSort`. -- `RadixSort.ArgSort`'s XML docs promised an `ArgumentException` when `indices` shares storage with `keys`, but only the `int`-keyed overload can throw it — the aliasing check is a same-element-type test by design. The doc now says which overload it covers, and why the rest treat a reinterpreted alias as out of contract. Documentation only. +- Corrected `RadixSort.ArgSort` XML documentation: only its `ReadOnlySpan` overload rejects `indices` that shares storage with `keys`. Documentation only. - Eight documentation links pointed at anchors that do not exist: seven `CeleritySet` / `SwissSet` references in `docs/api/collections.md` and one in `CHANGELOG.md`. GitHub deletes `<`, `>` and `,` from a heading without substituting a separator, so `CeleritySet<T, THasher>` anchors as `#celeritysett-thasher`, not the `#celerityset-t-thasher` everyone writes. Closes [#339](https://github.com/marius-bughiu/Celerity/issues/339). ## [2.5.0] - 2026-08-02 diff --git a/src/Celerity.Sorting/RadixSort.cs b/src/Celerity.Sorting/RadixSort.cs index 24cee7f..dbec107 100644 --- a/src/Celerity.Sorting/RadixSort.cs +++ b/src/Celerity.Sorting/RadixSort.cs @@ -155,17 +155,22 @@ public static void SortWithScratch(Span keys, Span values, /// The keys to rank. Not modified. /// Receives keys.Length indices into , in ascending key order. /// - /// is shorter than . It is also thrown when - /// shares storage with , but only on the - /// -keyed overload: the aliasing check is a same-element-type test, and for - /// every other key type an index buffer can be made to overlap the keys only - /// by reinterpreting one buffer as another type, which is out of contract rather than checked. + /// is shorter than . For + /// , it is also thrown when + /// shares storage with . See remarks for + /// cross-type aliases. /// /// + /// Aliasing is checked only by because its + /// and have the same element type. For the + /// other overloads, making their differently typed spans overlap requires reinterpreting one + /// buffer as another type; that is out-of-contract rather than checked. + /// /// The point of an argsort is to avoid moving a wide payload: rank once, then gather. This form /// rents three buffers, so a hot loop that already owns its scratch should copy the keys itself /// and call with /// an identity index array as the payload — that is exactly what this does. + /// /// public static void ArgSort(ReadOnlySpan keys, Span indices) {