dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Apply review comments 

- Use if the implementation of [`Ord`] for `T`
  language
- Link to total order wiki page
- Rework total order help and examples
- Improve language to be more precise and less
  prone to misunderstandings.
- Fix usage of `sort_unstable_by` in `sort_by`
  example
- Fix missing author mention
- Use more consistent example input for sort
- Use more idiomatic assert_eq! in examples
- Use more natural "comparison function" language
  instead of "comparator function"
This commit is contained in:
Lukas Bergdoll 2024-07-31 11:28:11 +02:00
parent 00ce238885
commit afc404fdfc
4 changed files with 97 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
library/alloc/src/slice.rs
View File

@ -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
/// is true if the implementation of [`Ord`] for `T` panics.
///
/// 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
/// 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
/// [`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
/// 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())`.
///
/// # Current implementation
///
/// The current implementation is based on [driftsort] by Orson Peters and Lukas Bergdoll, which
@ -201,18 +211,19 @@ impl<T> [T] {
///
/// # Panics
///
/// May panic if `T: Ord` does not implement a total order.
/// 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")]
@ -224,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 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.
///
/// 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`):
///
/// * 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
///
@ -262,21 +262,22 @@ impl<T> [T] {
///
/// # Panics
///
/// May panic if `compare` does not implement a total order.
/// 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")]
@ -293,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
///
@ -310,18 +312,19 @@ impl<T> [T] {
///
/// # Panics
///
/// May panic if `K: Ord` does not implement a total order.
/// 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")]
@ -343,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.
@ -364,18 +368,20 @@ impl<T> [T] {
///
/// # Panics
///
/// May panic if `K: Ord` does not implement a total order.
/// 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")]

80
library/core/src/slice/mod.rs
View File

@ -2884,9 +2884,19 @@ impl<T> [T] {
/// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not
/// allocate), 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
/// is true if the implementation of [`Ord`] for `T` panics.
///
/// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] requires
/// 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
/// [`slice::sort_unstable_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 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_unstable_by(|a, b| a.partial_cmp(b).unwrap())`.
///
/// # Current implementation
///
@ -2900,18 +2910,19 @@ impl<T> [T] {
///
/// # Panics
///
/// May panic if `T: Ord` does not implement a total order.
/// 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_unstable();
/// assert!(v == [-5, -3, 1, 2, 4]);
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
/// ```
///
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[stable(feature = "sort_unstable", since = "1.20.0")]
#[inline]
pub fn sort_unstable(&mut self)
@ -2921,31 +2932,20 @@ impl<T> [T] {
sort::unstable::sort(self, &mut T::lt);
}
/// Sorts the slice with a comparator function, **without** preserving the initial order of
/// Sorts the slice with a comparison function, **without** preserving the initial order of
/// equal elements.
///
/// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not
/// allocate), 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 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.
///
/// 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`):
///
/// * 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
///
@ -2959,21 +2959,22 @@ impl<T> [T] {
///
/// # Panics
///
/// May panic if `compare` does not implement a total order.
/// 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_unstable_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_unstable_by(|a, b| b.cmp(a));
/// assert!(v == [5, 4, 3, 2, 1]);
/// assert_eq!(v, [4, 2, 1, -3, -5]);
/// ```
///
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[stable(feature = "sort_unstable", since = "1.20.0")]
#[inline]
pub fn sort_unstable_by<F>(&mut self, mut compare: F)
@ -2989,9 +2990,10 @@ impl<T> [T] {
/// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not
/// allocate), and *O*(*n* \* log(*n*)) worst-case.
///
/// 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
///
@ -3005,18 +3007,19 @@ impl<T> [T] {
///
/// # Panics
///
/// May panic if `K: Ord` does not implement a total order.
/// 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_unstable_by_key(|k| k.abs());
/// assert!(v == [1, 2, -3, 4, -5]);
/// assert_eq!(v, [1, 2, -3, 4, -5]);
/// ```
///
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[stable(feature = "sort_unstable", since = "1.20.0")]
#[inline]
pub fn sort_unstable_by_key<K, F>(&mut self, mut f: F)
@ -3054,7 +3057,7 @@ impl<T> [T] {
///
/// Panics when `index >= len()`, meaning it always panics on empty slices.
///
/// May panic if `T: Ord` does not implement a total order.
/// May panic if the implementation of [`Ord`] for `T` does not implement a [total order].
///
/// # Examples
///
@ -3078,6 +3081,7 @@ impl<T> [T] {
/// ```
///
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[stable(feature = "slice_select_nth_unstable", since = "1.49.0")]
#[inline]
pub fn select_nth_unstable(&mut self, index: usize) -> (&mut [T], &mut T, &mut [T])
@ -3114,7 +3118,7 @@ impl<T> [T] {
///
/// Panics when `index >= len()`, meaning it always panics on empty slices.
///
/// May panic if `compare` does not implement a total order.
/// May panic if `compare` does not implement a [total order].
///
/// # Examples
///
@ -3138,6 +3142,7 @@ impl<T> [T] {
/// ```
///
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[stable(feature = "slice_select_nth_unstable", since = "1.49.0")]
#[inline]
pub fn select_nth_unstable_by<F>(
@ -3202,6 +3207,7 @@ impl<T> [T] {
/// ```
///
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
/// [total order]: https://en.wikipedia.org/wiki/Total_order
#[stable(feature = "slice_select_nth_unstable", since = "1.49.0")]
#[inline]
pub fn select_nth_unstable_by_key<K, F>(

13
library/core/src/slice/sort/shared/smallsort.rs
View File

@ -835,9 +835,8 @@ unsafe fn bidirectional_merge<T: FreezeMarker, F: FnMut(&T, &T) -> bool>(
}
// 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.
// user-provided comparison function fails to implement a strict weak ordering. In that case
// we panic and never access the inconsistent state in dst.
if left != left_end || right != right_end {
panic_on_ord_violation();
}
@ -850,11 +849,11 @@ fn panic_on_ord_violation() -> ! {
// 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
// By panicking 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 cannot yield a sorted
// result. E.g.: a < b < c < a
//
// The Ord documentation requires users to implement a total order, arguably that's
// 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.
//

2
library/core/src/slice/sort/unstable/mod.rs
View File

@ -9,7 +9,7 @@ use crate::slice::sort::shared::smallsort::insertion_sort_shift_left;
pub(crate) mod heapsort;
pub(crate) mod quicksort;
/// Unstable sort called ipnsort by Lukas Bergdoll.
/// Unstable sort called ipnsort by Lukas Bergdoll and Orson Peters.
/// Design document:
/// <https://github.com/Voultapher/sort-research-rs/blob/main/writeup/ipnsort_introduction/text.md>
///