Skip to content

Commit a6d648f

Browse files
authored
Rollup merge of #139357 - miried:master, r=Amanieu
Fix parameter order for `_by()` variants of `min` / `max`/ `minmax` in `std::cmp` We saw a regression introduced in version `1.86` that seems to be coming from switching the order of `v1` and `v2` when calling `comparison` functions in `min_by` / `max_by` / `minmax_by` (cf. this PR: #136307) When the `compare` function is not symmetric in the arguments, this leads to false results. Apparently, the test cases do not cover this scenario currently. While asymmetric comparison may be an edge case, but current behavior is unexpected nevertheless.
2 parents f605b57 + 36d309e commit a6d648f

File tree

1 file changed

+12
-3
lines changed

1 file changed

+12
-3
lines changed

library/core/src/cmp.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1554,6 +1554,9 @@ pub fn min<T: Ord>(v1: T, v2: T) -> T {
15541554
///
15551555
/// Returns the first argument if the comparison determines them to be equal.
15561556
///
1557+
/// The parameter order is preserved when calling the `compare` function, i.e. `v1` is
1558+
/// always passed as the first argument and `v2` as the second.
1559+
///
15571560
/// # Examples
15581561
///
15591562
/// ```
@@ -1574,7 +1577,7 @@ pub fn min<T: Ord>(v1: T, v2: T) -> T {
15741577
#[must_use]
15751578
#[stable(feature = "cmp_min_max_by", since = "1.53.0")]
15761579
pub fn min_by<T, F: FnOnce(&T, &T) -> Ordering>(v1: T, v2: T, compare: F) -> T {
1577-
if compare(&v2, &v1).is_lt() { v2 } else { v1 }
1580+
if compare(&v1, &v2).is_le() { v1 } else { v2 }
15781581
}
15791582

15801583
/// Returns the element that gives the minimum value from the specified function.
@@ -1646,6 +1649,9 @@ pub fn max<T: Ord>(v1: T, v2: T) -> T {
16461649
///
16471650
/// Returns the second argument if the comparison determines them to be equal.
16481651
///
1652+
/// The parameter order is preserved when calling the `compare` function, i.e. `v1` is
1653+
/// always passed as the first argument and `v2` as the second.
1654+
///
16491655
/// # Examples
16501656
///
16511657
/// ```
@@ -1666,7 +1672,7 @@ pub fn max<T: Ord>(v1: T, v2: T) -> T {
16661672
#[must_use]
16671673
#[stable(feature = "cmp_min_max_by", since = "1.53.0")]
16681674
pub fn max_by<T, F: FnOnce(&T, &T) -> Ordering>(v1: T, v2: T, compare: F) -> T {
1669-
if compare(&v2, &v1).is_lt() { v1 } else { v2 }
1675+
if compare(&v1, &v2).is_gt() { v1 } else { v2 }
16701676
}
16711677

16721678
/// Returns the element that gives the maximum value from the specified function.
@@ -1745,6 +1751,9 @@ where
17451751
///
17461752
/// Returns `[v1, v2]` if the comparison determines them to be equal.
17471753
///
1754+
/// The parameter order is preserved when calling the `compare` function, i.e. `v1` is
1755+
/// always passed as the first argument and `v2` as the second.
1756+
///
17481757
/// # Examples
17491758
///
17501759
/// ```
@@ -1769,7 +1778,7 @@ pub fn minmax_by<T, F>(v1: T, v2: T, compare: F) -> [T; 2]
17691778
where
17701779
F: FnOnce(&T, &T) -> Ordering,
17711780
{
1772-
if compare(&v2, &v1).is_lt() { [v2, v1] } else { [v1, v2] }
1781+
if compare(&v1, &v2).is_le() { [v1, v2] } else { [v2, v1] }
17731782
}
17741783

17751784
/// Returns minimum and maximum values with respect to the specified key function.

0 commit comments

Comments
 (0)