Rollup merge of rust-lang#138135 - scottmcm:chaining-ord, r=Mark-Simulacrum

Simplify `PartialOrd` on tuples containing primitives

We noticed in rust-lang#133984 (comment) that currently the tuple comparison code, while it [does optimize down](https://github.com/rust-lang/rust/blob/master/tests/codegen/comparison-operators-2-tuple.rs) today, is kinda huge: <https://rust.godbolt.org/z/xqMoeYbhE>

This PR changes the tuple code to go through an overridable "chaining" version of the comparison functions, so that for simple things like `(i16, u16)` and `(f32, f32)` (as seen in the new MIR pre-codegen test) we just directly get the
```rust
if lhs.0 == rhs.0 { lhs.0 OP rhs.0 }
else { lhs.1 OP rhs.1 }
```
version in MIR, rather than emitting a mess for LLVM to have to clean up.

Test added in the first commit, so you can see the MIR diff in the second one.

Author: jhpratt

core/src/cmp.rs

@@ -29,6 +29,7 @@ mod bytewise;
 pub(crate) use bytewise::BytewiseEq;
 
 use self::Ordering::*;
+use crate::ops::ControlFlow;
 
 /// Trait for comparisons using the equality operator.
 ///
@@ -1435,6 +1436,67 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     fn ge(&self, other: &Rhs) -> bool {
         self.partial_cmp(other).is_some_and(Ordering::is_ge)
     }
+
+    /// 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]
+    #[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]
+    #[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]
+    #[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]
+    #[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`].
@@ -1741,6 +1803,7 @@ where
 mod impls {
     use crate::cmp::Ordering::{self, Equal, Greater, Less};
     use crate::hint::unreachable_unchecked;
+    use crate::ops::ControlFlow::{self, Break, Continue};
 
     macro_rules! partial_eq_impl {
         ($($t:ty)*) => ($(
@@ -1779,6 +1842,35 @@ mod impls {
 
     eq_impl! { () bool char usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 }
 
+    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`.
+
+            #[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) }
+            }
+        };
+    }
+
     macro_rules! partial_ord_impl {
         ($($t:ty)*) => ($(
             #[stable(feature = "rust1", since = "1.0.0")]
@@ -1800,6 +1892,8 @@ mod impls {
                 fn ge(&self, other: &$t) -> bool { (*self) >= (*other) }
                 #[inline(always)]
                 fn gt(&self, other: &$t) -> bool { (*self) > (*other) }
+
+                chaining_methods_impl!($t);
             }
         )*)
     }
@@ -1838,6 +1932,8 @@ mod impls {
                 fn ge(&self, other: &$t) -> bool { (*self) >= (*other) }
                 #[inline(always)]
                 fn gt(&self, other: &$t) -> bool { (*self) > (*other) }
+
+                chaining_methods_impl!($t);
             }
 
             #[stable(feature = "rust1", since = "1.0.0")]

core/src/tuple.rs

@@ -2,6 +2,7 @@
 
 use crate::cmp::Ordering::{self, *};
 use crate::marker::{ConstParamTy_, StructuralPartialEq, UnsizedConstParamTy};
+use crate::ops::ControlFlow::{Break, Continue};
 
 // Recursive macro for implementing n-ary tuple functions and operations
 //
@@ -80,19 +81,19 @@ macro_rules! tuple_impls {
                 }
                 #[inline]
                 fn lt(&self, other: &($($T,)+)) -> bool {
-                    lexical_ord!(lt, Less, $( ${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, Less, $( ${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, Greater, $( ${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, Greater, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
+                    lexical_ord!(gt, __chaining_gt, $( ${ignore($T)} self.${index()}, other.${index()} ),+)
                 }
             }
         }
@@ -171,15 +172,16 @@ 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`)
 //
-// `$ne_rel` is only used to determine the result after checking that they're
-// not equal, so `lt` and `le` can both just use `Less`.
+// `$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, $ne_rel: ident, $a:expr, $b:expr, $($rest_a:expr, $rest_b:expr),+) => {{
-        let c = PartialOrd::partial_cmp(&$a, &$b);
-        if c != Some(Equal) { c == Some($ne_rel) }
-        else { lexical_ord!($rel, $ne_rel, $($rest_a, $rest_b),+) }
+    ($rel: ident, $chain_rel: ident, $a:expr, $b:expr, $($rest_a:expr, $rest_b:expr),+) => {{
+        match PartialOrd::$chain_rel(&$a, &$b) {
+            Break(val) => val,
+            Continue(()) => lexical_ord!($rel, $chain_rel, $($rest_a, $rest_b),+),
+        }
     }};
-    ($rel: ident, $ne_rel: ident, $a:expr, $b:expr) => {
+    ($rel: ident, $chain_rel: ident, $a:expr, $b:expr) => {
         // Use the specific method for the last element
         PartialOrd::$rel(&$a, &$b)
     };