about summary refs log tree commit diff
diff options
context:
space:
mode:
authorScott McMurray <scottmcm@users.noreply.github.com>2025-03-23 15:27:31 -0700
committerScott McMurray <scottmcm@users.noreply.github.com>2025-03-23 15:27:31 -0700
commit7781346243c1e1a038e0bc6fa11e5e1aefea7d4a (patch)
tree3c1dcaef98d6cf7d008879d0630fcf6b72079292
parent35248c6830383e67b8e2890ddd5ee48d44ad6b87 (diff)
downloadrust-7781346243c1e1a038e0bc6fa11e5e1aefea7d4a.tar.gz
rust-7781346243c1e1a038e0bc6fa11e5e1aefea7d4a.zip
Stop using specialization for this
Uses `__`-named `doc(hidden)` methods instead.
-rw-r--r--library/core/src/cmp.rs162
-rw-r--r--library/core/src/tuple.rs13
-rw-r--r--tests/mir-opt/pre-codegen/tuple_ord.demo_ge_partial.PreCodegen.after.mir2
-rw-r--r--tests/mir-opt/pre-codegen/tuple_ord.demo_le_total.PreCodegen.after.mir2
4 files changed, 95 insertions, 84 deletions
diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs
index af14c4b5072..0b0dbf723b6 100644
--- a/library/core/src/cmp.rs
+++ b/library/core/src/cmp.rs
@@ -29,7 +29,7 @@ mod bytewise;
 pub(crate) use bytewise::BytewiseEq;
 
 use self::Ordering::*;
-use crate::ops::ControlFlow::{self, Break, Continue};
+use crate::ops::ControlFlow;
 
 /// Trait for comparisons using the equality operator.
 ///
@@ -1436,65 +1436,78 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     fn ge(&self, other: &Rhs) -> bool {
         self.partial_cmp(other).is_some_and(Ordering::is_ge)
     }
-}
-
-/// Derive macro generating an impl of the trait [`PartialOrd`].
-/// The behavior of this macro is described in detail [here](PartialOrd#derivable).
-#[rustc_builtin_macro]
-#[stable(feature = "builtin_macro_prelude", since = "1.38.0")]
-#[allow_internal_unstable(core_intrinsics)]
-pub macro PartialOrd($item:item) {
-    /* compiler built-in */
-}
-
-/// Helpers for chaining together field PartialOrds into the full type's ordering.
-///
-/// If the two values are equal, returns `ControlFlow::Continue`.
-/// If the two values are not equal, returns `ControlFlow::Break(self OP other)`.
-///
-/// This allows simple types like `i32` and `f64` to just emit two comparisons
-/// directly, instead of needing to optimize the 3-way comparison.
-///
-/// Currently this is done using specialization, but it doesn't need that:
-/// it could be provided methods on `PartialOrd` instead and work fine.
-pub(crate) trait SpecChainingPartialOrd<Rhs>: PartialOrd<Rhs> {
-    fn spec_chain_lt(&self, other: &Rhs) -> ControlFlow<bool>;
-    fn spec_chain_le(&self, other: &Rhs) -> ControlFlow<bool>;
-    fn spec_chain_gt(&self, other: &Rhs) -> ControlFlow<bool>;
-    fn spec_chain_ge(&self, other: &Rhs) -> ControlFlow<bool>;
-}
 
