From 644550f3de4805b3dec6ade4a60ecf48c1d2487b Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Sat, 27 Jul 2024 16:47:10 +0200 Subject: [PATCH] Improve panic message and surrounding documentation for Ord violations The new sort implementations have the ability to detect Ord violations in many cases. This commit improves the message in a way that should help users realize what went wrong in their program. --- .../core/src/slice/sort/shared/smallsort.rs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/library/core/src/slice/sort/shared/smallsort.rs b/library/core/src/slice/sort/shared/smallsort.rs index 5111ed8756bf1..f3b0bf8b018e6 100644 --- a/library/core/src/slice/sort/shared/smallsort.rs +++ b/library/core/src/slice/sort/shared/smallsort.rs @@ -834,9 +834,10 @@ unsafe fn bidirectional_merge bool>( right = right.add((!left_nonempty) as usize); } - // We now should have consumed the full input exactly once. This can - // only fail if the comparison operator fails to be Ord, in which case - // we will panic and never access the inconsistent state in dst. + // We now should have consumed the full input exactly once. This can only fail if the + // user-provided comparison operator fails implements a strict weak ordering as required by + // Ord incorrectly, in which case we will panic and never access the inconsistent state in + // dst. if left != left_end || right != right_end { panic_on_ord_violation(); } @@ -845,7 +846,21 @@ unsafe fn bidirectional_merge bool>( #[inline(never)] fn panic_on_ord_violation() -> ! { - panic!("Ord violation"); + // This is indicative of a logic bug in the user-provided comparison function or Ord + // implementation. They are expected to implement a total order as explained in the Ord + // documentation. + // + // By raising this panic we inform the user, that they have a logic bug in their program. If a + // strict weak ordering is not given, the concept of comparison based sorting makes no sense. + // E.g.: a < b < c < a + // + // The Ord documentation requires users to implement a total order, arguably that's + // unnecessarily strict in the context of sorting. Issues only arise if the weaker requirement + // of a strict weak ordering is violated. + // + // The panic message talks about a total order because that's what the Ord documentation talks + // about and requires, so as to not confuse users. + panic!("user-provided comparison function does not correctly implement a total order"); } #[must_use]