Skip to content

Commit 4d74068

Browse files
authored
Unrolled build for rust-lang#128273
Rollup merge of rust-lang#128273 - Voultapher:improve-ord-violation-help, r=workingjubilee Improve `Ord` violation help Recent experience in rust-lang#128083 showed that the panic message when an Ord violation is detected by the new sort implementations can be confusing. So this PR aims to improve it, together with minor bug fixes in the doc comments for sort*, sort_unstable* and select_nth_unstable*. Is it possible to get these changes into the 1.81 release? It doesn't change behavior and would greatly help when users encounter this panic for the first time, which they may after upgrading to 1.81. Tagging `@orlp`
2 parents 8291d68 + 1be60b5 commit 4d74068

File tree

4 files changed

+129
-89
lines changed

4 files changed

+129
-89
lines changed

library/alloc/src/slice.rs

+56-40
Original file line numberDiff line numberDiff line change
@@ -178,15 +178,25 @@ impl<T> [T] {
178178
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*n* \* log(*n*))
179179
/// worst-case.
180180
///
181-
/// If `T: Ord` does not implement a total order the resulting order is unspecified. All
182-
/// original elements will remain in the slice and any possible modifications via interior
183-
/// mutability are observed in the input. Same is true if `T: Ord` panics.
181+
/// If the implementation of [`Ord`] for `T` does not implement a [total order] the resulting
182+
/// order of elements in the slice is unspecified. All original elements will remain in the
183+
/// slice and any possible modifications via interior mutability are observed in the input. Same
184+
/// is true if the implementation of [`Ord`] for `T` panics.
184185
///
185186
/// When applicable, unstable sorting is preferred because it is generally faster than stable
186187
/// sorting and it doesn't allocate auxiliary memory. See
187188
/// [`sort_unstable`](slice::sort_unstable). The exception are partially sorted slices, which
188189
/// may be better served with `slice::sort`.
189190
///
191+
/// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] require
192+
/// additional precautions. For example, `f32::NAN != f32::NAN`, which doesn't fulfill the
193+
/// reflexivity requirement of [`Ord`]. By using an alternative comparison function with
194+
/// `slice::sort_by` such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a [total
195+
/// order] users can sort slices containing floating-point values. Alternatively, if all values
196+
/// in the slice are guaranteed to be in a subset for which [`PartialOrd::partial_cmp`] forms a
197+
/// [total order], it's possible to sort the slice with `sort_by(|a, b|
198+
/// a.partial_cmp(b).unwrap())`.
199+
///
190200
/// # Current implementation
191201
///
192202
/// The current implementation is based on [driftsort] by Orson Peters and Lukas Bergdoll, which
@@ -198,18 +208,21 @@ impl<T> [T] {
198208
/// handled without allocation, medium sized slices allocate `self.len()` and beyond that it
199209
/// clamps at `self.len() / 2`.
200210
///
201-
/// If `T: Ord` does not implement a total order, the implementation may panic.
211+
/// # Panics
212+
///
213+
/// May panic if the implementation of [`Ord`] for `T` does not implement a [total order].
202214
///
203215
/// # Examples
204216
///
205217
/// ```
206-
/// let mut v = [-5, 4, 1, -3, 2];
218+
/// let mut v = [4, -5, 1, -3, 2];
207219
///
208220
/// v.sort();
209-
/// assert!(v == [-5, -3, 1, 2, 4]);
221+
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
210222
/// ```
211223
///
212224
/// [driftsort]: https://github.com/Voultapher/driftsort
225+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
213226
#[cfg(not(no_global_oom_handling))]
214227
#[rustc_allow_incoherent_impl]
215228
#[stable(feature = "rust1", since = "1.0.0")]
@@ -221,30 +234,19 @@ impl<T> [T] {
221234
stable_sort(self, T::lt);
222235
}
223236

224-
/// Sorts the slice with a comparator function, preserving initial order of equal elements.
237+
/// Sorts the slice with a comparison function, preserving initial order of equal elements.
225238
///
226239
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*n* \* log(*n*))
227240
/// worst-case.
228241
///
229-
/// The comparator function should define a total ordering for the elements in the slice. If the
230-
/// ordering is not total, the order of the elements is unspecified.
231-
///
232-
/// If the comparator function does not implement a total order the resulting order is
233-
/// unspecified. All original elements will remain in the slice and any possible modifications
234-
/// via interior mutability are observed in the input. Same is true if the comparator function
235-
/// panics. A total order (for all `a`, `b` and `c`):
242+
/// If the comparison function `compare` does not implement a [total order] the resulting order
243+
/// of elements in the slice is unspecified. All original elements will remain in the slice and
244+
/// any possible modifications via interior mutability are observed in the input. Same is true
245+
/// if `compare` panics.
236246
///
237-
/// * total and antisymmetric: exactly one of `a < b`, `a == b` or `a > b` is true, and
238-
/// * transitive, `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`.
239-
///
240-
/// For example, while [`f64`] doesn't implement [`Ord`] because `NaN != NaN`, we can use
241-
/// `partial_cmp` as our sort function when we know the slice doesn't contain a `NaN`.
242-
///
243-
/// ```
244-
/// let mut floats = [5f64, 4.0, 1.0, 3.0, 2.0];
245-
/// floats.sort_unstable_by(|a, b| a.partial_cmp(b).unwrap());
246-
/// assert_eq!(floats, [1.0, 2.0, 3.0, 4.0, 5.0]);
247-
/// ```
247+
/// For example `|a, b| (a - b).cmp(a)` is a comparison function that is neither transitive nor
248+
/// reflexive nor total, `a < b < c < a` with `a = 1, b = 2, c = 3`. For more information and
249+
/// examples see the [`Ord`] documentation.
248250
///
249251
/// # Current implementation
250252
///
@@ -257,21 +259,24 @@ impl<T> [T] {
257259
/// handled without allocation, medium sized slices allocate `self.len()` and beyond that it
258260
/// clamps at `self.len() / 2`.
259261
///
260-
/// If `T: Ord` does not implement a total order, the implementation may panic.
262+
/// # Panics
263+
///
264+
/// May panic if `compare` does not implement a [total order].
261265
///
262266
/// # Examples
263267
///
264268
/// ```
265-
/// let mut v = [5, 4, 1, 3, 2];
269+
/// let mut v = [4, -5, 1, -3, 2];
266270
/// v.sort_by(|a, b| a.cmp(b));
267-
/// assert!(v == [1, 2, 3, 4, 5]);
271+
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
268272
///
269273
/// // reverse sorting
270274
/// v.sort_by(|a, b| b.cmp(a));
271-
/// assert!(v == [5, 4, 3, 2, 1]);
275+
/// assert_eq!(v, [4, 2, 1, -3, -5]);
272276
/// ```
273277
///
274278
/// [driftsort]: https://github.com/Voultapher/driftsort
279+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
275280
#[cfg(not(no_global_oom_handling))]
276281
#[rustc_allow_incoherent_impl]
277282
#[stable(feature = "rust1", since = "1.0.0")]
@@ -288,9 +293,10 @@ impl<T> [T] {
288293
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*m* \* *n* \* log(*n*))
289294
/// worst-case, where the key function is *O*(*m*).
290295
///
291-
/// If `K: Ord` does not implement a total order the resulting order is unspecified.
292-
/// All original elements will remain in the slice and any possible modifications via interior
293-
/// mutability are observed in the input. Same is true if `K: Ord` panics.
296+
/// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting
297+
/// order of elements in the slice is unspecified. All original elements will remain in the
298+
/// slice and any possible modifications via interior mutability are observed in the input. Same
299+
/// is true if the implementation of [`Ord`] for `K` panics.
294300
///
295301
/// # Current implementation
296302
///
@@ -303,18 +309,21 @@ impl<T> [T] {
303309
/// handled without allocation, medium sized slices allocate `self.len()` and beyond that it
304310
/// clamps at `self.len() / 2`.
305311
///
306-
/// If `K: Ord` does not implement a total order, the implementation may panic.
312+
/// # Panics
313+
///
314+
/// May panic if the implementation of [`Ord`] for `K` does not implement a [total order].
307315
///
308316
/// # Examples
309317
///
310318
/// ```
311-
/// let mut v = [-5i32, 4, 1, -3, 2];
319+
/// let mut v = [4i32, -5, 1, -3, 2];
312320
///
313321
/// v.sort_by_key(|k| k.abs());
314-
/// assert!(v == [1, 2, -3, 4, -5]);
322+
/// assert_eq!(v, [1, 2, -3, 4, -5]);
315323
/// ```
316324
///
317325
/// [driftsort]: https://github.com/Voultapher/driftsort
326+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
318327
#[cfg(not(no_global_oom_handling))]
319328
#[rustc_allow_incoherent_impl]
320329
#[stable(feature = "slice_sort_by_key", since = "1.7.0")]
@@ -336,9 +345,10 @@ impl<T> [T] {
336345
/// storage to remember the results of key evaluation. The order of calls to the key function is
337346
/// unspecified and may change in future versions of the standard library.
338347
///
339-
/// If `K: Ord` does not implement a total order the resulting order is unspecified.
340-
/// All original elements will remain in the slice and any possible modifications via interior
341-
/// mutability are observed in the input. Same is true if `K: Ord` panics.
348+
/// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting
349+
/// order of elements in the slice is unspecified. All original elements will remain in the
350+
/// slice and any possible modifications via interior mutability are observed in the input. Same
351+
/// is true if the implementation of [`Ord`] for `K` panics.
342352
///
343353
/// For simple key functions (e.g., functions that are property accesses or basic operations),
344354
/// [`sort_by_key`](slice::sort_by_key) is likely to be faster.
@@ -355,16 +365,22 @@ impl<T> [T] {
355365
/// In the worst case, the algorithm allocates temporary storage in a `Vec<(K, usize)>` the
356366
/// length of the slice.
357367
///
368+
/// # Panics
369+
///
370+
/// May panic if the implementation of [`Ord`] for `K` does not implement a [total order].
371+
///
358372
/// # Examples
359373
///
360374
/// ```
361-
/// let mut v = [-5i32, 4, 32, -3, 2];
375+
/// let mut v = [4i32, -5, 1, -3, 2, 10];
362376
///
377+
/// // Strings are sorted by lexicographical order.
363378
/// v.sort_by_cached_key(|k| k.to_string());
364-
/// assert!(v == [-3, -5, 2, 32, 4]);
379+
/// assert_eq!(v, [-3, -5, 1, 10, 2, 4]);
365380
/// ```
366381
///
367382
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
383+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
368384
#[cfg(not(no_global_oom_handling))]
369385
#[rustc_allow_incoherent_impl]
370386
#[stable(feature = "slice_sort_by_cached_key", since = "1.34.0")]

0 commit comments

Comments
 (0)