Skip to content

Commit 7781346

Browse files
committed
Stop using specialization for this
Uses `__`-named `doc(hidden)` methods instead.
1 parent 35248c6 commit 7781346

File tree

4 files changed

+95
-84
lines changed

4 files changed

+95
-84
lines changed

library/core/src/cmp.rs

+87-75
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ mod bytewise;
2929
pub(crate) use bytewise::BytewiseEq;
3030

3131
use self::Ordering::*;
32-
use crate::ops::ControlFlow::{self, Break, Continue};
32+
use crate::ops::ControlFlow;
3333

3434
/// Trait for comparisons using the equality operator.
3535
///
@@ -1436,65 +1436,78 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
14361436
fn ge(&self, other: &Rhs) -> bool {
14371437
self.partial_cmp(other).is_some_and(Ordering::is_ge)
14381438
}
1439-
}
1440-
1441-
/// Derive macro generating an impl of the trait [`PartialOrd`].
1442-
/// The behavior of this macro is described in detail [here](PartialOrd#derivable).
1443-
#[rustc_builtin_macro]
1444-
#[stable(feature = "builtin_macro_prelude", since = "1.38.0")]
1445-
#[allow_internal_unstable(core_intrinsics)]
1446-
pub macro PartialOrd($item:item) {
1447-
/* compiler built-in */
1448-
}
1449-
1450-
/// Helpers for chaining together field PartialOrds into the full type's ordering.
1451-
///
1452-
/// If the two values are equal, returns `ControlFlow::Continue`.
1453-
/// If the two values are not equal, returns `ControlFlow::Break(self OP other)`.
1454-
///
1455-
/// This allows simple types like `i32` and `f64` to just emit two comparisons
1456-
/// directly, instead of needing to optimize the 3-way comparison.
1457-
///
1458-
/// Currently this is done using specialization, but it doesn't need that:
1459-
/// it could be provided methods on `PartialOrd` instead and work fine.
1460-
pub(crate) trait SpecChainingPartialOrd<Rhs>: PartialOrd<Rhs> {
1461-
fn spec_chain_lt(&self, other: &Rhs) -> ControlFlow<bool>;
1462-
fn spec_chain_le(&self, other: &Rhs) -> ControlFlow<bool>;
1463-
fn spec_chain_gt(&self, other: &Rhs) -> ControlFlow<bool>;
1464-
fn spec_chain_ge(&self, other: &Rhs) -> ControlFlow<bool>;
1465-
}
14661439

