Skip to content

Commit 361c599

Browse files
borsbors
committed
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 parents f241c0c + d4a5b03 commit 361c599

18 files changed

+38
-252
lines changed

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

+25-41
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,8 @@ pub fn expand_deriving_partial_eq(
1515
item: &Annotatable,
1616
push: &mut dyn FnMut(Annotatable),
1717
) {
18-
fn cs_op(
19-
cx: &mut ExtCtxt<'_>,
20-
span: Span,
21-
substr: &Substructure<'_>,
22-
op: BinOpKind,
23-
combiner: BinOpKind,
24-
base: bool,
25-
) -> BlockOrExpr {
18+
fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
19+
let base = true;
2620
let expr = cs_fold(
2721
true, // use foldl
2822
cx,
@@ -47,39 +41,22 @@ pub fn expand_deriving_partial_eq(
4741
cx.expr_deref(field.span, expr.clone())
4842
}
4943
};
50-
cx.expr_binary(field.span, op, convert(&field.self_expr), convert(other_expr))
44+
cx.expr_binary(
45+
field.span,
46+
BinOpKind::Eq,
47+
convert(&field.self_expr),
48+
convert(other_expr),
49+
)
50+
}
51+
CsFold::Combine(span, expr1, expr2) => {
52+
cx.expr_binary(span, BinOpKind::And, expr1, expr2)
5153
}
52-
CsFold::Combine(span, expr1, expr2) => cx.expr_binary(span, combiner, expr1, expr2),
5354
CsFold::Fieldless => cx.expr_bool(span, base),
5455
},
5556
);
5657
BlockOrExpr::new_expr(expr)
5758
}
5859

59-
fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
60-
cs_op(cx, span, substr, BinOpKind::Eq, BinOpKind::And, true)
61-
}
62-
fn cs_ne(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
63-
cs_op(cx, span, substr, BinOpKind::Ne, BinOpKind::Or, false)
64-
}
65-
66-
macro_rules! md {
67-
($name:expr, $f:ident) => {{
68-
let inline = cx.meta_word(span, sym::inline);
69-
let attrs = vec![cx.attribute(inline)];
70-
MethodDef {
71-
name: $name,
72-
generics: Bounds::empty(),
73-
explicit_self: true,
74-
nonself_args: vec![(self_ref(), sym::other)],
75-
ret_ty: Path(path_local!(bool)),
76-
attributes: attrs,
77-
unify_fieldless_variants: true,
78-
combine_substructure: combine_substructure(Box::new(|a, b, c| $f(a, b, c))),
79-
}
80-
}};
81-
}
82-
8360
super::inject_impl_of_structural_trait(
8461
cx,
8562
span,
@@ -88,13 +65,20 @@ pub fn expand_deriving_partial_eq(
8865
push,
8966
);
9067

91-
// avoid defining `ne` if we can
92-
// c-like enums, enums without any fields and structs without fields
93-
// can safely define only `eq`.
94-
let mut methods = vec![md!(sym::eq, cs_eq)];
95-
if !is_type_without_fields(item) {
96-
methods.push(md!(sym::ne, cs_ne));
97-
}
68+
// No need to generate `ne`, the default suffices, and not generating it is
69+
// faster.
70+
let inline = cx.meta_word(span, sym::inline);
71+
let attrs = vec![cx.attribute(inline)];
72+
let methods = vec![MethodDef {
73+
name: sym::eq,
74+
generics: Bounds::empty(),
75+
explicit_self: true,
76+
nonself_args: vec![(self_ref(), sym::other)],
77+
ret_ty: Path(path_local!(bool)),
78+
attributes: attrs,
79+
unify_fieldless_variants: true,
80+
combine_substructure: combine_substructure(Box::new(|a, b, c| cs_eq(a, b, c))),
81+
}];
9882

9983
let trait_def = TraitDef {
10084
span,

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

-16
Original file line numberDiff line numberDiff line change
@@ -1625,19 +1625,3 @@ where
16251625
StaticEnum(..) | StaticStruct(..) => cx.span_bug(trait_span, "static function in `derive`"),
16261626
}
16271627
}
1628-
1629-
/// Returns `true` if the type has no value fields
1630-
/// (for an enum, no variant has any fields)
1631-
pub fn is_type_without_fields(item: &Annotatable) -> bool {
1632-
if let Annotatable::Item(ref item) = *item {
1633-
match item.kind {
1634-
ast::ItemKind::Enum(ref enum_def, _) => {
1635-
enum_def.variants.iter().all(|v| v.data.fields().is_empty())
1636-
}
1637-
ast::ItemKind::Struct(ref variant_data, _) => variant_data.fields().is_empty(),
1638-
_ => false,
1639-
}
1640-
} else {
1641-
false
1642-
}
1643-
}

