Skip to content

Commit fbdb07f

Browse files
borsbors
committed
Auto merge of rust-lang#98758 - nnethercote:more-derive-output-improvements, r=Mark-Simulacrum
More derive output improvements This PR includes: - Some test improvements. - Some cosmetic changes to derive output that make the code look more like what a human would write. - Some more fundamental improvements to `cmp` and `partial_cmp` generation. r? `@Mark-Simulacrum`
2 parents 1dcff2d + 0da063c commit fbdb07f

File tree

15 files changed

+655
-568
lines changed

15 files changed

+655
-568
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,6 +2036,14 @@ impl TyKind {
20362036
pub fn is_unit(&self) -> bool {
20372037
matches!(self, TyKind::Tup(tys) if tys.is_empty())
20382038
}
2039+
2040+
pub fn is_simple_path(&self) -> Option<Symbol> {
2041+
if let TyKind::Path(None, Path { segments, .. }) = &self && segments.len() == 1 {
2042+
Some(segments[0].ident.name)
2043+
} else {
2044+
None
2045+
}
2046+
}
20392047
}
20402048

20412049
/// Syntax used to declare a trait object.

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

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use crate::deriving::generic::ty::*;
22
use crate::deriving::generic::*;
33
use crate::deriving::path_std;
44

5-
use rustc_ast::ptr::P;
6-
use rustc_ast::{self as ast, Expr, Generics, ItemKind, MetaItem, VariantData};
5+
use rustc_ast::{self as ast, Generics, ItemKind, MetaItem, VariantData};
6+
use rustc_data_structures::fx::FxHashSet;
77
use rustc_expand::base::{Annotatable, ExtCtxt};
88
use rustc_span::symbol::{kw, sym, Ident};
99
use rustc_span::Span;
@@ -98,22 +98,31 @@ fn cs_clone_simple(
9898
trait_span: Span,
9999
substr: &Substructure<'_>,
100100
is_union: bool,
101-
) -> P<Expr> {
101+
) -> BlockOrExpr {
102102
let mut stmts = Vec::new();
103+
let mut seen_type_names = FxHashSet::default();
103104
let mut process_variant = |variant: &VariantData| {
104105
for field in variant.fields() {
105-
// let _: AssertParamIsClone<FieldTy>;
106-
super::assert_ty_bounds(
107-
cx,
108-
&mut stmts,
109-
field.ty.clone(),
110-
field.span,
111-
&[sym::clone, sym::AssertParamIsClone],
112-
);
106+
// This basic redundancy checking only prevents duplication of
107+
// assertions like `AssertParamIsClone<Foo>` where the type is a
108+
// simple name. That's enough to get a lot of cases, though.
109+
if let Some(name) = field.ty.kind.is_simple_path() && !seen_type_names.insert(name) {
110+
// Already produced an assertion for this type.
111+
} else {
112+
// let _: AssertParamIsClone<FieldTy>;
113+
super::assert_ty_bounds(
114+
cx,
115+
&mut stmts,
116+
field.ty.clone(),
117+
field.span,
118+
&[sym::clone, sym::AssertParamIsClone],
119+
);
120+
}
113121
}
114122
};
115123

116124
if is_union {
125+
// Just a single assertion for unions, that the union impls `Copy`.
117126
// let _: AssertParamIsCopy<Self>;
118127
let self_ty = cx.ty_path(cx.path_ident(trait_span, Ident::with_dummy_span(kw::SelfUpper)));
119128
super::assert_ty_bounds(
@@ -139,16 +148,15 @@ fn cs_clone_simple(
139148
),
140149
}
141150
}
142-
stmts.push(cx.stmt_expr(cx.expr_deref(trait_span, cx.expr_self(trait_span))));
143-
cx.expr_block(cx.block(trait_span, stmts))
151+
BlockOrExpr::new_mixed(stmts, cx.expr_deref(trait_span, cx.expr_self(trait_span)))
144152
}
145153

