Skip to content

Commit 2188712

Browse files
committed
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"
1 parent b014b0d commit 2188712

File tree

4 files changed

+97
-86
lines changed

4 files changed

+97
-86
lines changed

Diff for: alloc/src/slice.rs

+47-41
Original file line numberDiff line numberDiff line change
@@ -179,15 +179,25 @@ impl<T> [T] {
179179
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*n* \* log(*n*))
180180
/// worst-case.
181181
///
182-
/// If `T: Ord` does not implement a total order the resulting order is unspecified. All
183-
/// original elements will remain in the slice and any possible modifications via interior
184-
/// mutability are observed in the input. Same is true if `T: Ord` panics.
182+
/// If the implementation of [`Ord`] for `T` does not implement a [total order] the resulting
183+
/// order of elements in the slice is unspecified. All original elements will remain in the
184+
/// slice and any possible modifications via interior mutability are observed in the input. Same
185+
/// is true if the implementation of [`Ord`] for `T` panics.
185186
///
186187
/// When applicable, unstable sorting is preferred because it is generally faster than stable
187188
/// sorting and it doesn't allocate auxiliary memory. See
188189
/// [`sort_unstable`](slice::sort_unstable). The exception are partially sorted slices, which
189190
/// may be better served with `slice::sort`.
190191
///
192+
/// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] requires
193+
/// additional precautions. For example Rust defines `NaN != NaN`, which doesn't fulfill the
194+
/// reflexivity requirement posed by [`Ord`]. By using an alternative comparison function with
195+
/// [`slice::sort_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a [total
196+
/// order] users can sort slices containing floating point numbers. Alternatively, if one can
197+
/// guarantee that all values in the slice are comparable with [`PartialOrd::partial_cmp`] *and*
198+
/// the implementation forms a [total order], it's possible to sort the slice with `sort_by(|a,
199+
/// b| a.partial_cmp(b).unwrap())`.
200+
///
191201
/// # Current implementation
192202
///
193203
/// The current implementation is based on [driftsort] by Orson Peters and Lukas Bergdoll, which
@@ -201,18 +211,19 @@ impl<T> [T] {
201211
///
202212
/// # Panics
203213
///
204-
/// May panic if `T: Ord` does not implement a total order.
214+
/// May panic if the implementation of [`Ord`] for `T` does not implement a [total order].
205215
///
206216
/// # Examples
207217
///
208218
/// ```
209-
/// let mut v = [-5, 4, 1, -3, 2];
219+
/// let mut v = [4, -5, 1, -3, 2];
210220
///
211221
/// v.sort();
212-
/// assert!(v == [-5, -3, 1, 2, 4]);
222+
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
213223
/// ```
214224
///
215225
/// [driftsort]: https://github.com/Voultapher/driftsort
226+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
216227
#[cfg(not(no_global_oom_handling))]
217228
#[rustc_allow_incoherent_impl]
218229
#[stable(feature = "rust1", since = "1.0.0")]
@@ -224,30 +235,19 @@ impl<T> [T] {
224235
stable_sort(self, T::lt);
225236
}
226237

227-
/// Sorts the slice with a comparator function, preserving initial order of equal elements.
238+
/// Sorts the slice with a comparison function, preserving initial order of equal elements.
228239
///
229240
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*n* \* log(*n*))
230241
/// worst-case.
231242
///
232-
/// The comparator function should define a total ordering for the elements in the slice. If the
233-
/// ordering is not total, the order of the elements is unspecified.
234-
///
235-
/// If the comparator function does not implement a total order the resulting order is
236-
/// unspecified. All original elements will remain in the slice and any possible modifications
237-
/// via interior mutability are observed in the input. Same is true if the comparator function
238-
/// panics. A total order (for all `a`, `b` and `c`):
239-
///
240-
/// * total and antisymmetric: exactly one of `a < b`, `a == b` or `a > b` is true, and
241-
/// * transitive, `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`.
243+
/// If the comparison function `compare` does not implement a [total order] the resulting order
244+
/// of elements in the slice is unspecified. All original elements will remain in the slice and
245+
/// any possible modifications via interior mutability are observed in the input. Same is true
246+
/// if `compare` panics.
242247
///
243-
/// For example, while [`f64`] doesn't implement [`Ord`] because `NaN != NaN`, we can use
244-
/// `partial_cmp` as our sort function when we know the slice doesn't contain a `NaN`.
245-
///
246-
/// ```
247-
/// let mut floats = [5f64, 4.0, 1.0, 3.0, 2.0];
248-
/// floats.sort_unstable_by(|a, b| a.partial_cmp(b).unwrap());
249-
/// assert_eq!(floats, [1.0, 2.0, 3.0, 4.0, 5.0]);
250-
/// ```
248+
/// For example `|a, b| (a - b).cmp(a)` is a comparison function that is neither transitive nor
249+
/// reflexive nor total, `a < b < c < a` with `a = 1, b = 2, c = 3`. For more information and
250+
/// examples see the [`Ord`] documentation.
251251
///
252252
/// # Current implementation
253253
///
@@ -262,21 +262,22 @@ impl<T> [T] {
262262
///
263263
/// # Panics
264264
///
265-
/// May panic if `compare` does not implement a total order.
265+
/// May panic if `compare` does not implement a [total order].
266266
///
267267
/// # Examples
268268
///
269269
/// ```
270-
/// let mut v = [5, 4, 1, 3, 2];
270+
/// let mut v = [4, -5, 1, -3, 2];
271271
/// v.sort_by(|a, b| a.cmp(b));
272-
/// assert!(v == [1, 2, 3, 4, 5]);
272+
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
273273
///
274274
/// // reverse sorting
275275
/// v.sort_by(|a, b| b.cmp(a));
276-
/// assert!(v == [5, 4, 3, 2, 1]);
276+
/// assert_eq!(v, [4, 2, 1, -3, -5]);
277277
/// ```
278278
///
279279
/// [driftsort]: https://github.com/Voultapher/driftsort
280+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
280281
#[cfg(not(no_global_oom_handling))]
281282
#[rustc_allow_incoherent_impl]
282283
#[stable(feature = "rust1", since = "1.0.0")]
@@ -293,9 +294,10 @@ impl<T> [T] {
293294
/// This sort is stable (i.e., does not reorder equal elements) and *O*(*m* \* *n* \* log(*n*))
294295
/// worst-case, where the key function is *O*(*m*).
295296
///
296-
/// If `K: Ord` does not implement a total order the resulting order is unspecified.
297-
/// All original elements will remain in the slice and any possible modifications via interior
298-
/// mutability are observed in the input. Same is true if `K: Ord` panics.
297+
/// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting
298+
/// order of elements in the slice is unspecified. All original elements will remain in the
299+
/// slice and any possible modifications via interior mutability are observed in the input. Same
300+
/// is true if the implementation of [`Ord`] for `K` panics.
299301
///
300302
/// # Current implementation
301303
///
@@ -310,18 +312,19 @@ impl<T> [T] {
310312
///
311313
/// # Panics
312314
///
313-
/// May panic if `K: Ord` does not implement a total order.
315+
/// May panic if the implementation of [`Ord`] for `K` does not implement a [total order].
314316
///
315317
/// # Examples
316318
///
317319
/// ```
318-
/// let mut v = [-5i32, 4, 1, -3, 2];
320+
/// let mut v = [4i32, -5, 1, -3, 2];
319321
///
320322
/// v.sort_by_key(|k| k.abs());
321-
/// assert!(v == [1, 2, -3, 4, -5]);
323+
/// assert_eq!(v, [1, 2, -3, 4, -5]);
322324
/// ```
323325
///
324326
/// [driftsort]: https://github.com/Voultapher/driftsort
327+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
325328
#[cfg(not(no_global_oom_handling))]
326329
#[rustc_allow_incoherent_impl]
327330
#[stable(feature = "slice_sort_by_key", since = "1.7.0")]
@@ -343,9 +346,10 @@ impl<T> [T] {
343346
/// storage to remember the results of key evaluation. The order of calls to the key function is
344347
/// unspecified and may change in future versions of the standard library.
345348
///
346-
/// If `K: Ord` does not implement a total order the resulting order is unspecified.
347-
/// All original elements will remain in the slice and any possible modifications via interior
348-
/// mutability are observed in the input. Same is true if `K: Ord` panics.
349+
/// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting
350+
/// order of elements in the slice is unspecified. All original elements will remain in the
351+
/// slice and any possible modifications via interior mutability are observed in the input. Same
352+
/// is true if the implementation of [`Ord`] for `K` panics.
349353
///
350354
/// For simple key functions (e.g., functions that are property accesses or basic operations),
351355
/// [`sort_by_key`](slice::sort_by_key) is likely to be faster.
@@ -364,18 +368,20 @@ impl<T> [T] {
364368
///
365369
/// # Panics
366370
///
367-
/// May panic if `K: Ord` does not implement a total order.
371+
/// May panic if the implementation of [`Ord`] for `K` does not implement a [total order].
368372
///
369373
/// # Examples
370374
///
371375
/// ```
372-
/// let mut v = [-5i32, 4, 32, -3, 2];
376+
/// let mut v = [4i32, -5, 1, -3, 2, 10];
373377
///
378+
/// // Strings are sorted by lexicographical order.
374379
/// v.sort_by_cached_key(|k| k.to_string());
375-
/// assert!(v == [-3, -5, 2, 32, 4]);
380+
/// assert_eq!(v, [-3, -5, 1, 10, 2, 4]);
376381
/// ```
377382
///
378383
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
384+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
379385
#[cfg(not(no_global_oom_handling))]
380386
#[rustc_allow_incoherent_impl]
381387
#[stable(feature = "slice_sort_by_cached_key", since = "1.34.0")]

Diff for: core/src/slice/mod.rs

+43-37
Original file line numberDiff line numberDiff line change
@@ -2884,9 +2884,19 @@ impl<T> [T] {
28842884
/// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not
28852885
/// allocate), and *O*(*n* \* log(*n*)) worst-case.
28862886
///
2887-
/// If `T: Ord` does not implement a total order the resulting order is unspecified. All
2888-
/// original elements will remain in the slice and any possible modifications via interior
2889-
/// mutability are observed in the input. Same is true if `T: Ord` panics.
2887+
/// If the implementation of [`Ord`] for `T` does not implement a [total order] the resulting
2888+
/// order of elements in the slice is unspecified. All original elements will remain in the
2889+
/// slice and any possible modifications via interior mutability are observed in the input. Same
2890+
/// is true if the implementation of [`Ord`] for `T` panics.
2891+
///
2892+
/// Sorting types that only implement [`PartialOrd`] such as [`f32`] and [`f64`] requires
2893+
/// additional precautions. For example Rust defines `NaN != NaN`, which doesn't fulfill the
2894+
/// reflexivity requirement posed by [`Ord`]. By using an alternative comparison function with
2895+
/// [`slice::sort_unstable_by`] such as [`f32::total_cmp`] or [`f64::total_cmp`] that defines a
2896+
/// [total order] users can sort slices containing floating point numbers. Alternatively, if one
2897+
/// can guarantee that all values in the slice are comparable with [`PartialOrd::partial_cmp`]
2898+
/// *and* the implementation forms a [total order], it's possible to sort the slice with
2899+
/// `sort_unstable_by(|a, b| a.partial_cmp(b).unwrap())`.
28902900
///
28912901
/// # Current implementation
28922902
///
@@ -2900,18 +2910,19 @@ impl<T> [T] {
29002910
///
29012911
/// # Panics
29022912
///
2903-
/// May panic if `T: Ord` does not implement a total order.
2913+
/// May panic if the implementation of [`Ord`] for `T` does not implement a [total order].
29042914
///
29052915
/// # Examples
29062916
///
29072917
/// ```
2908-
/// let mut v = [-5, 4, 1, -3, 2];
2918+
/// let mut v = [4, -5, 1, -3, 2];
29092919
///
29102920
/// v.sort_unstable();
2911-
/// assert!(v == [-5, -3, 1, 2, 4]);
2921+
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
29122922
/// ```
29132923
///
29142924
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
2925+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
29152926
#[stable(feature = "sort_unstable", since = "1.20.0")]
29162927
#[inline]
29172928
pub fn sort_unstable(&mut self)
@@ -2921,31 +2932,20 @@ impl<T> [T] {
29212932
sort::unstable::sort(self, &mut T::lt);
29222933
}
29232934

2924-
/// Sorts the slice with a comparator function, **without** preserving the initial order of
2935+
/// Sorts the slice with a comparison function, **without** preserving the initial order of
29252936
/// equal elements.
29262937
///
29272938
/// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not
29282939
/// allocate), and *O*(*n* \* log(*n*)) worst-case.
29292940
///
2930-
/// The comparator function should define a total ordering for the elements in the slice. If the
2931-
/// ordering is not total, the order of the elements is unspecified.
2941+
/// If the comparison function `compare` does not implement a [total order] the resulting order
2942+
/// of elements in the slice is unspecified. All original elements will remain in the slice and
2943+
/// any possible modifications via interior mutability are observed in the input. Same is true
2944+
/// if `compare` panics.
29322945
///
2933-
/// If the comparator function does not implement a total order the resulting order is
2934-
/// unspecified. All original elements will remain in the slice and any possible modifications
2935-
/// via interior mutability are observed in the input. Same is true if the comparator function
2936-
/// panics. A total order (for all `a`, `b` and `c`):
2937-
///
2938-
/// * total and antisymmetric: exactly one of `a < b`, `a == b` or `a > b` is true, and
2939-
/// * transitive, `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`.
2940-
///
2941-
/// For example, while [`f64`] doesn't implement [`Ord`] because `NaN != NaN`, we can use
2942-
/// `partial_cmp` as our sort function when we know the slice doesn't contain a `NaN`.
2943-
///
2944-
/// ```
2945-
/// let mut floats = [5f64, 4.0, 1.0, 3.0, 2.0];
2946-
/// floats.sort_unstable_by(|a, b| a.partial_cmp(b).unwrap());
2947-
/// assert_eq!(floats, [1.0, 2.0, 3.0, 4.0, 5.0]);
2948-
/// ```
2946+
/// For example `|a, b| (a - b).cmp(a)` is a comparison function that is neither transitive nor
2947+
/// reflexive nor total, `a < b < c < a` with `a = 1, b = 2, c = 3`. For more information and
2948+
/// examples see the [`Ord`] documentation.
29492949
///
29502950
/// # Current implementation
29512951
///
@@ -2959,21 +2959,22 @@ impl<T> [T] {
29592959
///
29602960
/// # Panics
29612961
///
2962-
/// May panic if `compare` does not implement a total order.
2962+
/// May panic if `compare` does not implement a [total order].
29632963
///
29642964
/// # Examples
29652965
///
29662966
/// ```
2967-
/// let mut v = [5, 4, 1, 3, 2];
2967+
/// let mut v = [4, -5, 1, -3, 2];
29682968
/// v.sort_unstable_by(|a, b| a.cmp(b));
2969-
/// assert!(v == [1, 2, 3, 4, 5]);
2969+
/// assert_eq!(v, [-5, -3, 1, 2, 4]);
29702970
///
29712971
/// // reverse sorting
29722972
/// v.sort_unstable_by(|a, b| b.cmp(a));
2973-
/// assert!(v == [5, 4, 3, 2, 1]);
2973+
/// assert_eq!(v, [4, 2, 1, -3, -5]);
29742974
/// ```
29752975
///
29762976
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
2977+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
29772978
#[stable(feature = "sort_unstable", since = "1.20.0")]
29782979
#[inline]
29792980
pub fn sort_unstable_by<F>(&mut self, mut compare: F)
@@ -2989,9 +2990,10 @@ impl<T> [T] {
29892990
/// This sort is unstable (i.e., may reorder equal elements), in-place (i.e., does not
29902991
/// allocate), and *O*(*n* \* log(*n*)) worst-case.
29912992
///
2992-
/// If `K: Ord` does not implement a total order the resulting order is unspecified.
2993-
/// All original elements will remain in the slice and any possible modifications via interior
2994-
/// mutability are observed in the input. Same is true if `K: Ord` panics.
2993+
/// If the implementation of [`Ord`] for `K` does not implement a [total order] the resulting
2994+
/// order of elements in the slice is unspecified. All original elements will remain in the
2995+
/// slice and any possible modifications via interior mutability are observed in the input. Same
2996+
/// is true if the implementation of [`Ord`] for `K` panics.
29952997
///
29962998
/// # Current implementation
29972999
///
@@ -3005,18 +3007,19 @@ impl<T> [T] {
30053007
///
30063008
/// # Panics
30073009
///
3008-
/// May panic if `K: Ord` does not implement a total order.
3010+
/// May panic if the implementation of [`Ord`] for `K` does not implement a [total order].
30093011
///
30103012
/// # Examples
30113013
///
30123014
/// ```
3013-
/// let mut v = [-5i32, 4, 1, -3, 2];
3015+
/// let mut v = [4i32, -5, 1, -3, 2];
30143016
///
30153017
/// v.sort_unstable_by_key(|k| k.abs());
3016-
/// assert!(v == [1, 2, -3, 4, -5]);
3018+
/// assert_eq!(v, [1, 2, -3, 4, -5]);
30173019
/// ```
30183020
///
30193021
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
3022+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
30203023
#[stable(feature = "sort_unstable", since = "1.20.0")]
30213024
#[inline]
30223025
pub fn sort_unstable_by_key<K, F>(&mut self, mut f: F)
@@ -3054,7 +3057,7 @@ impl<T> [T] {
30543057
///
30553058
/// Panics when `index >= len()`, meaning it always panics on empty slices.
30563059
///
3057-
/// May panic if `T: Ord` does not implement a total order.
3060+
/// May panic if the implementation of [`Ord`] for `T` does not implement a [total order].
30583061
///
30593062
/// # Examples
30603063
///
@@ -3078,6 +3081,7 @@ impl<T> [T] {
30783081
/// ```
30793082
///
30803083
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
3084+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
30813085
#[stable(feature = "slice_select_nth_unstable", since = "1.49.0")]
30823086
#[inline]
30833087
pub fn select_nth_unstable(&mut self, index: usize) -> (&mut [T], &mut T, &mut [T])
@@ -3114,7 +3118,7 @@ impl<T> [T] {
31143118
///
31153119
/// Panics when `index >= len()`, meaning it always panics on empty slices.
31163120
///
3117-
/// May panic if `compare` does not implement a total order.
3121+
/// May panic if `compare` does not implement a [total order].
31183122
///
31193123
/// # Examples
31203124
///
@@ -3138,6 +3142,7 @@ impl<T> [T] {
31383142
/// ```
31393143
///
31403144
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
3145+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
31413146
#[stable(feature = "slice_select_nth_unstable", since = "1.49.0")]
31423147
#[inline]
31433148
pub fn select_nth_unstable_by<F>(
@@ -3202,6 +3207,7 @@ impl<T> [T] {
32023207
/// ```
32033208
///
32043209
/// [ipnsort]: https://github.com/Voultapher/sort-research-rs/tree/main/ipnsort
3210+
/// [total order]: https://en.wikipedia.org/wiki/Total_order
32053211
#[stable(feature = "slice_select_nth_unstable", since = "1.49.0")]
32063212
#[inline]
32073213
pub fn select_nth_unstable_by_key<K, F>(

0 commit comments

Comments
 (0)