Auto merge of #98655 - nnethercote:dont-derive-PartialEq-ne, r=dtolnay

Don't derive `PartialEq::ne`.

Currently we skip deriving `PartialEq::ne` for C-like (fieldless) enums
and empty structs, thus reyling on the default `ne`. This behaviour is
unnecessarily conservative, because the `PartialEq` docs say this:

> Implementations must ensure that eq and ne are consistent with each other:
>
> `a != b` if and only if `!(a == b)` (ensured by the default
> implementation).

This means that the default implementation (`!(a == b)`) is always good
enough. So this commit changes things such that `ne` is never derived.

The motivation for this change is that not deriving `ne` reduces compile
times and binary sizes.

Observable behaviour may change if a user has defined a type `A` with an
inconsistent `PartialEq` and then defines a type `B` that contains an
`A` and also derives `PartialEq`. Such code is already buggy and
preserving bug-for-bug compatibility isn't necessary.

Two side-effects of the change:
- There is only one error message produced for types where `PartialEq`
  cannot be derived, instead of two.
- For coverage reports, some warnings about generated `ne` methods not
  being executed have disappeared.

Both side-effects seem fine, and possibly preferable.

2 files changed, 25 insertions, 57 deletions

diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
index 73be9362e4e..6f86092ba49 100644
--- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
+++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
@@ -15,14 +15,8 @@ pub fn expand_deriving_partial_eq(
     item: &Annotatable,
     push: &mut dyn FnMut(Annotatable),
 ) {
-    fn cs_op(
-        cx: &mut ExtCtxt<'_>,
-        span: Span,
-        substr: &Substructure<'_>,
-        op: BinOpKind,
-        combiner: BinOpKind,
-        base: bool,
-    ) -> BlockOrExpr {
+    fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
+        let base = true;
         let expr = cs_fold(
             true, // use foldl
             cx,
@@ -47,39 +41,22 @@ pub fn expand_deriving_partial_eq(
                             cx.expr_deref(field.span, expr.clone())
                         }
                     };
-                    cx.expr_binary(field.span, op, convert(&field.self_expr), convert(other_expr))
+                    cx.expr_binary(
+                        field.span,
+                        BinOpKind::Eq,
+                        convert(&field.self_expr),
+                        convert(other_expr),
+                    )
+                }
+                CsFold::Combine(span, expr1, expr2) => {
+                    cx.expr_binary(span, BinOpKind::And, expr1, expr2)
                 }
-                CsFold::Combine(span, expr1, expr2) => cx.expr_binary(span, combiner, expr1, expr2),
                 CsFold::Fieldless => cx.expr_bool(span, base),
             },
         );
         BlockOrExpr::new_expr(expr)
     }
 
-    fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
-        cs_op(cx, span, substr, BinOpKind::Eq, BinOpKind::And, true)
-    }
-    fn cs_ne(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
-        cs_op(cx, span, substr, BinOpKind::Ne, BinOpKind::Or, false)
-    }
-
-    macro_rules! md {
-        ($name:expr, $f:ident) => {{
-            let inline = cx.meta_word(span, sym::inline);
-            let attrs = vec![cx.attribute(inline)];
-            MethodDef {
-                name: $name,
-                generics: Bounds::empty(),
-                explicit_self: true,
-                nonself_args: vec![(self_ref(), sym::other)],
-                ret_ty: Path(path_local!(bool)),
-                attributes: attrs,
-                unify_fieldless_variants: true,
-                combine_substructure: combine_substructure(Box::new(|a, b, c| $f(a, b, c))),
-            }
-        }};
-    }
-
     super::inject_impl_of_structural_trait(
         cx,
         span,
@@ -88,13 +65,20 @@ pub fn expand_deriving_partial_eq(
         push,
     );
 
-    // avoid defining `ne` if we can
-    // c-like enums, enums without any fields and structs without fields
-    // can safely define only `eq`.
-    let mut methods = vec![md!(sym::eq, cs_eq)];
-    if !is_type_without_fields(item) {
-        methods.push(md!(sym::ne, cs_ne));
-    }
+    // No need to generate `ne`, the default suffices, and not generating it is
+    // faster.
+    let inline = cx.meta_word(span, sym::inline);
+    let attrs = vec![cx.attribute(inline)];
+    let methods = vec![MethodDef {
+        name: sym::eq,
+        generics: Bounds::empty(),
+        explicit_self: true,
+        nonself_args: vec![(self_ref(), sym::other)],
+        ret_ty: Path(path_local!(bool)),
+        attributes: attrs,
+        unify_fieldless_variants: true,
+        combine_substructure: combine_substructure(Box::new(|a, b, c| cs_eq(a, b, c))),
+    }];
 
     let trait_def = TraitDef {
         span,
diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
index 9882f7958aa..a39b97d07ef 100644
--- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
+++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
@@ -1625,19 +1625,3 @@ where
         StaticEnum(..) | StaticStruct(..) => cx.span_bug(trait_span, "static function in `derive`"),
     }
 }
-
-/// Returns `true` if the type has no value fields
-/// (for an enum, no variant has any fields)
-pub fn is_type_without_fields(item: &Annotatable) -> bool {
-    if let Annotatable::Item(ref item) = *item {
-        match item.kind {
-            ast::ItemKind::Enum(ref enum_def, _) => {
-                enum_def.variants.iter().all(|v| v.data.fields().is_empty())
-            }
-            ast::ItemKind::Struct(ref variant_data, _) => variant_data.fields().is_empty(),
-            _ => false,
-        }
-    } else {
-        false
-    }
-}