146154
fn cs_clone(
147155
name: &str,
148156
cx: &mut ExtCtxt<'_>,
149157
trait_span: Span,
150158
substr: &Substructure<'_>,
151-
) -> P<Expr> {
159+
) -> BlockOrExpr {
152160
let ctor_path;
153161
let all_fields;
154162
let fn_path = cx.std_path(&[sym::clone, sym::Clone, sym::clone]);
@@ -177,7 +185,7 @@ fn cs_clone(
177185
}
178186
}
179187

180-
match *vdata {
188+
let expr = match *vdata {
181189
VariantData::Struct(..) => {
182190
let fields = all_fields
183191
.iter()
@@ -201,5 +209,6 @@ fn cs_clone(
201209
cx.expr_call(trait_span, path, subcalls)
202210
}
203211
VariantData::Unit(..) => cx.expr_path(ctor_path),
204-
}
212+
};
213+
BlockOrExpr::new_expr(expr)
205214
}

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

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use crate::deriving::generic::ty::*;
22
use crate::deriving::generic::*;
33
use crate::deriving::path_std;
44

5-
use rustc_ast::ptr::P;
6-
use rustc_ast::{self as ast, Expr, MetaItem};
5+
use rustc_ast::{self as ast, MetaItem};
6+
use rustc_data_structures::fx::FxHashSet;
77
use rustc_expand::base::{Annotatable, ExtCtxt};
88
use rustc_span::symbol::{sym, Ident};
99
use rustc_span::Span;
@@ -52,18 +52,26 @@ fn cs_total_eq_assert(
5252
cx: &mut ExtCtxt<'_>,
5353
trait_span: Span,
5454
substr: &Substructure<'_>,
55-
) -> P<Expr> {
55+
) -> BlockOrExpr {
5656
let mut stmts = Vec::new();
57+
let mut seen_type_names = FxHashSet::default();
5758
let mut process_variant = |variant: &ast::VariantData| {
5859
for field in variant.fields() {
59-
// let _: AssertParamIsEq<FieldTy>;
60-
super::assert_ty_bounds(
61-
cx,
62-
&mut stmts,
63-
field.ty.clone(),
64-
field.span,
65-
&[sym::cmp, sym::AssertParamIsEq],
66-
);
60+
// This basic redundancy checking only prevents duplication of
61+
// assertions like `AssertParamIsEq<Foo>` where the type is a
62+
// simple name. That's enough to get a lot of cases, though.
63+
if let Some(name) = field.ty.kind.is_simple_path() && !seen_type_names.insert(name) {
64+
// Already produced an assertion for this type.
65+
} else {
66+
// let _: AssertParamIsEq<FieldTy>;
67+
super::assert_ty_bounds(
68+
cx,
69+
&mut stmts,
70+
field.ty.clone(),
71+
field.span,
72+
&[sym::cmp, sym::AssertParamIsEq],
73+
);
74+
}
6775
}
6876
};
6977

@@ -78,5 +86,5 @@ fn cs_total_eq_assert(
7886
}
7987
_ => cx.span_bug(trait_span, "unexpected substructure in `derive(Eq)`"),
8088
}
81-
cx.expr_block(cx.block(trait_span, stmts))
89+
BlockOrExpr::new_stmts(stmts)
8290
}

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ 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, MetaItem};
6+
use rustc_ast::{self as ast, MetaItem};
77
use rustc_expand::base::{Annotatable, ExtCtxt};
88
use rustc_span::symbol::{sym, Ident};
99
use rustc_span::Span;
@@ -51,7 +51,7 @@ pub fn ordering_collapsed(
5151
cx.expr_call_global(span, fn_cmp_path, vec![lft, rgt])
5252
}
5353

