]> git.lizzy.rs Git - rust.git/commitdiff
Auto merge of #103659 - clubby789:improve-partialord-derive, r=nagisa
authorbors <bors@rust-lang.org>
Sat, 28 Jan 2023 22:11:11 +0000 (22:11 +0000)
committerbors <bors@rust-lang.org>
Sat, 28 Jan 2023 22:11:11 +0000 (22:11 +0000)
Special-case deriving `PartialOrd` for enums with dataless variants

I was able to get slightly better codegen by flipping the derived `PartialOrd` logic for two-variant enums.  I also tried to document the implementation of the derive macro to make the special-case logic a little clearer.
```rs
#[derive(PartialEq, PartialOrd)]
pub enum A<T> {
    A,
    B(T)
}
```
```diff
impl<T: ::core::cmp::PartialOrd> ::core::cmp::PartialOrd for A<T> {
   #[inline]
   fn partial_cmp(
       &self,
       other: &A<T>,
   ) -> ::core::option::Option<::core::cmp::Ordering> {
       let __self_tag = ::core::intrinsics::discriminant_value(self);
       let __arg1_tag = ::core::intrinsics::discriminant_value(other);
-      match ::core::cmp::PartialOrd::partial_cmp(&__self_tag, &__arg1_tag) {
-          ::core::option::Option::Some(::core::cmp::Ordering::Equal) => {
-              match (self, other) {
-                  (A::B(__self_0), A::B(__arg1_0)) => {
-                      ::core::cmp::PartialOrd::partial_cmp(__self_0, __arg1_0)
-                  }
-                  _ => ::core::option::Option::Some(::core::cmp::Ordering::Equal),
-              }
+      match (self, other) {
+          (A::B(__self_0), A::B(__arg1_0)) => {
+              ::core::cmp::PartialOrd::partial_cmp(__self_0, __arg1_0)
           }
-          cmp => cmp,
+          _ => ::core::cmp::PartialOrd::partial_cmp(&__self_tag, &__arg1_tag),
       }
   }
}
```
Godbolt: [Current](https://godbolt.org/z/GYjEzG1T8), [New](https://godbolt.org/z/GoK78qx15)
I'm not sure how common a case comparing two enums like this (such as `Option`) is, and if it's worth the slowdown of adding a special case to the derive. If it causes overall regressions it might be worth just manually implementing this for `Option`.

1  2 
compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs
tests/ui/deriving/deriving-all-codegen.stdout

index c9dc89212622d70d7d6ddc2bdbfac0093a7bf263,be247d415c7fede890dfc61d38f6ffbc1389e6d7..2fc30d8e05f2c6748c4e03ed0a07d056105e3439
@@@ -28,9 -49,9 +49,9 @@@ pub fn expand_deriving_partial_ord
          nonself_args: vec![(self_ref(), sym::other)],
          ret_ty,
          attributes: attrs,
 -        unify_fieldless_variants: true,
 +        fieldless_variants_strategy: FieldlessVariantsStrategy::Unify,
          combine_substructure: combine_substructure(Box::new(|cx, span, substr| {
-             cs_partial_cmp(cx, span, substr)
+             cs_partial_cmp(cx, span, substr, tag_then_data)
          })),
      };