‎library/core/src/cmp.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@ use self::Ordering::*;
3838
///
3939
/// Implementations must ensure that `eq` and `ne` are consistent with each other:
4040
///
41-
/// - `a != b` if and only if `!(a == b)`
42-
/// (ensured by the default implementation).
41+
/// - `a != b` if and only if `!(a == b)`.
42+
///
43+
/// The default implementation of `ne` provides this consistency and is almost
44+
/// always sufficient. It should not be overridden without very good reason.
4345
///
4446
/// If [`PartialOrd`] or [`Ord`] are also implemented for `Self` and `Rhs`, their methods must also
4547
/// be consistent with `PartialEq` (see the documentation of those traits for the exact
@@ -225,7 +227,8 @@ pub trait PartialEq<Rhs: ?Sized = Self> {
225227
#[stable(feature = "rust1", since = "1.0.0")]
226228
fn eq(&self, other: &Rhs) -> bool;
227229

228-
/// This method tests for `!=`.
230+
/// This method tests for `!=`. The default implementation is almost always
231+
/// sufficient, and should not be overridden without very good reason.
229232
#[inline]
230233
#[must_use]
231234
#[stable(feature = "rust1", since = "1.0.0")]

‎src/test/run-make/coverage-reports/expected_show_coverage.doctest.txt

-6
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,6 @@
2323
22| 1|//! ```
2424
23| 2|//! #[derive(Debug, PartialEq)]
2525
^1
26-
------------------
27-
| Unexecuted instantiation: <rust_out::main::_doctest_main____coverage_doctest_rs_22_0::SomeError as core::cmp::PartialEq>::ne
28-
------------------
29-
| <rust_out::main::_doctest_main____coverage_doctest_rs_22_0::SomeError as core::cmp::PartialEq>::eq:
30-
| 23| 2|//! #[derive(Debug, PartialEq)]
31-
------------------
3226
24| 1|//! struct SomeError {
3327
25| 1|//! msg: String,
3428
26| 1|//! }

‎src/test/run-make/coverage-reports/expected_show_coverage.issue-83601.txt

-6
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,6 @@
22
2| |
33
3| 3|#[derive(Debug, PartialEq, Eq)]
44
^2
5-
------------------
6-
| <issue_83601::Foo as core::cmp::PartialEq>::eq:
7-
| 3| 2|#[derive(Debug, PartialEq, Eq)]
8-
------------------
9-
| Unexecuted instantiation: <issue_83601::Foo as core::cmp::PartialEq>::ne
10-
------------------
115
4| |struct Foo(u32);
126
5| |
137
6| 1|fn main() {

‎src/test/run-make/coverage-reports/expected_show_coverage.issue-84561.txt

-6
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,6 @@
22
2| |
33
3| |// expect-exit-status-101
44
4| 21|#[derive(PartialEq, Eq)]
5-
------------------
6-
| <issue_84561::Foo as core::cmp::PartialEq>::eq:
7-
| 4| 21|#[derive(PartialEq, Eq)]
8-
------------------
9-
| Unexecuted instantiation: <issue_84561::Foo as core::cmp::PartialEq>::ne
10-
------------------
115
5| |struct Foo(u32);
126
6| 1|fn test3() {
137
7| 1| let is_true = std::env::args().len() == 1;

‎src/test/run-make/coverage-reports/expected_show_coverage.partial_eq.txt

-5
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,6 @@
33
3| |
44
4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
55
^0 ^0 ^0 ^1 ^1 ^0^0
6-
------------------
7-
| Unexecuted instantiation: <partial_eq::Version as core::cmp::PartialEq>::ne
8-
------------------
9-
| Unexecuted instantiation: <partial_eq::Version as core::cmp::PartialEq>::eq
10-
------------------
116
5| |pub struct Version {
127
6| | major: usize,
138
7| | minor: usize,

‎src/test/ui/derives/derives-span-PartialEq-enum-struct-variant.rs

-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ struct Error;
77
enum Enum {
88
A {
99
x: Error //~ ERROR
10-
//~^ ERROR
1110
}
1211
}
1312

‎src/test/ui/derives/derives-span-PartialEq-enum-struct-variant.stderr

+1-21
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,6 @@ help: consider annotating `Error` with `#[derive(PartialEq)]`
1818
LL | #[derive(PartialEq)]
1919
|
2020

21-
error[E0369]: binary operation `!=` cannot be applied to type `Error`
22-
--> $DIR/derives-span-PartialEq-enum-struct-variant.rs:9:6
23-
|
24-
LL | #[derive(PartialEq)]
25-
| --------- in this derive macro expansion
26-
...
27-
LL | x: Error
28-
| ^^^^^^^^
29-
|
30-
note: an implementation of `PartialEq<_>` might be missing for `Error`
31-
--> $DIR/derives-span-PartialEq-enum-struct-variant.rs:4:1
32-
|
33-
LL | struct Error;
34-
| ^^^^^^^^^^^^ must implement `PartialEq<_>`
35-
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)
36-
help: consider annotating `Error` with `#[derive(PartialEq)]`
37-
|
38-
LL | #[derive(PartialEq)]
39-
|
40-
41-
error: aborting due to 2 previous errors
21+
error: aborting due to previous error
4222

4323
For more information about this error, try `rustc --explain E0369`.

‎src/test/ui/derives/derives-span-PartialEq-enum.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ struct Error;
77
enum Enum {
88
A(
99
Error //~ ERROR
10-
//~^ ERROR
11-
)
10+
)
1211
}
1312

1413
fn main() {}

‎src/test/ui/derives/derives-span-PartialEq-enum.stderr

+1-21
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,6 @@ help: consider annotating `Error` with `#[derive(PartialEq)]`
1818
LL | #[derive(PartialEq)]
1919
|
2020

21-
error[E0369]: binary operation `!=` cannot be applied to type `Error`
22-
--> $DIR/derives-span-PartialEq-enum.rs:9:6
23-
|
24-
LL | #[derive(PartialEq)]
25-
| --------- in this derive macro expansion
26-
...
27-
LL | Error
28-
| ^^^^^
29-
|
30-
note: an implementation of `PartialEq<_>` might be missing for `Error`
31-
--> $DIR/derives-span-PartialEq-enum.rs:4:1
32-
|
33-
LL | struct Error;
34-
| ^^^^^^^^^^^^ must implement `PartialEq<_>`
35-
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)
36-
help: consider annotating `Error` with `#[derive(PartialEq)]`
37-
|
38-
LL | #[derive(PartialEq)]
39-
|
40-
41-
error: aborting due to 2 previous errors
21+
error: aborting due to previous error
4222

4323
For more information about this error, try `rustc --explain E0369`.

‎src/test/ui/derives/derives-span-PartialEq-struct.rs

-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ struct Error;
66
#[derive(PartialEq)]
77
struct Struct {
88
x: Error //~ ERROR
9-
//~^ ERROR
109
}
1110

1211
fn main() {}

‎src/test/ui/derives/derives-span-PartialEq-struct.stderr

+1-21
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,6 @@ help: consider annotating `Error` with `#[derive(PartialEq)]`
1818
LL | #[derive(PartialEq)]
1919
|
2020

21-
error[E0369]: binary operation `!=` cannot be applied to type `Error`
22-
--> $DIR/derives-span-PartialEq-struct.rs:8:5
23-
|
24-
LL | #[derive(PartialEq)]
25-
| --------- in this derive macro expansion
26-
LL | struct Struct {
27-
LL | x: Error
28-
| ^^^^^^^^
29-
|
30-
note: an implementation of `PartialEq<_>` might be missing for `Error`
31-
--> $DIR/derives-span-PartialEq-struct.rs:4:1
32-
|
33-
LL | struct Error;
34-
| ^^^^^^^^^^^^ must implement `PartialEq<_>`
35-
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)
36-
help: consider annotating `Error` with `#[derive(PartialEq)]`
37-
|
38-
LL | #[derive(PartialEq)]
39-
|
40-
41-
error: aborting due to 2 previous errors
21+
error: aborting due to previous error
4222

4323
For more information about this error, try `rustc --explain E0369`.

‎src/test/ui/derives/derives-span-PartialEq-tuple-struct.rs

-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ struct Error;
66
#[derive(PartialEq)]
77
struct Struct(
88
Error //~ ERROR
9-
//~^ ERROR
109
);
1110

1211
fn main() {}

‎src/test/ui/derives/derives-span-PartialEq-tuple-struct.stderr

+1-21
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,6 @@ help: consider annotating `Error` with `#[derive(PartialEq)]`
1818
LL | #[derive(PartialEq)]
1919
|
2020

21-
error[E0369]: binary operation `!=` cannot be applied to type `Error`
22-
--> $DIR/derives-span-PartialEq-tuple-struct.rs:8:5
23-
|
24-
LL | #[derive(PartialEq)]
25-
| --------- in this derive macro expansion
26-
LL | struct Struct(
27-
LL | Error
28-
| ^^^^^
29-
|
30-
note: an implementation of `PartialEq<_>` might be missing for `Error`
31-
--> $DIR/derives-span-PartialEq-tuple-struct.rs:4:1
32-
|
33-
LL | struct Error;
34-
| ^^^^^^^^^^^^ must implement `PartialEq<_>`
35-
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)
36-
help: consider annotating `Error` with `#[derive(PartialEq)]`
37-
|
38-
LL | #[derive(PartialEq)]
39-
|
40-
41-
error: aborting due to 2 previous errors
21+
error: aborting due to previous error
4222

4323
For more information about this error, try `rustc --explain E0369`.

‎src/test/ui/derives/deriving-no-inner-impl-error-message.rs

-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ struct NoCloneOrEq;
33
#[derive(PartialEq)]
44
struct E {
55
x: NoCloneOrEq //~ ERROR binary operation `==` cannot be applied to type `NoCloneOrEq`
6-
//~^ ERROR binary operation `!=` cannot be applied to type `NoCloneOrEq`
76
}
87
#[derive(Clone)]
98
struct C {

‎src/test/ui/derives/deriving-no-inner-impl-error-message.stderr

+2-22
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,8 @@ help: consider annotating `NoCloneOrEq` with `#[derive(PartialEq)]`
1818
LL | #[derive(PartialEq)]
1919
|
2020

21-
error[E0369]: binary operation `!=` cannot be applied to type `NoCloneOrEq`
22-
--> $DIR/deriving-no-inner-impl-error-message.rs:5:5
23-
|
24-
LL | #[derive(PartialEq)]
25-
| --------- in this derive macro expansion
26-
LL | struct E {
27-
LL | x: NoCloneOrEq
28-
| ^^^^^^^^^^^^^^
29-
|
30-
note: an implementation of `PartialEq<_>` might be missing for `NoCloneOrEq`
31-
--> $DIR/deriving-no-inner-impl-error-message.rs:1:1
32-
|
33-
LL | struct NoCloneOrEq;
34-
| ^^^^^^^^^^^^^^^^^^ must implement `PartialEq<_>`
35-
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)
36-
help: consider annotating `NoCloneOrEq` with `#[derive(PartialEq)]`
37-
|
38-
LL | #[derive(PartialEq)]
39-
|
40-
4121
error[E0277]: the trait bound `NoCloneOrEq: Clone` is not satisfied
42-
--> $DIR/deriving-no-inner-impl-error-message.rs:10:5
22+
--> $DIR/deriving-no-inner-impl-error-message.rs:9:5
4323
|
4424
LL | #[derive(Clone)]
4525
| ----- in this derive macro expansion
@@ -53,7 +33,7 @@ help: consider annotating `NoCloneOrEq` with `#[derive(Clone)]`
5333
LL | #[derive(Clone)]
5434
|
5535

56-
error: aborting due to 3 previous errors
36+
error: aborting due to 2 previous errors
5737

5838
Some errors have detailed explanations: E0277, E0369.
5939
For more information about an error, try `rustc --explain E0277`.

0 commit comments

Comments
 (0)