Check equality of scalar types first.

Author: Sol-Ell

compiler/rustc_ast/src/ast.rs

@@ -2195,6 +2195,34 @@ impl FloatTy {
     }
 }
 
+impl<'a> TryFrom<&'a str> for FloatTy {
+    type Error = ();
+
+    fn try_from(value: &'a str) -> Result<Self, Self::Error> {
+        Ok(match value {
+            "f16" => Self::F16,
+            "f32" => Self::F32,
+            "f64" => Self::F64,
+            "f128" => Self::F128,
+            _ => return Err(()),
+        })
+    }
+}
+
+impl TryFrom<Symbol> for FloatTy {
+    type Error = ();
+
+    fn try_from(value: Symbol) -> Result<Self, Self::Error> {
+        Ok(match value {
+            sym::f16 => Self::F16,
+            sym::f32 => Self::F32,
+            sym::f64 => Self::F64,
+            sym::f128 => Self::F128,
+            _ => return Err(()),
+        })
+    }
+}
+
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 #[derive(Encodable, Decodable, HashStable_Generic)]
 pub enum IntTy {
@@ -2230,6 +2258,38 @@ impl IntTy {
     }
 }
 
+impl<'a> TryFrom<&'a str> for IntTy {
+    type Error = ();
+
+    fn try_from(value: &'a str) -> Result<Self, Self::Error> {
+        Ok(match value {
+            "isize" => Self::Isize,
+            "i8" => Self::I8,
+            "i16" => Self::I16,
+            "i32" => Self::I32,
+            "i64" => Self::I64,
+            "i128" => Self::I128,
+            _ => return Err(()),
+        })
+    }
+}
+
+impl TryFrom<Symbol> for IntTy {
+    type Error = ();
+
+    fn try_from(value: Symbol) -> Result<Self, Self::Error> {
+        Ok(match value {
+            sym::isize => Self::Isize,
+            sym::i8 => Self::I8,
+            sym::i16 => Self::I16,
+            sym::i32 => Self::I32,
+            sym::i64 => Self::I64,
+            sym::i128 => Self::I128,
+            _ => return Err(()),
+        })
+    }
+}
+
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Debug)]
 #[derive(Encodable, Decodable, HashStable_Generic)]
 pub enum UintTy {
@@ -2265,6 +2325,38 @@ impl UintTy {
     }
 }
 
+impl<'a> TryFrom<&'a str> for UintTy {
+    type Error = ();
+
+    fn try_from(value: &'a str) -> Result<Self, Self::Error> {
+        Ok(match value {
+            "usize" => Self::Usize,
+            "u8" => Self::U8,
+            "u16" => Self::U16,
+            "u32" => Self::U32,
+            "u64" => Self::U64,
+            "u128" => Self::U128,
+            _ => return Err(()),
+        })
+    }
+}
+
+impl TryFrom<Symbol> for UintTy {
+    type Error = ();
+
+    fn try_from(value: Symbol) -> Result<Self, Self::Error> {
+        Ok(match value {
+            sym::usize => Self::Usize,
+            sym::u8 => Self::U8,
+            sym::u16 => Self::U16,
+            sym::u32 => Self::U32,
+            sym::u64 => Self::U64,
+            sym::u128 => Self::U128,
+            _ => return Err(()),
+        })
+    }
+}
+
 /// A constraint on an associated item.
 ///
 /// ### Examples
@@ -2452,6 +2544,21 @@ impl TyKind {
             None
         }
     }
+
+    pub fn is_scalar(&self) -> bool {
+        let Some(ty_kind) = self.is_simple_path() else {
+            match self {
+                TyKind::Tup(tys) => return tys.is_empty(), // unit type
+                _ => return false,
+            }
+        };
+
+        ty_kind == sym::bool
+            || ty_kind == sym::char
+            || IntTy::try_from(ty_kind).is_ok()
+            || UintTy::try_from(ty_kind).is_ok()
+            || FloatTy::try_from(ty_kind).is_ok()
+    }
 }
 
 /// A pattern type pattern.

compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs

