Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Ord violation help #128273

Merged
merged 6 commits into from
Aug 10, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 56 additions & 40 deletions library/alloc/src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,25 @@ impl<T> [T] {
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*n* \* log(*n*))
/// worst-case.
///
/// If `T: Ord` does not implement a total order the resulting order is unspecified. All
/// original elements will remain in the slice and any possible modifications via interior
/// mutability are observed in the input. Same is true if `T: Ord` panics.
/// If the implementation of [`Ord`] for `T` does not implement a [total order] the resulting
/// order of elements in the slice is unspecified. All original elements will remain in the
/// slice and any possible modifications via interior mutability are observed in the input. Same
Voultapher marked this conversation as resolved.
Show resolved Hide resolved
/// is true if the implementation of [`Ord`] for `T` panics.
Voultapher marked this conversation as resolved.
Show resolved Hide resolved
///
/// When applicable, unstable sorting is preferred because it is generally faster than stable
/// sorting and it doesn't allocate auxiliary memory. See
/// [`sort_unstable`](slice::sort_unstable). The exception are partially sorted slices, which
/// may be better served with `slice::sort`.
///
/// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] requires
Voultapher marked this conversation as resolved.
Show resolved Hide resolved
/// additional precautions. For example Rust defines `NaN != NaN`, which doesn't fulfill the
/// reflexivity requirement posed by [`Ord`]. By using an alternative comparison function with
Voultapher marked this conversation as resolved.
Show resolved Hide resolved
/// [`slice::sort_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a [total
/// order] users can sort slices containing floating point numbers. Alternatively, if one can
Voultapher marked this conversation as resolved.
Show resolved Hide resolved
/// guarantee that all values in the slice are comparable with [`PartialOrd::partial_cmp`] *and*
/// the implementation forms a [total order], it's possible to sort the slice with `sort_by(|a,
/// b| a.partial_cmp(b).unwrap())`.
Voultapher marked this conversation as resolved.
Show resolved Hide resolved
///
/// # Current implementation
///
/// The current implementation is based on [driftsort] by Orson Peters and Lukas Bergdoll, which
Expand All @@ -199,18 +209,21 @@ impl<T> [T] {
/// handled without allocation, medium sized slices allocate `self.len()` and beyond that it
/// clamps at `self.len() / 2`.
///
/// If `T: Ord` does not implement a total order, the implementation may panic.
/// # Panics
///
/// May panic if the implementation of [`Ord`] for `T` does not implement a [total order].
///
/// # Examples
///
/// ```
/// let mut v = [-5, 4, 1, -3, 2];
/// let mut v = [4, -5, 1, -3, 2];
///
/// v.sort();
/// assert!(v == [-5, -3, 1, 2, 4]);
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
/// ```
///
/// [driftsort]: https://github.com/Voultapher/driftsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[cfg(not(no_global_oom_handling))]
#[rustc_allow_incoherent_impl]
#[stable(feature = "rust1", since = "1.0.0")]
Expand All @@ -222,30 +235,19 @@ impl<T> [T] {
stable_sort(self, T::lt);
}

/// Sorts the slice with a comparator function, preserving initial order of equal elements.
/// Sorts the slice with a comparison function, preserving initial order of equal elements.
///
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*n* \* log(*n*))
/// worst-case.
///
/// The comparator function should define a total ordering for the elements in the slice. If the
/// ordering is not total, the order of the elements is unspecified.
///
/// If the comparator function does not implement a total order the resulting order is
/// unspecified. All original elements will remain in the slice and any possible modifications
/// via interior mutability are observed in the input. Same is true if the comparator function
/// panics. A total order (for all `a`, `b` and `c`):
/// If the comparison function `compare` does not implement a [total order] the resulting order
/// of elements in the slice is unspecified. All original elements will remain in the slice and
/// any possible modifications via interior mutability are observed in the input. Same is true
/// if `compare` panics.
Voultapher marked this conversation as resolved.
Show resolved Hide resolved
///
/// * total and antisymmetric: exactly one of `a < b`, `a == b` or `a > b` is true, and
/// * transitive, `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`.
///
/// For example, while [`f64`] doesn't implement [`Ord`] because `NaN != NaN`, we can use
/// `partial_cmp` as our sort function when we know the slice doesn't contain a `NaN`.
///
/// ```
/// let mut floats = [5f64, 4.0, 1.0, 3.0, 2.0];
/// floats.sort_unstable_by(|a, b| a.partial_cmp(b).unwrap());
/// assert_eq!(floats, [1.0, 2.0, 3.0, 4.0, 5.0]);
/// ```
/// For example `|a, b| (a - b).cmp(a)` is a comparison function that is neither transitive nor
/// reflexive nor total, `a < b < c < a` with `a = 1, b = 2, c = 3`. For more information and
/// examples see the [`Ord`] documentation.
///
/// # Current implementation
///
Expand All @@ -258,21 +260,24 @@ impl<T> [T] {
/// handled without allocation, medium sized slices allocate `self.len()` and beyond that it
/// clamps at `self.len() / 2`.
///
/// If `T: Ord` does not implement a total order, the implementation may panic.
/// # Panics
///
/// May panic if `compare` does not implement a [total order].
///
/// # Examples
///
/// ```
/// let mut v = [5, 4, 1, 3, 2];
/// let mut v = [4, -5, 1, -3, 2];
/// v.sort_by(|a, b| a.cmp(b));
/// assert!(v == [1, 2, 3, 4, 5]);
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
///
/// // reverse sorting
/// v.sort_by(|a, b| b.cmp(a));
/// assert!(v == [5, 4, 3, 2, 1]);
/// assert_eq!(v, [4, 2, 1, -3, -5]);
/// ```
///
/// [driftsort]: https://github.com/Voultapher/driftsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[cfg(not(no_global_oom_handling))]
#[rustc_allow_incoherent_impl]
#[stable(feature = "rust1", since = "1.0.0")]
Expand All @@ -289,9 +294,10 @@ impl<T> [T] {
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*m* \* *n* \* log(*n*))
/// worst-case, where the key function is *O*(*m*).
///
/// If `K: Ord` does not implement a total order the resulting order is unspecified.
/// All original elements will remain in the slice and any possible modifications via interior
/// mutability are observed in the input. Same is true if `K: Ord` panics.
/// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting
/// order of elements in the slice is unspecified. All original elements will remain in the
/// slice and any possible modifications via interior mutability are observed in the input. Same
/// is true if the implementation of [`Ord`] for `K` panics.
///
/// # Current implementation
///
Expand All @@ -304,18 +310,21 @@ impl<T> [T] {
/// handled without allocation, medium sized slices allocate `self.len()` and beyond that it
/// clamps at `self.len() / 2`.
///
/// If `K: Ord` does not implement a total order, the implementation may panic.
/// # Panics
///
/// May panic if the implementation of [`Ord`] for `K` does not implement a [total order].
///
/// # Examples
///
/// ```
/// let mut v = [-5i32, 4, 1, -3, 2];
/// let mut v = [4i32, -5, 1, -3, 2];
///
/// v.sort_by_key(|k| k.abs());
/// assert!(v == [1, 2, -3, 4, -5]);
/// assert_eq!(v, [1, 2, -3, 4, -5]);
/// ```
///
/// [driftsort]: https://github.com/Voultapher/driftsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[cfg(not(no_global_oom_handling))]
#[rustc_allow_incoherent_impl]
#[stable(feature = "slice_sort_by_key", since = "1.7.0")]
Expand All @@ -337,9 +346,10 @@ impl<T> [T] {
/// storage to remember the results of key evaluation. The order of calls to the key function is
/// unspecified and may change in future versions of the standard library.
///
/// If `K: Ord` does not implement a total order the resulting order is unspecified.
/// All original elements will remain in the slice and any possible modifications via interior
/// mutability are observed in the input. Same is true if `K: Ord` panics.
/// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting
/// order of elements in the slice is unspecified. All original elements will remain in the
/// slice and any possible modifications via interior mutability are observed in the input. Same
/// is true if the implementation of [`Ord`] for `K` panics.
///
/// For simple key functions (e.g., functions that are property accesses or basic operations),
/// [`sort_by_key`](slice::sort_by_key) is likely to be faster.
Expand All @@ -356,16 +366,22 @@ impl<T> [T] {
/// In the worst case, the algorithm allocates temporary storage in a `Vec<(K, usize)>` the
/// length of the slice.
///
/// # Panics
///
/// May panic if the implementation of [`Ord`] for `K` does not implement a [total order].
///
/// # Examples
///
/// ```
/// let mut v = [-5i32, 4, 32, -3, 2];
/// let mut v = [4i32, -5, 1, -3, 2, 10];
///
/// // Strings are sorted by lexicographical order.
/// v.sort_by_cached_key(|k| k.to_string());
/// assert!(v == [-3, -5, 2, 32, 4]);
/// assert_eq!(v, [-3, -5, 1, 10, 2, 4]);
/// ```
///
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[cfg(not(no_global_oom_handling))]
#[rustc_allow_incoherent_impl]
#[stable(feature = "slice_sort_by_cached_key", since = "1.34.0")]
Expand Down
Loading
Loading