Skip to content

Commit 126e3df

Browse files
borsbors
committed
Auto merge of #98376 - nnethercote:improve-derive-PartialEq, r=petrochenkov
Improve some deriving code and add a test The `.stdout` test is particularly useful. r? `@petrochenkov`
2 parents 2953edc + 02d2cdf commit 126e3df

File tree

7 files changed

+1251
-131
lines changed

7 files changed

+1251
-131
lines changed

‎compiler/rustc_builtin_macros/src/deriving/clone.rs

+21-27
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ use crate::deriving::generic::*;
33
use crate::deriving::path_std;
44

55
use rustc_ast::ptr::P;
6-
use rustc_ast::{self as ast, Expr, GenericArg, Generics, ItemKind, MetaItem, VariantData};
6+
use rustc_ast::{self as ast, Expr, Generics, ItemKind, MetaItem, VariantData};
77
use rustc_expand::base::{Annotatable, ExtCtxt};
8-
use rustc_span::symbol::{kw, sym, Ident, Symbol};
8+
use rustc_span::symbol::{kw, sym, Ident};
99
use rustc_span::Span;
1010

1111
pub fn expand_deriving_clone(
@@ -107,44 +107,38 @@ fn cs_clone_shallow(
107107
substr: &Substructure<'_>,
108108
is_union: bool,
109109
) -> P<Expr> {
110-
fn assert_ty_bounds(
111-
cx: &mut ExtCtxt<'_>,
112-
stmts: &mut Vec<ast::Stmt>,
113-
ty: P<ast::Ty>,
114-
span: Span,
115-
helper_name: &str,
116-
) {
117-
// Generate statement `let _: helper_name<ty>;`,
118-
// set the expn ID so we can use the unstable struct.
119-
let span = cx.with_def_site_ctxt(span);
120-
let assert_path = cx.path_all(
121-
span,
122-
true,
123-
cx.std_path(&[sym::clone, Symbol::intern(helper_name)]),
124-
vec![GenericArg::Type(ty)],
125-
);
126-
stmts.push(cx.stmt_let_type_only(span, cx.ty_path(assert_path)));
127-
}
128-
fn process_variant(cx: &mut ExtCtxt<'_>, stmts: &mut Vec<ast::Stmt>, variant: &VariantData) {
110+
let mut stmts = Vec::new();
111+
let mut process_variant = |variant: &VariantData| {
129112
for field in variant.fields() {
130113
// let _: AssertParamIsClone<FieldTy>;
131-
assert_ty_bounds(cx, stmts, field.ty.clone(), field.span, "AssertParamIsClone");
114+
super::assert_ty_bounds(
115+
cx,
116+
&mut stmts,
117+
field.ty.clone(),
118+
field.span,
119+
&[sym::clone, sym::AssertParamIsClone],
120+
);
132121
}
133-
}
122+
};
134123

135-
let mut stmts = Vec::new();
136124
if is_union {
137125
// let _: AssertParamIsCopy<Self>;
138126
let self_ty = cx.ty_path(cx.path_ident(trait_span, Ident::with_dummy_span(kw::SelfUpper)));
139-
assert_ty_bounds(cx, &mut stmts, self_ty, trait_span, "AssertParamIsCopy");
127+
super::assert_ty_bounds(
128+
cx,
129+
&mut stmts,
130+
self_ty,
131+
trait_span,
132+
&[sym::clone, sym::AssertParamIsCopy],
133+
);
140134
} else {
141135
match *substr.fields {
142136
StaticStruct(vdata, ..) => {
143-
process_variant(cx, &mut stmts, vdata);
137+
process_variant(vdata);
144138
}
145139
StaticEnum(enum_def, ..) => {
146140
for variant in &enum_def.variants {
147-
process_variant(cx, &mut stmts, &variant.data);
141+
process_variant(&variant.data);
148142
}
149143
}
150144
_ => cx.span_bug(

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

+14-30
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ use crate::deriving::generic::*;
33
use crate::deriving::path_std;
44

55
use rustc_ast::ptr::P;
6-
use rustc_ast::{self as ast, Expr, GenericArg, MetaItem};
6+
use rustc_ast::{self as ast, Expr, MetaItem};
77
use rustc_expand::base::{Annotatable, ExtCtxt};
8-
use rustc_span::symbol::{sym, Ident, Symbol};
8+
use rustc_span::symbol::{sym, Ident};
99
use rustc_span::Span;
1010

1111
pub fn expand_deriving_eq(
@@ -55,43 +55,27 @@ fn cs_total_eq_assert(
5555
trait_span: Span,
5656
substr: &Substructure<'_>,
5757
) -> P<Expr> {
58-
fn assert_ty_bounds(
59-
cx: &mut ExtCtxt<'_>,
60-
stmts: &mut Vec<ast::Stmt>,
61-
ty: P<ast::Ty>,
62-
span: Span,
63-
helper_name: &str,
64-
) {
65-
// Generate statement `let _: helper_name<ty>;`,
66-
// set the expn ID so we can use the unstable struct.
67-
let span = cx.with_def_site_ctxt(span);
68-
let assert_path = cx.path_all(
69-
span,
70-
true,
71-
cx.std_path(&[sym::cmp, Symbol::intern(helper_name)]),
72-
vec![GenericArg::Type(ty)],
73-
);
74-
stmts.push(cx.stmt_let_type_only(span, cx.ty_path(assert_path)));
75-
}
76-
fn process_variant(
77-
cx: &mut ExtCtxt<'_>,
78-
stmts: &mut Vec<ast::Stmt>,
79-
variant: &ast::VariantData,
80-
) {
58+
let mut stmts = Vec::new();
59+
let mut process_variant = |variant: &ast::VariantData| {
8160
for field in variant.fields() {
8261
// let _: AssertParamIsEq<FieldTy>;
83-
assert_ty_bounds(cx, stmts, field.ty.clone(), field.span, "AssertParamIsEq");
62+
super::assert_ty_bounds(
63+
cx,
64+
&mut stmts,
65+
field.ty.clone(),
66+
field.span,
67+
&[sym::cmp, sym::AssertParamIsEq],
68+
);
8469
}
85-
}
70+
};
8671

87-
let mut stmts = Vec::new();
8872
match *substr.fields {
8973
StaticStruct(vdata, ..) => {
90-
process_variant(cx, &mut stmts, vdata);
74+
process_variant(vdata);
9175
}
9276
StaticEnum(enum_def, ..) => {
9377
for variant in &enum_def.variants {
94-
process_variant(cx, &mut stmts, &variant.data);
78+
process_variant(&variant.data);
9579
}
9680
}
9781
_ => cx.span_bug(trait_span, "unexpected substructure in `derive(Eq)`"),

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

+36-73
Original file line numberDiff line numberDiff line change
@@ -1126,75 +1126,43 @@ impl<'a> MethodDef<'a> {
11261126
/// A1,
11271127
/// A2(i32)
11281128
/// }
1129-
///
1130-
/// // is equivalent to
1131-
///
1132-
/// impl PartialEq for A {
1129+
/// ```
1130+
/// is equivalent to:
1131+
/// ```
1132+
/// impl ::core::cmp::PartialEq for A {
1133+
/// #[inline]
11331134
/// fn eq(&self, other: &A) -> bool {
1134-
/// use A::*;
1135-
/// match (&*self, &*other) {
1136-
/// (&A1, &A1) => true,
1137-
/// (&A2(ref self_0),
1138-
/// &A2(ref __arg_1_0)) => (*self_0).eq(&(*__arg_1_0)),
1139-
/// _ => {
1140-
/// let __self_vi = match *self { A1 => 0, A2(..) => 1 };
1141-
/// let __arg_1_vi = match *other { A1 => 0, A2(..) => 1 };
1142-
/// false
1135+
/// {
1136+
/// let __self_vi = ::core::intrinsics::discriminant_value(&*self);
1137+
/// let __arg_1_vi = ::core::intrinsics::discriminant_value(&*other);
1138+
/// if true && __self_vi == __arg_1_vi {
1139+
/// match (&*self, &*other) {
1140+
/// (&A::A2(ref __self_0), &A::A2(ref __arg_1_0)) =>
1141+
/// (*__self_0) == (*__arg_1_0),
1142+
/// _ => true,
1143+
/// }
1144+
/// } else {
1145+
/// false // catch-all handler
11431146
/// }
11441147
/// }
11451148
/// }
11461149
/// }
11471150
/// ```
1148-
///
1149-
/// (Of course `__self_vi` and `__arg_1_vi` are unused for
1150-
/// `PartialEq`, and those subcomputations will hopefully be removed
1151-
/// as their results are unused. The point of `__self_vi` and
1152-
/// `__arg_1_vi` is for `PartialOrd`; see #15503.)
1153-
fn expand_enum_method_body<'b>(
1154-
&self,
1155-
cx: &mut ExtCtxt<'_>,
1156-
trait_: &TraitDef<'b>,
1157-
enum_def: &'b EnumDef,
1158-
type_ident: Ident,
1159-
self_args: Vec<P<Expr>>,
1160-
nonself_args: &[P<Expr>],
1161-
) -> P<Expr> {
1162-
self.build_enum_match_tuple(cx, trait_, enum_def, type_ident, self_args, nonself_args)
1163-
}
1164-
11651151
/// Creates a match for a tuple of all `self_args`, where either all
11661152
/// variants match, or it falls into a catch-all for when one variant
11671153
/// does not match.
1168-
1154+
///
11691155
/// There are N + 1 cases because is a case for each of the N
11701156
/// variants where all of the variants match, and one catch-all for
11711157
/// when one does not match.
1172-
1158+
///
11731159
/// As an optimization we generate code which checks whether all variants
11741160
/// match first which makes llvm see that C-like enums can be compiled into
11751161
/// a simple equality check (for PartialEq).
1176-
1162+
///
11771163
/// The catch-all handler is provided access the variant index values
11781164
/// for each of the self-args, carried in precomputed variables.
1179-
1180-
/// ```{.text}
1181-
/// let __self0_vi = std::intrinsics::discriminant_value(&self);
1182-
/// let __self1_vi = std::intrinsics::discriminant_value(&arg1);
1183-
/// let __self2_vi = std::intrinsics::discriminant_value(&arg2);
1184-
///
1185-
/// if __self0_vi == __self1_vi && __self0_vi == __self2_vi && ... {
1186-
/// match (...) {
1187-
/// (Variant1, Variant1, ...) => Body1
1188-
/// (Variant2, Variant2, ...) => Body2,
1189-
/// ...
1190-
/// _ => ::core::intrinsics::unreachable()
1191-
/// }
1192-
/// }
1193-
/// else {
1194-
/// ... // catch-all remainder can inspect above variant index values.
1195-
/// }
1196-
/// ```
1197-
fn build_enum_match_tuple<'b>(
1165+
fn expand_enum_method_body<'b>(
11981166
&self,
11991167
cx: &mut ExtCtxt<'_>,
12001168
trait_: &TraitDef<'b>,
@@ -1392,37 +1360,32 @@ impl<'a> MethodDef<'a> {
13921360
//
13931361
// i.e., for `enum E<T> { A, B(1), C(T, T) }`, and a deriving
13941362
// with three Self args, builds three statements:
1395-
//
13961363
// ```
1397-
// let __self0_vi = std::intrinsics::discriminant_value(&self);
1398-
// let __self1_vi = std::intrinsics::discriminant_value(&arg1);
1399-
// let __self2_vi = std::intrinsics::discriminant_value(&arg2);
1364+
// let __self_vi = std::intrinsics::discriminant_value(&self);
1365+
// let __arg_1_vi = std::intrinsics::discriminant_value(&arg1);
1366+
// let __arg_2_vi = std::intrinsics::discriminant_value(&arg2);
14001367
// ```
14011368
let mut index_let_stmts: Vec<ast::Stmt> = Vec::with_capacity(vi_idents.len() + 1);
14021369

1403-
// We also build an expression which checks whether all discriminants are equal
1404-
// discriminant_test = __self0_vi == __self1_vi && __self0_vi == __self2_vi && ...
1370+
// We also build an expression which checks whether all discriminants are equal:
1371+
// `__self_vi == __arg_1_vi && __self_vi == __arg_2_vi && ...`
14051372
let mut discriminant_test = cx.expr_bool(span, true);
1406-
1407-
let mut first_ident = None;
1408-
for (&ident, self_arg) in iter::zip(&vi_idents, &self_args) {
1373+
for (i, (&ident, self_arg)) in iter::zip(&vi_idents, &self_args).enumerate() {
14091374
let self_addr = cx.expr_addr_of(span, self_arg.clone());
14101375
let variant_value =
14111376
deriving::call_intrinsic(cx, span, sym::discriminant_value, vec![self_addr]);
14121377
let let_stmt = cx.stmt_let(span, false, ident, variant_value);
14131378
index_let_stmts.push(let_stmt);
14141379

1415-
match first_ident {
1416-
Some(first) => {
1417-
let first_expr = cx.expr_ident(span, first);
1418-
let id = cx.expr_ident(span, ident);
1419-
let test = cx.expr_binary(span, BinOpKind::Eq, first_expr, id);
1420-
discriminant_test =
1421-
cx.expr_binary(span, BinOpKind::And, discriminant_test, test)
1422-
}
1423-
None => {
1424-
first_ident = Some(ident);
1425-
}
1380+
if i > 0 {
1381+
let id0 = cx.expr_ident(span, vi_idents[0]);
1382+
let id = cx.expr_ident(span, ident);
1383+
let test = cx.expr_binary(span, BinOpKind::Eq, id0, id);
1384+
discriminant_test = if i == 1 {
1385+
test
1386+
} else {
1387+
cx.expr_binary(span, BinOpKind::And, discriminant_test, test)
1388+
};
14261389
}
14271390
}
14281391

@@ -1453,7 +1416,7 @@ impl<'a> MethodDef<'a> {
14531416
// }
14541417
// }
14551418
// else {
1456-
// <delegated expression referring to __self0_vi, et al.>
1419+
// <delegated expression referring to __self_vi, et al.>
14571420
// }
14581421
let all_match = cx.expr_match(span, match_arg, match_arms);
14591422
let arm_expr = cx.expr_if(span, discriminant_test, all_match, Some(arm_expr));

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

+14-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use rustc_ast as ast;
44
use rustc_ast::ptr::P;
5-
use rustc_ast::{Impl, ItemKind, MetaItem};
5+
use rustc_ast::{GenericArg, Impl, ItemKind, MetaItem};
66
use rustc_expand::base::{Annotatable, ExpandResult, ExtCtxt, MultiItemModifier};
77
use rustc_span::symbol::{sym, Ident, Symbol};
88
use rustc_span::Span;
@@ -193,3 +193,16 @@ fn inject_impl_of_structural_trait(
193193

194194
push(Annotatable::Item(newitem));
195195
}
196+
197+
fn assert_ty_bounds(
198+
cx: &mut ExtCtxt<'_>,
199+
stmts: &mut Vec<ast::Stmt>,
200+
ty: P<ast::Ty>,
201+
span: Span,
202+
assert_path: &[Symbol],
203+
) {
204+
// Generate statement `let _: assert_path<ty>;`.
205+
let span = cx.with_def_site_ctxt(span);
206+
let assert_path = cx.path_all(span, true, cx.std_path(assert_path), vec![GenericArg::Type(ty)]);
207+
stmts.push(cx.stmt_let_type_only(span, cx.ty_path(assert_path)));
208+
}

‎compiler/rustc_span/src/symbol.rs

+3
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ symbols! {
135135
Arguments,
136136
AsMut,
137137
AsRef,
138+
AssertParamIsClone,
139+
AssertParamIsCopy,
140+
AssertParamIsEq,
138141
AtomicBool,
139142
AtomicI128,
140143
AtomicI16,

0 commit comments

Comments
 (0)