@@ -66,7 +66,8 @@ pub(crate) fn cs_cmp(cx: &ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) ->
                 let args = thin_vec![field.self_expr.clone(), other_expr.clone()];
                 cx.expr_call_global(field.span, cmp_path.clone(), args)
             }
-            CsFold::Combine(span, expr1, expr2) => {
+            CsFold::Combine { field_info, field_expr: expr2, other_expr: expr1 } => {
+                let span = field_info.span;
                 let eq_arm = cx.arm(span, cx.pat_path(span, equal_path.clone()), expr1);
                 let neq_arm =
                     cx.arm(span, cx.pat_ident(span, test_id), cx.expr_ident(span, test_id));

compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs

@@ -18,6 +18,7 @@ pub(crate) fn expand_deriving_partial_eq(
 ) {
     fn cs_eq(cx: &ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
         let base = true;
+
         let expr = cs_fold(
             true, // use foldl
             cx,
@@ -63,8 +64,12 @@ pub(crate) fn expand_deriving_partial_eq(
                         convert(other_expr),
                     )
                 }
-                CsFold::Combine(span, expr1, expr2) => {
-                    cx.expr_binary(span, BinOpKind::And, expr1, expr2)
+                CsFold::Combine { field_info, mut field_expr, mut other_expr } => {
+                    // Comparison of numeric primitives is cheap, therefore compare them first.
+                    if field_info.is_scalar {
+                        (field_expr, other_expr) = (other_expr, field_expr);
+                    }
+                    cx.expr_binary(field_info.span, BinOpKind::And, other_expr, field_expr)
                 }
                 CsFold::Fieldless => cx.expr_bool(span, base),
             },

compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs

@@ -100,7 +100,7 @@ fn cs_partial_cmp(
                 let args = thin_vec![field.self_expr.clone(), other_expr.clone()];
                 cx.expr_call_global(field.span, partial_cmp_path.clone(), args)
             }
-            CsFold::Combine(span, mut expr1, expr2) => {
+            CsFold::Combine { field_info, field_expr: expr2, other_expr: mut expr1 } => {
                 // When the item is an enum, this expands to
                 // ```
                 // match (expr2) {
@@ -130,6 +130,7 @@ fn cs_partial_cmp(
                 // }
                 // ```
                 // Reference: https://github.com/rust-lang/rust/pull/103659#issuecomment-1328126354
+                let span = field_info.span;
 
                 if !discr_then_data
                     && let ExprKind::Match(_, arms, _) = &mut expr1.kind

compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

@@ -284,6 +284,7 @@ pub(crate) struct FieldInfo {
     /// The expressions corresponding to references to this field in
     /// the other selflike arguments.
     pub other_selflike_exprs: Vec<P<Expr>>,
+    pub is_scalar: bool,
 }
 
 #[derive(Copy, Clone)]
@@ -1220,7 +1221,8 @@ impl<'a> MethodDef<'a> {
 
             let self_expr = discr_exprs.remove(0);
             let other_selflike_exprs = discr_exprs;
-            let discr_field = FieldInfo { span, name: None, self_expr, other_selflike_exprs };
+            let discr_field =
+                FieldInfo { span, name: None, self_expr, other_selflike_exprs, is_scalar: true };
 
             let discr_let_stmts: ThinVec<_> = iter::zip(&discr_idents, &selflike_args)
                 .map(|(&ident, selflike_arg)| {
@@ -1533,6 +1535,7 @@ impl<'a> TraitDef<'a> {
                     name: struct_field.ident,
                     self_expr,
                     other_selflike_exprs,
+                    is_scalar: struct_field.ty.peel_refs().kind.is_scalar(),
                 }
             })
             .collect()
@@ -1607,7 +1610,11 @@ pub(crate) enum CsFold<'a> {
 
     /// The combination of two field expressions. E.g. for `PartialEq::eq` this
     /// is something like `<field1 equality> && <field2 equality>`.
-    Combine(Span, P<Expr>, P<Expr>),
+    Combine {
+        field_info: &'a FieldInfo,
+        field_expr: P<Expr>,
+        other_expr: P<Expr>,
+    },
 
     // The fallback case for a struct or enum variant with no fields.
     Fieldless,
@@ -1641,7 +1648,7 @@ where
 
             let op = |old, field: &FieldInfo| {
                 let new = f(cx, CsFold::Single(field));
-                f(cx, CsFold::Combine(field.span, old, new))
+                f(cx, CsFold::Combine { field_info: field, field_expr: new, other_expr: old })
             };
 
             if use_foldl {
@@ -1653,11 +1660,14 @@ where
         EnumDiscr(discr_field, match_expr) => {
             let discr_check_expr = f(cx, CsFold::Single(discr_field));
             if let Some(match_expr) = match_expr {
-                if use_foldl {
-                    f(cx, CsFold::Combine(trait_span, discr_check_expr, match_expr.clone()))
-                } else {
-                    f(cx, CsFold::Combine(trait_span, match_expr.clone(), discr_check_expr))
-                }
+                f(
+                    cx,
+                    CsFold::Combine {
+                        field_info: discr_field,
+                        field_expr: discr_check_expr,
+                        other_expr: match_expr.clone(),
+                    },
+                )
             } else {
                 discr_check_expr
             }

tests/ui/deriving/deriving-all-codegen.rs

@@ -45,6 +45,23 @@ struct Big {
     b1: u32, b2: u32, b3: u32, b4: u32, b5: u32, b6: u32, b7: u32, b8: u32,
 }
 
+// `PartialEq` must reorder scalar types to be first and preserve the order of
+// non-scalar types. In contrast, `PartialOrd` must compare by order of declaration.
+#[derive(PartialEq, PartialOrd)]
+struct Reorder {
+    b1: Option<f32>,
+    b2: u16,
+    b3: &'static str,
+    b4: i8,
+    b5: u128,
+    _b: *mut &'static dyn FnMut() -> (),
+    b6: f64,
+    b7: &'static mut (),
+    b8: char,
+    b9: &'static [i64],
+    b10: &'static *const bool,
+}
+
 // A struct that doesn't impl `Copy`, which means it gets the non-simple
 // `clone` implemention that clones the fields individually.
 #[derive(Clone)]
@@ -130,6 +147,22 @@ enum Mixed {
     S { d1: Option<u32>, d2: Option<i32> },
 }
 
+// `PartialEq` must reorder scalar types of `G`, but preserve the order of
+// other fields of that variant. In contrast, `PartialOrd` must compare by
+// order of declaration.
+#[derive(PartialEq, PartialOrd)]
+enum ReorderEnum {
+    A(i32),
+    B,
+    C(i8),
+    D,
+    E,
+    F,
+    G(&'static mut str, *const u8, *const dyn Fn() -> ()),
+    H,
+    I,
+}
+
 // An enum with no fieldless variants. Note that `Default` cannot be derived
 // for this enum.
 #[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]

tests/ui/deriving/deriving-all-codegen.stdout

@@ -126,7 +126,7 @@ impl ::core::marker::StructuralPartialEq for Point { }
 impl ::core::cmp::PartialEq for Point {
     #[inline]
     fn eq(&self, other: &Point) -> bool {
-        self.x == other.x && self.y == other.y
+        self.y == other.y && self.x == other.x
     }
 }
 #[automatically_derived]
@@ -211,7 +211,7 @@ impl ::core::marker::StructuralPartialEq for PackedPoint { }
 impl ::core::cmp::PartialEq for PackedPoint {
     #[inline]
     fn eq(&self, other: &PackedPoint) -> bool {
-        ({ self.x }) == ({ other.x }) && ({ self.y }) == ({ other.y })
+        ({ self.y }) == ({ other.y }) && ({ self.x }) == ({ other.x })
     }
 }
 #[automatically_derived]
@@ -320,10 +320,13 @@ impl ::core::marker::StructuralPartialEq for Big { }
 impl ::core::cmp::PartialEq for Big {
     #[inline]
     fn eq(&self, other: &Big) -> bool {
-        self.b1 == other.b1 && self.b2 == other.b2 && self.b3 == other.b3 &&
-                            self.b4 == other.b4 && self.b5 == other.b5 &&
-                    self.b6 == other.b6 && self.b7 == other.b7 &&
-            self.b8 == other.b8
+        self.b8 == other.b8 &&
+            (self.b7 == other.b7 &&
+                (self.b6 == other.b6 &&
+                    (self.b5 == other.b5 &&
+                        (self.b4 == other.b4 &&
+                            (self.b3 == other.b3 &&
+                                (self.b2 == other.b2 && self.b1 == other.b1))))))
     }
 }
 #[automatically_derived]
@@ -419,6 +422,105 @@ impl ::core::cmp::Ord for Big {
     }
 }
 
+// `PartialEq` must reorder scalar types to be first and preserve the order of
+// non-scalar types. In contrast, `PartialOrd` must compare by order of declaration.
+struct Reorder {
+    b1: Option<f32>,
+    b2: u16,
+    b3: &'static str,
+    b4: i8,
+    b5: u128,
+    _b: *mut &'static dyn FnMut() -> (),
+    b6: f64,
+    b7: &'static mut (),
+    b8: char,
+    b9: &'static [i64],
+    b10: &'static *const bool,
+}
+#[automatically_derived]
+impl ::core::marker::StructuralPartialEq for Reorder { }
+#[automatically_derived]
+impl ::core::cmp::PartialEq for Reorder {
+    #[inline]
+    fn eq(&self, other: &Reorder) -> bool {
+        self.b10 == other.b10 &&
+            (self.b8 == other.b8 &&
+                    (self.b7 == other.b7 &&
+                        (self.b6 == other.b6 &&
+                            (self.b5 == other.b5 &&
+                                    (self.b4 == other.b4 &&
+                                        (self.b2 == other.b2 && self.b1 == other.b1 &&
+                                            self.b3 == other.b3)) && self._b == other._b))) &&
+                self.b9 == other.b9)
+    }
+}
+#[automatically_derived]
+impl ::core::cmp::PartialOrd for Reorder {
+    #[inline]
+    fn partial_cmp(&self, other: &Reorder)
+        -> ::core::option::Option<::core::cmp::Ordering> {
+        match ::core::cmp::PartialOrd::partial_cmp(&self.b1, &other.b1) {
+            ::core::option::Option::Some(::core::cmp::Ordering::Equal) =>
+                match ::core::cmp::PartialOrd::partial_cmp(&self.b2,
+                        &other.b2) {
+                    ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                        =>
+                        match ::core::cmp::PartialOrd::partial_cmp(&self.b3,
+                                &other.b3) {
+                            ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                =>
+                                match ::core::cmp::PartialOrd::partial_cmp(&self.b4,
+                                        &other.b4) {
+                                    ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                        =>
+                                        match ::core::cmp::PartialOrd::partial_cmp(&self.b5,
+                                                &other.b5) {
+                                            ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                                =>
+                                                match ::core::cmp::PartialOrd::partial_cmp(&self._b,
+                                                        &other._b) {
+                                                    ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                                        =>
+                                                        match ::core::cmp::PartialOrd::partial_cmp(&self.b6,
+                                                                &other.b6) {
+                                                            ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                                                =>
+                                                                match ::core::cmp::PartialOrd::partial_cmp(&self.b7,
+                                                                        &other.b7) {
+                                                                    ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                                                        =>
+                                                                        match ::core::cmp::PartialOrd::partial_cmp(&self.b8,
+                                                                                &other.b8) {
+                                                                            ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                                                                =>
+                                                                                match ::core::cmp::PartialOrd::partial_cmp(&self.b9,
+                                                                                        &other.b9) {
+                                                                                    ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                                                                        =>
+                                                                                        ::core::cmp::PartialOrd::partial_cmp(&self.b10, &other.b10),
+                                                                                    cmp => cmp,
+                                                                                },
+                                                                            cmp => cmp,
+                                                                        },
+                                                                    cmp => cmp,
+                                                                },
+                                                            cmp => cmp,
+                                                        },
+                                                    cmp => cmp,
+                                                },
+                                            cmp => cmp,
+                                        },
+                                    cmp => cmp,
+                                },
+                            cmp => cmp,
+                        },
+                    cmp => cmp,
+                },
+            cmp => cmp,
+        }
+    }
+}
+
 // A struct that doesn't impl `Copy`, which means it gets the non-simple
 // `clone` implemention that clones the fields individually.
 struct NonCopy(u32);
@@ -1167,6 +1269,79 @@ impl ::core::cmp::Ord for Mixed {
     }
 }
 
+// `PartialEq` must reorder scalar types of `G`, but preserve the order of
+// other fields of that variant. In contrast, `PartialOrd` must compare by
+// order of declaration.
+enum ReorderEnum {
+    A(i32),
+    B,
+    C(i8),
+    D,
+    E,
+    F,
+    G(&'static mut str, *const u8, *const dyn Fn() -> ()),
+    H,
+    I,
+}
+#[automatically_derived]
+impl ::core::marker::StructuralPartialEq for ReorderEnum { }
+#[automatically_derived]
+impl ::core::cmp::PartialEq for ReorderEnum {
+    #[inline]
+    fn eq(&self, other: &ReorderEnum) -> bool {
+        let __self_discr = ::core::intrinsics::discriminant_value(self);
+        let __arg1_discr = ::core::intrinsics::discriminant_value(other);
+        __self_discr == __arg1_discr &&
+            match (self, other) {
+                (ReorderEnum::A(__self_0), ReorderEnum::A(__arg1_0)) =>
+                    __self_0 == __arg1_0,
+                (ReorderEnum::C(__self_0), ReorderEnum::C(__arg1_0)) =>
+                    __self_0 == __arg1_0,
+                (ReorderEnum::G(__self_0, __self_1, __self_2),
+                    ReorderEnum::G(__arg1_0, __arg1_1, __arg1_2)) =>
+                    __self_1 == __arg1_1 && __self_0 == __arg1_0 &&
+                        __self_2 == __arg1_2,
+                _ => true,
+            }
+    }
+}
+#[automatically_derived]
+impl ::core::cmp::PartialOrd for ReorderEnum {
+    #[inline]
+    fn partial_cmp(&self, other: &ReorderEnum)
+        -> ::core::option::Option<::core::cmp::Ordering> {
+        let __self_discr = ::core::intrinsics::discriminant_value(self);
+        let __arg1_discr = ::core::intrinsics::discriminant_value(other);
+        match ::core::cmp::PartialOrd::partial_cmp(&__self_discr,
+                &__arg1_discr) {
+            ::core::option::Option::Some(::core::cmp::Ordering::Equal) =>
+                match (self, other) {
+                    (ReorderEnum::A(__self_0), ReorderEnum::A(__arg1_0)) =>
+                        ::core::cmp::PartialOrd::partial_cmp(__self_0, __arg1_0),
+                    (ReorderEnum::C(__self_0), ReorderEnum::C(__arg1_0)) =>
+                        ::core::cmp::PartialOrd::partial_cmp(__self_0, __arg1_0),
+                    (ReorderEnum::G(__self_0, __self_1, __self_2),
+                        ReorderEnum::G(__arg1_0, __arg1_1, __arg1_2)) =>
+                        match ::core::cmp::PartialOrd::partial_cmp(__self_0,
+                                __arg1_0) {
+                            ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                =>
+                                match ::core::cmp::PartialOrd::partial_cmp(__self_1,
+                                        __arg1_1) {
+                                    ::core::option::Option::Some(::core::cmp::Ordering::Equal)
+                                        => ::core::cmp::PartialOrd::partial_cmp(__self_2, __arg1_2),
+                                    cmp => cmp,
+                                },
+                            cmp => cmp,
+                        },
+                    _ =>
+                        ::core::option::Option::Some(::core::cmp::Ordering::Equal),
+                },
+            cmp => cmp,
+        }
+    }
+}
+
 // An enum with no fieldless variants. Note that `Default` cannot be derived
 // for this enum.
 enum Fielded { X(u32), Y(bool), Z(Option<i32>), }