-impl<T: PartialOrd<U>, U> SpecChainingPartialOrd<U> for T {
+    /// If `self == other`, returns `ControlFlow::Continue(())`.
+    /// Otherwise, returns `ControlFlow::Break(self < other)`.
+    ///
+    /// This is useful for chaining together calls when implementing a lexical
+    /// `PartialOrd::lt`, as it allows types (like primitives) which can cheaply
+    /// check `==` and `<` separately to do rather than needing to calculate
+    /// (then optimize out) the three-way `Ordering` result.
     #[inline]
-    default fn spec_chain_lt(&self, other: &U) -> ControlFlow<bool> {
-        match PartialOrd::partial_cmp(self, other) {
-            Some(Equal) => Continue(()),
-            c => Break(c.is_some_and(Ordering::is_lt)),
-        }
+    #[must_use]
+    // Added to improve the behaviour of tuples; not necessarily stabilization-track.
+    #[unstable(feature = "partial_ord_chaining_methods", issue = "none")]
+    #[doc(hidden)]
+    fn __chaining_lt(&self, other: &Rhs) -> ControlFlow<bool> {
+        default_chaining_impl(self, other, Ordering::is_lt)
     }
+
+    /// Same as `__chaining_lt`, but for `<=` instead of `<`.
     #[inline]
-    default fn spec_chain_le(&self, other: &U) -> ControlFlow<bool> {
-        match PartialOrd::partial_cmp(self, other) {
-            Some(Equal) => Continue(()),
-            c => Break(c.is_some_and(Ordering::is_le)),
-        }
+    #[must_use]
+    #[unstable(feature = "partial_ord_chaining_methods", issue = "none")]
+    #[doc(hidden)]
+    fn __chaining_le(&self, other: &Rhs) -> ControlFlow<bool> {
+        default_chaining_impl(self, other, Ordering::is_le)
     }
+
+    /// Same as `__chaining_lt`, but for `>` instead of `<`.
     #[inline]
-    default fn spec_chain_gt(&self, other: &U) -> ControlFlow<bool> {
-        match PartialOrd::partial_cmp(self, other) {
-            Some(Equal) => Continue(()),
-            c => Break(c.is_some_and(Ordering::is_gt)),
-        }
+    #[must_use]
+    #[unstable(feature = "partial_ord_chaining_methods", issue = "none")]
+    #[doc(hidden)]
+    fn __chaining_gt(&self, other: &Rhs) -> ControlFlow<bool> {
+        default_chaining_impl(self, other, Ordering::is_gt)
     }
+
+    /// Same as `__chaining_lt`, but for `>=` instead of `<`.
     #[inline]
-    default fn spec_chain_ge(&self, other: &U) -> ControlFlow<bool> {
-        match PartialOrd::partial_cmp(self, other) {
-            Some(Equal) => Continue(()),
-            c => Break(c.is_some_and(Ordering::is_ge)),
-        }
+    #[must_use]
+    #[unstable(feature = "partial_ord_chaining_methods", issue = "none")]
+    #[doc(hidden)]
+    fn __chaining_ge(&self, other: &Rhs) -> ControlFlow<bool> {
+        default_chaining_impl(self, other, Ordering::is_ge)
     }
 }
 
+fn default_chaining_impl<T: ?Sized, U: ?Sized>(
+    lhs: &T,
+    rhs: &U,
+    p: impl FnOnce(Ordering) -> bool,
+) -> ControlFlow<bool>
+where
+    T: PartialOrd<U>,
+{
+    // It's important that this only call `partial_cmp` once, not call `eq` then
+    // one of the relational operators.  We don't want to `bcmp`-then-`memcp` a
+    // `String`, for example, or similarly for other data structures (#108157).
+    match <T as PartialOrd<U>>::partial_cmp(lhs, rhs) {
+        Some(Equal) => ControlFlow::Continue(()),
+        Some(c) => ControlFlow::Break(p(c)),
+        None => ControlFlow::Break(false),
+    }
+}
+
+/// Derive macro generating an impl of the trait [`PartialOrd`].
+/// The behavior of this macro is described in detail [here](PartialOrd#derivable).
+#[rustc_builtin_macro]
+#[stable(feature = "builtin_macro_prelude", since = "1.38.0")]
+#[allow_internal_unstable(core_intrinsics)]
+pub macro PartialOrd($item:item) {
+    /* compiler built-in */
+}
+
 /// Compares and returns the minimum of two values.
 ///
 /// Returns the first argument if the comparison determines them to be equal.
@@ -1829,32 +1842,31 @@ mod impls {
 
     eq_impl! { () bool char usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 }
 
-    macro_rules! chaining_impl {
+    macro_rules! chaining_methods_impl {
         ($t:ty) => {
             // These implementations are the same for `Ord` or `PartialOrd` types
             // because if either is NAN the `==` test will fail so we end up in
             // the `Break` case and the comparison will correctly return `false`.
-            impl super::SpecChainingPartialOrd<$t> for $t {
-                #[inline]
-                fn spec_chain_lt(&self, other: &Self) -> ControlFlow<bool> {
-                    let (lhs, rhs) = (*self, *other);
-                    if lhs == rhs { Continue(()) } else { Break(lhs < rhs) }
-                }
-                #[inline]
-                fn spec_chain_le(&self, other: &Self) -> ControlFlow<bool> {
-                    let (lhs, rhs) = (*self, *other);
-                    if lhs == rhs { Continue(()) } else { Break(lhs <= rhs) }
-                }
-                #[inline]
-                fn spec_chain_gt(&self, other: &Self) -> ControlFlow<bool> {
-                    let (lhs, rhs) = (*self, *other);
-                    if lhs == rhs { Continue(()) } else { Break(lhs > rhs) }
-                }
-                #[inline]
-                fn spec_chain_ge(&self, other: &Self) -> ControlFlow<bool> {
-                    let (lhs, rhs) = (*self, *other);
-                    if lhs == rhs { Continue(()) } else { Break(lhs >= rhs) }
-                }
+
+            #[inline]
+            fn __chaining_lt(&self, other: &Self) -> ControlFlow<bool> {
+                let (lhs, rhs) = (*self, *other);
+                if lhs == rhs { Continue(()) } else { Break(lhs < rhs) }
+            }
+            #[inline]
+            fn __chaining_le(&self, other: &Self) -> ControlFlow<bool> {
+                let (lhs, rhs) = (*self, *other);
+                if lhs == rhs { Continue(()) } else { Break(lhs <= rhs) }
+            }
+            #[inline]
+            fn __chaining_gt(&self, other: &Self) -> ControlFlow<bool> {
+                let (lhs, rhs) = (*self, *other);
+                if lhs == rhs { Continue(()) } else { Break(lhs > rhs) }
+            }
+            #[inline]
+            fn __chaining_ge(&self, other: &Self) -> ControlFlow<bool> {
+                let (lhs, rhs) = (*self, *other);
+                if lhs == rhs { Continue(()) } else { Break(lhs >= rhs) }
             }
         };
     }
@@ -1880,9 +1892,9 @@ mod impls {
                 fn ge(&self, other: &$t) -> bool { (*self) >= (*other) }
                 #[inline(always)]
                 fn gt(&self, other: &$t) -> bool { (*self) > (*other) }
-            }
 
-            chaining_impl!($t);
+                chaining_methods_impl!($t);
+            }
         )*)
     }
 
@@ -1920,9 +1932,9 @@ mod impls {
                 fn ge(&self, other: &$t) -> bool { (*self) >= (*other) }
                 #[inline(always)]
                 fn gt(&self, other: &$t) -> bool { (*self) > (*other) }
-            }
 
-            chaining_impl!($t);
+                chaining_methods_impl!($t);
+            }
 
             #[stable(feature = "rust1", since = "1.0.0")]
             impl Ord for $t {
diff --git a/library/core/src/tuple.rs b/library/core/src/tuple.rs
index 75faaa06ee7..d754bb90343 100644
--- a/library/core/src/tuple.rs
+++ b/library/core/src/tuple.rs
@@ -1,7 +1,6 @@
 // See core/src/primitive_docs.rs for documentation.
 
 use crate::cmp::Ordering::{self, *};
-use crate::cmp::SpecChainingPartialOrd;
 use crate::marker::{ConstParamTy_, StructuralPartialEq, UnsizedConstParamTy};
 use crate::ops::ControlFlow::{Break, Continue};
 
@@ -82,19 +81,19 @@ macro_rules! tuple_impls {
                 }
                 #[inline]
                 fn lt(&self, other: &($($T,)+)) -> bool {
-                    lexical_ord!(lt, spec_chain_lt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
+                    lexical_ord!(lt, __chaining_lt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
                 }
                 #[inline]
                 fn le(&self, other: &($($T,)+)) -> bool {
-                    lexical_ord!(le, spec_chain_le, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
+                    lexical_ord!(le, __chaining_le, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
                 }
                 #[inline]
                 fn ge(&self, other: &($($T,)+)) -> bool {
-                    lexical_ord!(ge, spec_chain_ge, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
+                    lexical_ord!(ge, __chaining_ge, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
                 }
                 #[inline]
                 fn gt(&self, other: &($($T,)+)) -> bool {
-                    lexical_ord!(gt, spec_chain_gt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
+                    lexical_ord!(gt, __chaining_gt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
                 }
             }
         }
@@ -173,11 +172,11 @@ macro_rules! maybe_tuple_doc {
 // `(a1, a2, a3) < (b1, b2, b3)` would be `lexical_ord!(lt, opt_is_lt, a1, b1,
 // a2, b2, a3, b3)` (and similarly for `lexical_cmp`)
 //
-// `$chain_rel` is the method from `SpecChainingPartialOrd` to use for all but the
+// `$chain_rel` is the chaining method from `PartialOrd` to use for all but the
 // final value, to produce better results for simple primitives.
 macro_rules! lexical_ord {
     ($rel: ident, $chain_rel: ident, $a:expr, $b:expr, $($rest_a:expr, $rest_b:expr),+) => {{
-        match SpecChainingPartialOrd::$chain_rel(&$a, &$b) {
+        match PartialOrd::$chain_rel(&$a, &$b) {
             Break(val) => val,
             Continue(()) => lexical_ord!($rel, $chain_rel, $($rest_a, $rest_b),+),
         }
diff --git a/tests/mir-opt/pre-codegen/tuple_ord.demo_ge_partial.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/tuple_ord.demo_ge_partial.PreCodegen.after.mir
index 6531683b644..dd2eebc8f4a 100644
--- a/tests/mir-opt/pre-codegen/tuple_ord.demo_ge_partial.PreCodegen.after.mir
+++ b/tests/mir-opt/pre-codegen/tuple_ord.demo_ge_partial.PreCodegen.after.mir
@@ -10,7 +10,7 @@ fn demo_ge_partial(_1: &(f32, f32), _2: &(f32, f32)) -> bool {
             let _8: bool;
             scope 3 {
             }
-            scope 4 (inlined std::cmp::impls::<impl std::cmp::SpecChainingPartialOrd<f32> for f32>::spec_chain_ge) {
+            scope 4 (inlined std::cmp::impls::<impl PartialOrd for f32>::__chaining_ge) {
                 let mut _3: f32;
                 let mut _4: f32;
                 let mut _5: bool;
diff --git a/tests/mir-opt/pre-codegen/tuple_ord.demo_le_total.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/tuple_ord.demo_le_total.PreCodegen.after.mir
index d252052f0ae..ea1d164cefa 100644
--- a/tests/mir-opt/pre-codegen/tuple_ord.demo_le_total.PreCodegen.after.mir
+++ b/tests/mir-opt/pre-codegen/tuple_ord.demo_le_total.PreCodegen.after.mir
@@ -10,7 +10,7 @@ fn demo_le_total(_1: &(u16, i16), _2: &(u16, i16)) -> bool {
             let _8: bool;
             scope 3 {
             }
-            scope 4 (inlined std::cmp::impls::<impl std::cmp::SpecChainingPartialOrd<u16> for u16>::spec_chain_le) {
+            scope 4 (inlined std::cmp::impls::<impl PartialOrd for u16>::__chaining_le) {
                 let mut _3: u16;
                 let mut _4: u16;
                 let mut _5: bool;