54-
pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<Expr> {
54+
pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
5555
let test_id = Ident::new(sym::cmp, span);
5656
let equals_path = cx.path_global(span, cx.std_path(&[sym::cmp, sym::Ordering, sym::Equal]));
5757

@@ -70,7 +70,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<
7070
// cmp => cmp
7171
// }
7272
//
73-
cs_fold(
73+
let expr = cs_fold(
7474
// foldr nests the if-elses correctly, leaving the first field
7575
// as the outermost one, and the last as the innermost.
7676
false,
@@ -79,15 +79,12 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<
7979
// ::std::cmp::Ordering::Equal => old,
8080
// cmp => cmp
8181
// }
82-
8382
let new = {
8483
let [other_f] = other_fs else {
8584
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`");
8685
};
87-
8886
let args =
8987
vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())];
90-
9188
cx.expr_call_global(span, cmp_path.clone(), args)
9289
};
9390

@@ -96,7 +93,21 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<
9693

9794
cx.expr_match(span, new, vec![eq_arm, neq_arm])
9895
},
99-
cx.expr_path(equals_path.clone()),
96+
|cx, args| match args {
97+
Some((span, self_f, other_fs)) => {
98+
let new = {
99+
let [other_f] = other_fs else {
100+
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`");
101+
};
102+
let args =
103+
vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())];
104+
cx.expr_call_global(span, cmp_path.clone(), args)
105+
};
106+
107+
new
108+
}
109+
None => cx.expr_path(equals_path.clone()),
110+
},
100111
Box::new(|cx, span, tag_tuple| {
101112
if tag_tuple.len() != 2 {
102113
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`")
@@ -107,5 +118,6 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<
107118
cx,
108119
span,
109120
substr,
110-
)
121+
);
122+
BlockOrExpr::new_expr(expr)
111123
}

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,14 @@ pub fn expand_deriving_partial_eq(
1515
item: &Annotatable,
1616
push: &mut dyn FnMut(Annotatable),
1717
) {
18-
// structures are equal if all fields are equal, and non equal, if
19-
// any fields are not equal or if the enum variants are different
2018
fn cs_op(
2119
cx: &mut ExtCtxt<'_>,
2220
span: Span,
2321
substr: &Substructure<'_>,
2422
op: BinOpKind,
2523
combiner: BinOpKind,
2624
base: bool,
27-
) -> P<Expr> {
25+
) -> BlockOrExpr {
2826
let op = |cx: &mut ExtCtxt<'_>, span: Span, self_f: P<Expr>, other_fs: &[P<Expr>]| {
2927
let [other_f] = other_fs else {
3028
cx.span_bug(span, "not exactly 2 arguments in `derive(PartialEq)`");
@@ -33,7 +31,7 @@ pub fn expand_deriving_partial_eq(
3331
cx.expr_binary(span, op, self_f, other_f.clone())
3432
};
3533

36-
cs_fold1(
34+
let expr = cs_fold(
3735
true, // use foldl
3836
|cx, span, subexpr, self_f, other_fs| {
3937
let eq = op(cx, span, self_f, other_fs);
@@ -52,13 +50,14 @@ pub fn expand_deriving_partial_eq(
5250
cx,
5351
span,
5452
substr,
55-
)
53+
);
54+
BlockOrExpr::new_expr(expr)
5655
}
5756

58-
fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<Expr> {
57+
fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
5958
cs_op(cx, span, substr, BinOpKind::Eq, BinOpKind::And, true)
6059
}
61-
fn cs_ne(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<Expr> {
60+
fn cs_ne(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
6261
cs_op(cx, span, substr, BinOpKind::Ne, BinOpKind::Or, false)
6362
}
6463

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,10 @@ pub fn expand_deriving_partial_ord(
4848
trait_def.expand(cx, mitem, item, push)
4949
}
5050

51-
pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<Expr> {
51+
pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
5252
let test_id = Ident::new(sym::cmp, span);
5353
let ordering = cx.path_global(span, cx.std_path(&[sym::cmp, sym::Ordering, sym::Equal]));
5454
let ordering_expr = cx.expr_path(ordering.clone());
55-
let equals_expr = cx.expr_some(span, ordering_expr);
5655

5756
let partial_cmp_path = cx.std_path(&[sym::cmp, sym::PartialOrd, sym::partial_cmp]);
5857

@@ -69,7 +68,7 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
6968
// cmp => cmp
7069
// }
7170
//
72-
cs_fold(
71+
let expr = cs_fold(
7372
// foldr nests the if-elses correctly, leaving the first field
7473
// as the outermost one, and the last as the innermost.
7574
false,
@@ -95,7 +94,21 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
9594

9695
cx.expr_match(span, new, vec![eq_arm, neq_arm])
9796
},
98-
equals_expr,
97+
|cx: &mut ExtCtxt<'_>, args: Option<(Span, P<Expr>, &[P<Expr>])>| match args {
98+
Some((span, self_f, other_fs)) => {
99+
let new = {
100+
let [other_f] = other_fs else {
101+
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`");
102+
};
103+
let args =
104+
vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())];
105+
cx.expr_call_global(span, partial_cmp_path.clone(), args)
106+
};
107+
108+
new
109+
}
110+
None => cx.expr_some(span, ordering_expr.clone()),
111+
},
99112
Box::new(|cx, span, tag_tuple| {
100113
if tag_tuple.len() != 2 {
101114
cx.span_bug(span, "not exactly 2 arguments in `derive(PartialOrd)`")
@@ -110,5 +123,6 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
110123
cx,
111124
span,
112125
substr,
113-
)
126+
);
127+
BlockOrExpr::new_expr(expr)
114128
}

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ use crate::deriving::generic::ty::*;
22
use crate::deriving::generic::*;
33
use crate::deriving::path_std;
44

