Skip to content

Commit

Permalink
Improve panic message and surrounding documentation for Ord violations
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Voultapher committed Jul 27, 2024
1 parent 3942254 commit 644550f
Showing 1 changed file with 19 additions and 4 deletions.
23 changes: 19 additions & 4 deletions library/core/src/slice/sort/shared/smallsort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -834,9 +834,10 @@ unsafe fn bidirectional_merge<T: FreezeMarker, F: FnMut(&T, &T) -> 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();
}
Expand All @@ -845,7 +846,21 @@ unsafe fn bidirectional_merge<T: FreezeMarker, F: FnMut(&T, &T) -> 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]
Expand Down

0 comments on commit 644550f

Please sign in to comment.