1467-
impl<T: PartialOrd<U>, U> SpecChainingPartialOrd<U> for T {
1440+
/// If `self == other`, returns `ControlFlow::Continue(())`.
1441+
/// Otherwise, returns `ControlFlow::Break(self < other)`.
1442+
///
1443+
/// This is useful for chaining together calls when implementing a lexical
1444+
/// `PartialOrd::lt`, as it allows types (like primitives) which can cheaply
1445+
/// check `==` and `<` separately to do rather than needing to calculate
1446+
/// (then optimize out) the three-way `Ordering` result.
14681447
#[inline]
1469-
default fn spec_chain_lt(&self, other: &U) -> ControlFlow<bool> {
1470-
match PartialOrd::partial_cmp(self, other) {
1471-
Some(Equal) => Continue(()),
1472-
c => Break(c.is_some_and(Ordering::is_lt)),
1473-
}
1448+
#[must_use]
1449+
// Added to improve the behaviour of tuples; not necessarily stabilization-track.
1450+
#[unstable(feature = "partial_ord_chaining_methods", issue = "none")]
1451+
#[doc(hidden)]
1452+
fn __chaining_lt(&self, other: &Rhs) -> ControlFlow<bool> {
1453+
default_chaining_impl(self, other, Ordering::is_lt)
14741454
}
1455+
1456+
/// Same as `__chaining_lt`, but for `<=` instead of `<`.
14751457
#[inline]
1476-
default fn spec_chain_le(&self, other: &U) -> ControlFlow<bool> {
1477-
match PartialOrd::partial_cmp(self, other) {
1478-
Some(Equal) => Continue(()),
1479-
c => Break(c.is_some_and(Ordering::is_le)),
1480-
}
1458+
#[must_use]
1459+
#[unstable(feature = "partial_ord_chaining_methods", issue = "none")]
1460+
#[doc(hidden)]
1461+
fn __chaining_le(&self, other: &Rhs) -> ControlFlow<bool> {
1462+
default_chaining_impl(self, other, Ordering::is_le)
14811463
}
1464+
1465+
/// Same as `__chaining_lt`, but for `>` instead of `<`.
14821466
#[inline]
1483-
default fn spec_chain_gt(&self, other: &U) -> ControlFlow<bool> {
1484-
match PartialOrd::partial_cmp(self, other) {
1485-
Some(Equal) => Continue(()),
1486-
c => Break(c.is_some_and(Ordering::is_gt)),
1487-
}
1467+
#[must_use]
1468+
#[unstable(feature = "partial_ord_chaining_methods", issue = "none")]
1469+
#[doc(hidden)]
1470+
fn __chaining_gt(&self, other: &Rhs) -> ControlFlow<bool> {
1471+
default_chaining_impl(self, other, Ordering::is_gt)
14881472
}
1473+
1474+
/// Same as `__chaining_lt`, but for `>=` instead of `<`.
14891475
#[inline]
1490-
default fn spec_chain_ge(&self, other: &U) -> ControlFlow<bool> {
1491-
match PartialOrd::partial_cmp(self, other) {
1492-
Some(Equal) => Continue(()),
1493-
c => Break(c.is_some_and(Ordering::is_ge)),
1494-
}
1476+
#[must_use]
1477+
#[unstable(feature = "partial_ord_chaining_methods", issue = "none")]
1478+
#[doc(hidden)]
1479+
fn __chaining_ge(&self, other: &Rhs) -> ControlFlow<bool> {
1480+
default_chaining_impl(self, other, Ordering::is_ge)
14951481
}
14961482
}
14971483

1484+
fn default_chaining_impl<T: ?Sized, U: ?Sized>(
1485+
lhs: &T,
1486+
rhs: &U,
1487+
p: impl FnOnce(Ordering) -> bool,
1488+
) -> ControlFlow<bool>
1489+
where
1490+
T: PartialOrd<U>,
1491+
{
1492+
// It's important that this only call `partial_cmp` once, not call `eq` then
1493+
// one of the relational operators. We don't want to `bcmp`-then-`memcp` a
1494+
// `String`, for example, or similarly for other data structures (#108157).
1495+
match <T as PartialOrd<U>>::partial_cmp(lhs, rhs) {
1496+
Some(Equal) => ControlFlow::Continue(()),
1497+
Some(c) => ControlFlow::Break(p(c)),
1498+
None => ControlFlow::Break(false),
1499+
}
1500+
}
1501+
1502+
/// Derive macro generating an impl of the trait [`PartialOrd`].
1503+
/// The behavior of this macro is described in detail [here](PartialOrd#derivable).
1504+
#[rustc_builtin_macro]
1505+
#[stable(feature = "builtin_macro_prelude", since = "1.38.0")]
1506+
#[allow_internal_unstable(core_intrinsics)]
1507+
pub macro PartialOrd($item:item) {
1508+
/* compiler built-in */
1509+
}
1510+
14981511
/// Compares and returns the minimum of two values.
14991512
///
15001513
/// Returns the first argument if the comparison determines them to be equal.
@@ -1829,32 +1842,31 @@ mod impls {
18291842

18301843
eq_impl! { () bool char usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 }
18311844

1832-
macro_rules! chaining_impl {
1845+
macro_rules! chaining_methods_impl {
18331846
($t:ty) => {
18341847
// These implementations are the same for `Ord` or `PartialOrd` types
18351848
// because if either is NAN the `==` test will fail so we end up in
18361849
// the `Break` case and the comparison will correctly return `false`.
1837-
impl super::SpecChainingPartialOrd<$t> for $t {
1838-
#[inline]
1839-
fn spec_chain_lt(&self, other: &Self) -> ControlFlow<bool> {
1840-
let (lhs, rhs) = (*self, *other);
1841-
if lhs == rhs { Continue(()) } else { Break(lhs < rhs) }
1842-
}
1843-
#[inline]
1844-
fn spec_chain_le(&self, other: &Self) -> ControlFlow<bool> {
1845-
let (lhs, rhs) = (*self, *other);
1846-
if lhs == rhs { Continue(()) } else { Break(lhs <= rhs) }
1847-
}
1848-
#[inline]
1849-
fn spec_chain_gt(&self, other: &Self) -> ControlFlow<bool> {
1850-
let (lhs, rhs) = (*self, *other);
1851-
if lhs == rhs { Continue(()) } else { Break(lhs > rhs) }
1852-
}
1853-
#[inline]
1854-
fn spec_chain_ge(&self, other: &Self) -> ControlFlow<bool> {
1855-
let (lhs, rhs) = (*self, *other);
1856-
if lhs == rhs { Continue(()) } else { Break(lhs >= rhs) }
1857-
}
1850+
1851+
#[inline]
1852+
fn __chaining_lt(&self, other: &Self) -> ControlFlow<bool> {
1853+
let (lhs, rhs) = (*self, *other);
1854+
if lhs == rhs { Continue(()) } else { Break(lhs < rhs) }
1855+
}
1856+
#[inline]
1857+
fn __chaining_le(&self, other: &Self) -> ControlFlow<bool> {
1858+
let (lhs, rhs) = (*self, *other);
1859+
if lhs == rhs { Continue(()) } else { Break(lhs <= rhs) }
1860+
}
1861+
#[inline]
1862+
fn __chaining_gt(&self, other: &Self) -> ControlFlow<bool> {
1863+
let (lhs, rhs) = (*self, *other);
1864+
if lhs == rhs { Continue(()) } else { Break(lhs > rhs) }
1865+
}
1866+
#[inline]
1867+
fn __chaining_ge(&self, other: &Self) -> ControlFlow<bool> {
1868+
let (lhs, rhs) = (*self, *other);
1869+
if lhs == rhs { Continue(()) } else { Break(lhs >= rhs) }
18581870
}
18591871
};
18601872
}
@@ -1880,9 +1892,9 @@ mod impls {
18801892
fn ge(&self, other: &$t) -> bool { (*self) >= (*other) }
18811893
#[inline(always)]
18821894
fn gt(&self, other: &$t) -> bool { (*self) > (*other) }
1883-
}
18841895

1885-
chaining_impl!($t);
1896+
chaining_methods_impl!($t);
1897+
}
18861898
)*)
18871899
}
18881900

@@ -1920,9 +1932,9 @@ mod impls {
19201932
fn ge(&self, other: &$t) -> bool { (*self) >= (*other) }
19211933
#[inline(always)]
19221934
fn gt(&self, other: &$t) -> bool { (*self) > (*other) }
1923-
}
19241935

1925-
chaining_impl!($t);
1936+
chaining_methods_impl!($t);
1937+
}
19261938

19271939
#[stable(feature = "rust1", since = "1.0.0")]
19281940
impl Ord for $t {

library/core/src/tuple.rs

+6-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// See core/src/primitive_docs.rs for documentation.
22

33
use crate::cmp::Ordering::{self, *};
4-
use crate::cmp::SpecChainingPartialOrd;
54
use crate::marker::{ConstParamTy_, StructuralPartialEq, UnsizedConstParamTy};
65
use crate::ops::ControlFlow::{Break, Continue};
76

@@ -82,19 +81,19 @@ macro_rules! tuple_impls {
8281
}
8382
#[inline]
8483
fn lt(&self, other: &($($T,)+)) -> bool {
85-
lexical_ord!(lt, spec_chain_lt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
84+
lexical_ord!(lt, __chaining_lt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
8685
}
8786
#[inline]
8887
fn le(&self, other: &($($T,)+)) -> bool {
89-
lexical_ord!(le, spec_chain_le, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
88+
lexical_ord!(le, __chaining_le, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
9089
}
9190
#[inline]
9291
fn ge(&self, other: &($($T,)+)) -> bool {
93-
lexical_ord!(ge, spec_chain_ge, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
92+
lexical_ord!(ge, __chaining_ge, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
9493
}
9594
#[inline]
9695
fn gt(&self, other: &($($T,)+)) -> bool {
97-
lexical_ord!(gt, spec_chain_gt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
96+
lexical_ord!(gt, __chaining_gt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
9897
}
9998
}
10099
}
@@ -173,11 +172,11 @@ macro_rules! maybe_tuple_doc {
173172
// `(a1, a2, a3) < (b1, b2, b3)` would be `lexical_ord!(lt, opt_is_lt, a1, b1,
174173
// a2, b2, a3, b3)` (and similarly for `lexical_cmp`)
175174
//
176-
// `$chain_rel` is the method from `SpecChainingPartialOrd` to use for all but the
175+
// `$chain_rel` is the chaining method from `PartialOrd` to use for all but the
177176
// final value, to produce better results for simple primitives.
178177
macro_rules! lexical_ord {
179178
($rel: ident, $chain_rel: ident, $a:expr, $b:expr, $($rest_a:expr, $rest_b:expr),+) => {{
180-
match SpecChainingPartialOrd::$chain_rel(&$a, &$b) {
179+
match PartialOrd::$chain_rel(&$a, &$b) {
181180
Break(val) => val,
182181
Continue(()) => lexical_ord!($rel, $chain_rel, $($rest_a, $rest_b),+),
183182
}

tests/mir-opt/pre-codegen/tuple_ord.demo_ge_partial.PreCodegen.after.mir

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ fn demo_ge_partial(_1: &(f32, f32), _2: &(f32, f32)) -> bool {
1010
let _8: bool;
1111
scope 3 {
1212
}
13-
scope 4 (inlined std::cmp::impls::<impl std::cmp::SpecChainingPartialOrd<f32> for f32>::spec_chain_ge) {
13+
scope 4 (inlined std::cmp::impls::<impl PartialOrd for f32>::__chaining_ge) {
1414
let mut _3: f32;
1515
let mut _4: f32;
1616
let mut _5: bool;

tests/mir-opt/pre-codegen/tuple_ord.demo_le_total.PreCodegen.after.mir

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ fn demo_le_total(_1: &(u16, i16), _2: &(u16, i16)) -> bool {
1010
let _8: bool;
1111
scope 3 {
1212
}
13-
scope 4 (inlined std::cmp::impls::<impl std::cmp::SpecChainingPartialOrd<u16> for u16>::spec_chain_le) {
13+
scope 4 (inlined std::cmp::impls::<impl PartialOrd for u16>::__chaining_le) {
1414
let mut _3: u16;
1515
let mut _4: u16;
1616
let mut _5: bool;

0 commit comments

Comments
 (0)