5-
use rustc_ast::ptr::P;
6-
use rustc_ast::{self as ast, Expr, MetaItem};
5+
use rustc_ast::{self as ast, MetaItem};
76
use rustc_expand::base::{Annotatable, ExtCtxt};
87
use rustc_span::symbol::{sym, Ident, Symbol};
98
use rustc_span::Span;
@@ -42,7 +41,7 @@ pub fn expand_deriving_debug(
4241
trait_def.expand(cx, mitem, item, push)
4342
}
4443

45-
fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<Expr> {
44+
fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
4645
let (ident, vdata, fields) = match substr.fields {
4746
Struct(vdata, fields) => (substr.type_ident, *vdata, fields),
4847
EnumMatching(_, _, v, fields) => (v.ident, &v.data, fields),
@@ -74,7 +73,8 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
7473
if fields.is_empty() {
7574
// Special case for no fields.
7675
let fn_path_write_str = cx.std_path(&[sym::fmt, sym::Formatter, sym::write_str]);
77-
cx.expr_call_global(span, fn_path_write_str, vec![fmt, name])
76+
let expr = cx.expr_call_global(span, fn_path_write_str, vec![fmt, name]);
77+
BlockOrExpr::new_expr(expr)
7878
} else if fields.len() <= CUTOFF {
7979
// Few enough fields that we can use a specific-length method.
8080
let debug = if is_struct {
@@ -100,7 +100,8 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
100100
let field = cx.expr_addr_of(field.span, field);
101101
args.push(field);
102102
}
103-
cx.expr_call_global(span, fn_path_debug, args)
103+
let expr = cx.expr_call_global(span, fn_path_debug, args);
104+
BlockOrExpr::new_expr(expr)
104105
} else {
105106
// Enough fields that we must use the any-length method.
106107
let mut name_exprs = Vec::with_capacity(fields.len());
@@ -176,8 +177,6 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
176177
stmts.push(names_let.unwrap());
177178
}
178179
stmts.push(values_let);
179-
stmts.push(cx.stmt_expr(expr));
180-
181-
cx.expr_block(cx.block(span, stmts))
180+
BlockOrExpr::new_mixed(stmts, expr)
182181
}
183182
}

0 commit comments

Comments
 (0)