Generate PartialEq::ne() (#475, #163)

tyranron · web-flow · commit b982f9025635 · 2025-06-02T09:45:02.000+02:00
Part of #163 ## Synopsis `derive(PartialEq)` in `std` generates only `PartialEq::eq()` method implementation in its expansion. This silently kills any potential performance benefits if the underlying types implement `PartialEq::ne()` more efficiently than the default implementation (`!PartialEq::eq()`). ## Solution Generate `PartialEq::ne()` implementation always as well, by structurally calling `!=` operator instead of `==`. ## Additionally Unfortunately, the `assert_ne!()` macro also doesn't call `PartialEq::ne()` or `!=` operator inside its expansion, and only negates the equality check. Found this when running tests against a purposely incorrect `PartialEq::ne()` implementation. That's why the `assert_ne!()` macro is redefined in `partial_eq` tests as `asset!(left != right)`.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,8 +19,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   ([#459](https://github.com/JelteF/derive_more/pull/459))
 - Support structs with no fields in `FromStr` derive.
   ([#469](https://github.com/JelteF/derive_more/pull/469))
-- Add `PartialEq` derive similar to `std`'s one, but considering generics correctly.
-  ([#473](https://github.com/JelteF/derive_more/pull/473))
+- Add `PartialEq` derive similar to `std`'s one, but considering generics correctly,
+  and implementing `ne()` method as well.
+  ([#473](https://github.com/JelteF/derive_more/pull/473),
+   [#475](https://github.com/JelteF/derive_more/pull/475))
 - Proxy-pass `#[allow]`/`#[expect]` attributes of the type in `Constructor` derive.
   ([#477](https://github.com/JelteF/derive_more/pull/477))
 
diff --git a/impl/doc/eq.md b/impl/doc/eq.md
@@ -9,8 +9,10 @@ their type structure.
 ## Structural equality
 
 Deriving `PartialEq` for enums/structs works in a similar way to the one in `std`,
-by comparing all the available fields, but, in the contrast, does not overconstrain
-generic parameters.
+by comparing all the available fields, but, in the contrast:
+1. Does not overconstrain generic parameters.
+2. Implements `PartialEq::ne()` method as well, to propagate possible efficient
+   implementations of this method from the underlying types.
 
 
 ### Structs
@@ -72,6 +74,15 @@ where
             }
         }
     }
+
+    #[inline]
+    fn ne(&self, other: &Self) -> bool {
+        match (self, other) {
+            (Self { a: self_0, b: self_1, c: self_2 }, Self { a: other_0, b: other_1, c: other_2 }) => {
+                self_0 != other_0 || self_1 != other_1 || self_2 != other_2
+            }
+        }
+    }
 }
 ```
 
@@ -143,5 +154,16 @@ where
                 _ => unsafe { std::hint::unreachable_unchecked() },
             }
     }
+
+    #[inline]
+    fn ne(&self, other: &Self) -> bool {
+        std::mem::discriminant(self) != std::mem::discriminant(other) ||
+            match (self, other) {
+                (Self::A(self_0), Self::A(other_0)) => { self_0 != other_0 }
+                (Self::B { b: self_0 }, Self::B { b: other_0 }) => { self_0 != other_0 }
+                (Self::C(self_0), Self::C(other_0)) => { self_0 != other_0 }
+                _ => unsafe { std::hint::unreachable_unchecked() },
+            }
+    }
 }
 ```
diff --git a/impl/src/partial_eq.rs b/impl/src/partial_eq.rs
@@ -2,7 +2,11 @@
 
 use proc_macro2::TokenStream;
 use quote::{format_ident, quote, ToTokens};
-use syn::{parse_quote, spanned::Spanned as _, token};
+use syn::{
+    parse_quote,
+    punctuated::{self, Punctuated},
+    spanned::Spanned as _,
+};
 
 use crate::utils::GenericsSearch;
 
@@ -52,54 +56,69 @@ impl<'i> TryFrom<&'i syn::DeriveInput> for StructuralExpansion<'i> {
 }
 
 impl StructuralExpansion<'_> {
-    /// Generates body of the [`PartialEq::eq()`] method implementation for this
-    /// [`StructuralExpansion`].
-    fn eq_body(&self) -> TokenStream {
-        // Special case: empty enum.
+    /// Generates body of the [`PartialEq::eq()`]/[`PartialEq::ne()`] method implementation for this
+    /// [`StructuralExpansion`], if it's required.
+    fn body(&self, eq: bool) -> Option<TokenStream> {
+        // Special case: empty enum (also, no need for `ne()` method in this case).
         if self.variants.is_empty() {
-            return quote! { match *self {} };
+            return eq.then(|| quote! { match *self {} });
         }
-        // Special case: no fields to compare.
+
+        let no_fields_result = quote! { #eq };
+
+        // Special case: no fields to compare (also, no need for `ne()` method in this case).
         if self.variants.len() == 1 && self.variants[0].1.is_empty() {
-            return quote! { true };
+            return eq.then_some(no_fields_result);
         }
 
-        let discriminants_eq = (self.variants.len() > 1).then(|| {
+        let (cmp, chain) = if eq {
+            (quote! { == }, quote! { && })
+        } else {
+            (quote! { != }, quote! { || })
+        };
+
+        let discriminants_cmp = (self.variants.len() > 1).then(|| {
             quote! {
-                derive_more::core::mem::discriminant(self) ==
+                derive_more::core::mem::discriminant(self) #cmp
                     derive_more::core::mem::discriminant(__other)
             }
         });
 
-        let matched_variants = self
+        let match_arms = self
             .variants
             .iter()
             .filter_map(|(variant, fields)| {
                 if fields.is_empty() {
                     return None;
                 }
+
                 let variant = variant.map(|variant| quote! { :: #variant });
                 let self_pattern = fields.arm_pattern("__self_");
                 let other_pattern = fields.arm_pattern("__other_");
-                let val_eqs = (0..fields.len()).map(|num| {
-                    let self_val = format_ident!("__self_{num}");
-                    let other_val = format_ident!("__other_{num}");
-                    quote! { #self_val == #other_val }
-                });
+
+                let mut val_eqs = (0..fields.len())
+                    .map(|num| {
+                        let self_val = format_ident!("__self_{num}");
+                        let other_val = format_ident!("__other_{num}");
+                        punctuated::Pair::Punctuated(
+                            quote! { #self_val #cmp #other_val },
+                            &chain,
+                        )
+                    })
+                    .collect::<Punctuated<TokenStream, _>>();
+                _ = val_eqs.pop_punct();
+
                 Some(quote! {
-                    (Self #variant #self_pattern, Self #variant #other_pattern) => {
-                        #( #val_eqs )&&*
-                    }
+                    (Self #variant #self_pattern, Self #variant #other_pattern) => { #val_eqs }
                 })
             })
             .collect::<Vec<_>>();
-        let match_expr = (!matched_variants.is_empty()).then(|| {
-            let always_true_arm =
-                (matched_variants.len() != self.variants.len()).then(|| {
-                    quote! { _ => true }
-                });
+        let match_expr = (!match_arms.is_empty()).then(|| {
+            let no_fields_arm = (match_arms.len() != self.variants.len()).then(|| {
+                quote! { _ => #no_fields_result }
+            });
             let unreachable_arm = (self.variants.len() > 1
-                && always_true_arm.is_none())
+                && no_fields_arm.is_none())
             .then(|| {
                 quote! {
                     // SAFETY: This arm is never reachable, but is required by the expanded
@@ -110,19 +129,25 @@ impl StructuralExpansion<'_> {
 
             quote! {
                 match (self, __other) {
-                    #( #matched_variants , )*
-                    #always_true_arm
+                    #( #match_arms , )*
+                    #no_fields_arm
                     #unreachable_arm
                 }
             }
         });
 
-        let and = (discriminants_eq.is_some() && match_expr.is_some())
-            .then_some(token::AndAnd::default());
-
-        quote! {
-            #discriminants_eq #and #match_expr
+        // If there is only `mem::discriminant()` comparison, there is no need to generate `ne()`
+        // method in the expansion, as its default implementation will do just fine.
+        if !eq && discriminants_cmp.is_some() && match_expr.is_none() {
+            return None;
         }
+
+        let chain =
+            (discriminants_cmp.is_some() && match_expr.is_some()).then_some(chain);
+
+        Some(quote! {
+            #discriminants_cmp #chain #match_expr
+        })
     }
 }
 
@@ -143,17 +168,26 @@ impl ToTokens for StructuralExpansion<'_> {
         }
         let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
 
-        let eq_body = self.eq_body();
+        let eq_method = self.body(true).map(|body| {
+            quote! {
+                #[inline]
+                fn eq(&self, __other: &Self) -> bool { #body }
+            }
+        });
+        let ne_method = self.body(false).map(|body| {
+            quote! {
+                #[inline]
+                fn ne(&self, __other: &Self) -> bool { #body }
+            }
+        });
 
         quote! {
             #[automatically_derived]
             impl #impl_generics derive_more::core::cmp::PartialEq for #ty #ty_generics
                  #where_clause
             {
-                #[inline]
-                fn eq(&self, __other: &Self) -> bool {
-                    #eq_body
-                }
+                #eq_method
+                #ne_method
             }
         }
         .to_tokens(tokens);
diff --git a/tests/partial_eq.rs b/tests/partial_eq.rs
@@ -1,6 +1,18 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 #![allow(dead_code)] // some code is tested for type checking only
 
+/// Since [`assert_ne!()`] macro in [`core`] doesn't use `$left != $right` comparison, but rather
+/// checks as `!($left == $right)`, it should be redefined for tests to consider actual
+/// [`PartialEq::ne()`] implementations.
+///
+/// [`assert_ne!()`]: core::assert_ne
+#[macro_export]
+macro_rules! assert_ne {
+    ($left:expr, $right:expr $(,)?) => {
+        assert!($left != $right)
+    };
+}
+
 mod structs {
     mod structural {
         use derive_more::PartialEq;
