Skip to content

Commit

Permalink
Rollup merge of #98741 - nnethercote:deriving-cleanups, r=Mark-Simula…
Browse files Browse the repository at this point in the history
…crum

Many small deriving cleanups

These commits remove lots of little unnecessary things, and clarifies other things.

r? `@Mark-Simulacrum`
  • Loading branch information
Dylan-DPC authored Jul 1, 2022
2 parents 528202f + 85e8d94 commit c0cf363
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 304 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_builtin_macros/src/deriving/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ pub fn expand_deriving_copy(
path: path_std!(marker::Copy),
additional_bounds: Vec::new(),
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: true,
methods: Vec::new(),
associated_types: Vec::new(),
Expand Down
56 changes: 24 additions & 32 deletions compiler/rustc_builtin_macros/src/deriving/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,22 @@ pub fn expand_deriving_clone(
item: &Annotatable,
push: &mut dyn FnMut(Annotatable),
) {
// check if we can use a short form
// The simple form is `fn clone(&self) -> Self { *self }`, possibly with
// some additional `AssertParamIsClone` assertions.
//
// the short form is `fn clone(&self) -> Self { *self }`
//
// we can use the short form if:
// - the item is Copy (unfortunately, all we can check is whether it's also deriving Copy)
// - there are no generic parameters (after specialization this limitation can be removed)
// if we used the short form with generics, we'd have to bound the generics with
// Clone + Copy, and then there'd be no Clone impl at all if the user fills in something
// that is Clone but not Copy. and until specialization we can't write both impls.
// - the item is a union with Copy fields
// Unions with generic parameters still can derive Clone because they require Copy
// for deriving, Clone alone is not enough.
// Wherever Clone is implemented for fields is irrelevant so we don't assert it.
// We can use the simple form if either of the following are true.
// - The type derives Copy and there are no generic parameters. (If we
// used the simple form with generics, we'd have to bound the generics
// with Clone + Copy, and then there'd be no Clone impl at all if the
// user fills in something that is Clone but not Copy. After
// specialization we can remove this no-generics limitation.)
// - The item is a union. (Unions with generic parameters still can derive
// Clone because they require Copy for deriving, Clone alone is not
// enough. Whether Clone is implemented for fields is irrelevant so we
// don't assert it.)
let bounds;
let substructure;
let is_shallow;
let is_simple;
match *item {
Annotatable::Item(ref annitem) => match annitem.kind {
ItemKind::Struct(_, Generics { ref params, .. })
Expand All @@ -44,30 +43,25 @@ pub fn expand_deriving_clone(
.any(|param| matches!(param.kind, ast::GenericParamKind::Type { .. }))
{
bounds = vec![];
is_shallow = true;
is_simple = true;
substructure = combine_substructure(Box::new(|c, s, sub| {
cs_clone_shallow("Clone", c, s, sub, false)
cs_clone_simple("Clone", c, s, sub, false)
}));
} else {
bounds = vec![];
is_shallow = false;
is_simple = false;
substructure =
combine_substructure(Box::new(|c, s, sub| cs_clone("Clone", c, s, sub)));
}
}
ItemKind::Union(..) => {
bounds = vec![Literal(path_std!(marker::Copy))];
is_shallow = true;
bounds = vec![Path(path_std!(marker::Copy))];
is_simple = true;
substructure = combine_substructure(Box::new(|c, s, sub| {
cs_clone_shallow("Clone", c, s, sub, true)
cs_clone_simple("Clone", c, s, sub, true)
}));
}
_ => {
bounds = vec![];
is_shallow = false;
substructure =
combine_substructure(Box::new(|c, s, sub| cs_clone("Clone", c, s, sub)));
}
_ => cx.span_bug(span, "`#[derive(Clone)]` on wrong item kind"),
},

_ => cx.span_bug(span, "`#[derive(Clone)]` on trait item or impl item"),
Expand All @@ -81,26 +75,24 @@ pub fn expand_deriving_clone(
path: path_std!(clone::Clone),
additional_bounds: bounds,
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: true,
methods: vec![MethodDef {
name: sym::clone,
generics: Bounds::empty(),
explicit_self: borrowed_explicit_self(),
explicit_self: true,
args: Vec::new(),
ret_ty: Self_,
attributes: attrs,
is_unsafe: false,
unify_fieldless_variants: false,
combine_substructure: substructure,
}],
associated_types: Vec::new(),
};

trait_def.expand_ext(cx, mitem, item, push, is_shallow)
trait_def.expand_ext(cx, mitem, item, push, is_simple)
}

fn cs_clone_shallow(
fn cs_clone_simple(
name: &str,
cx: &mut ExtCtxt<'_>,
trait_span: Span,
Expand Down Expand Up @@ -143,7 +135,7 @@ fn cs_clone_shallow(
}
_ => cx.span_bug(
trait_span,
&format!("unexpected substructure in shallow `derive({})`", name),
&format!("unexpected substructure in simple `derive({})`", name),
),
}
}
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,14 @@ pub fn expand_deriving_eq(
path: path_std!(cmp::Eq),
additional_bounds: Vec::new(),
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: true,
methods: vec![MethodDef {
name: sym::assert_receiver_is_total_eq,
generics: Bounds::empty(),
explicit_self: borrowed_explicit_self(),
explicit_self: true,
args: vec![],
ret_ty: nil_ty(),
ret_ty: Unit,
attributes: attrs,
is_unsafe: false,
unify_fieldless_variants: true,
combine_substructure: combine_substructure(Box::new(|a, b, c| {
cs_total_eq_assert(a, b, c)
Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,14 @@ pub fn expand_deriving_ord(
path: path_std!(cmp::Ord),
additional_bounds: Vec::new(),
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: false,
methods: vec![MethodDef {
name: sym::cmp,
generics: Bounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![(borrowed_self(), sym::other)],
ret_ty: Literal(path_std!(cmp::Ordering)),
explicit_self: true,
args: vec![(self_ref(), sym::other)],
ret_ty: Path(path_std!(cmp::Ordering)),
attributes: attrs,
is_unsafe: false,
unify_fieldless_variants: true,
combine_substructure: combine_substructure(Box::new(|a, b, c| cs_cmp(a, b, c))),
}],
Expand Down Expand Up @@ -99,8 +97,8 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<
cx.expr_match(span, new, vec![eq_arm, neq_arm])
},
cx.expr_path(equals_path.clone()),
Box::new(|cx, span, (self_args, tag_tuple), _non_self_args| {
if self_args.len() != 2 {
Box::new(|cx, span, tag_tuple| {
if tag_tuple.len() != 2 {
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`")
} else {
ordering_collapsed(cx, span, tag_tuple)
Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub fn expand_deriving_partial_eq(
None => cx.expr_bool(span, base),
}
},
Box::new(|cx, span, _, _| cx.expr_bool(span, !base)),
Box::new(|cx, span, _| cx.expr_bool(span, !base)),
cx,
span,
substr,
Expand All @@ -69,11 +69,10 @@ pub fn expand_deriving_partial_eq(
MethodDef {
name: $name,
generics: Bounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![(borrowed_self(), sym::other)],
ret_ty: Literal(path_local!(bool)),
explicit_self: true,
args: vec![(self_ref(), sym::other)],
ret_ty: Path(path_local!(bool)),
attributes: attrs,
is_unsafe: false,
unify_fieldless_variants: true,
combine_substructure: combine_substructure(Box::new(|a, b, c| $f(a, b, c))),
}
Expand Down Expand Up @@ -102,7 +101,6 @@ pub fn expand_deriving_partial_eq(
path: path_std!(cmp::PartialEq),
additional_bounds: Vec::new(),
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: false,
methods,
associated_types: Vec::new(),
Expand Down
20 changes: 7 additions & 13 deletions compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,20 @@ pub fn expand_deriving_partial_ord(
item: &Annotatable,
push: &mut dyn FnMut(Annotatable),
) {
let ordering_ty = Literal(path_std!(cmp::Ordering));
let ret_ty = Literal(Path::new_(
pathvec_std!(option::Option),
None,
vec![Box::new(ordering_ty)],
PathKind::Std,
));
let ordering_ty = Path(path_std!(cmp::Ordering));
let ret_ty =
Path(Path::new_(pathvec_std!(option::Option), vec![Box::new(ordering_ty)], PathKind::Std));

let inline = cx.meta_word(span, sym::inline);
let attrs = vec![cx.attribute(inline)];

let partial_cmp_def = MethodDef {
name: sym::partial_cmp,
generics: Bounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![(borrowed_self(), sym::other)],
explicit_self: true,
args: vec![(self_ref(), sym::other)],
ret_ty,
attributes: attrs,
is_unsafe: false,
unify_fieldless_variants: true,
combine_substructure: combine_substructure(Box::new(|cx, span, substr| {
cs_partial_cmp(cx, span, substr)
Expand All @@ -46,7 +41,6 @@ pub fn expand_deriving_partial_ord(
path: path_std!(cmp::PartialOrd),
additional_bounds: vec![],
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: false,
methods: vec![partial_cmp_def],
associated_types: Vec::new(),
Expand Down Expand Up @@ -102,8 +96,8 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
cx.expr_match(span, new, vec![eq_arm, neq_arm])
},
equals_expr,
Box::new(|cx, span, (self_args, tag_tuple), _non_self_args| {
if self_args.len() != 2 {
Box::new(|cx, span, tag_tuple| {
if tag_tuple.len() != 2 {
cx.span_bug(span, "not exactly 2 arguments in `derive(PartialOrd)`")
} else {
let lft = cx.expr_addr_of(span, cx.expr_ident(span, tag_tuple[0]));
Expand Down
11 changes: 3 additions & 8 deletions compiler/rustc_builtin_macros/src/deriving/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,22 @@ pub fn expand_deriving_debug(
push: &mut dyn FnMut(Annotatable),
) {
// &mut ::std::fmt::Formatter
let fmtr =
Ptr(Box::new(Literal(path_std!(fmt::Formatter))), Borrowed(None, ast::Mutability::Mut));
let fmtr = Ref(Box::new(Path(path_std!(fmt::Formatter))), ast::Mutability::Mut);

let trait_def = TraitDef {
span,
attributes: Vec::new(),
path: path_std!(fmt::Debug),
additional_bounds: Vec::new(),
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: false,
methods: vec![MethodDef {
name: sym::fmt,
generics: Bounds::empty(),
explicit_self: borrowed_explicit_self(),
explicit_self: true,
args: vec![(fmtr, sym::f)],
ret_ty: Literal(path_std!(fmt::Result)),
ret_ty: Path(path_std!(fmt::Result)),
attributes: Vec::new(),
is_unsafe: false,
unify_fieldless_variants: false,
combine_substructure: combine_substructure(Box::new(|a, b, c| {
show_substructure(a, b, c)
Expand Down Expand Up @@ -64,8 +61,6 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
let (is_struct, args_per_field) = match vdata {
ast::VariantData::Unit(..) => {
// Special fast path for unit variants.
//let fn_path_write_str = cx.std_path(&[sym::fmt, sym::Formatter, sym::write_str]);
//return cx.expr_call_global(span, fn_path_write_str, vec![fmt, name]);
assert!(fields.is_empty());
(false, 0)
}
Expand Down
23 changes: 6 additions & 17 deletions compiler/rustc_builtin_macros/src/deriving/decodable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,40 +23,29 @@ pub fn expand_deriving_rustc_decodable(
let trait_def = TraitDef {
span,
attributes: Vec::new(),
path: Path::new_(vec![krate, sym::Decodable], None, vec![], PathKind::Global),
path: Path::new_(vec![krate, sym::Decodable], vec![], PathKind::Global),
additional_bounds: Vec::new(),
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: false,
methods: vec![MethodDef {
name: sym::decode,
generics: Bounds {
bounds: vec![(
typaram,
vec![Path::new_(vec![krate, sym::Decoder], None, vec![], PathKind::Global)],
vec![Path::new_(vec![krate, sym::Decoder], vec![], PathKind::Global)],
)],
},
explicit_self: None,
args: vec![(
Ptr(Box::new(Literal(Path::new_local(typaram))), Borrowed(None, Mutability::Mut)),
sym::d,
)],
ret_ty: Literal(Path::new_(
explicit_self: false,
args: vec![(Ref(Box::new(Path(Path::new_local(typaram))), Mutability::Mut), sym::d)],
ret_ty: Path(Path::new_(
pathvec_std!(result::Result),
None,
vec![
Box::new(Self_),
Box::new(Literal(Path::new_(
vec![typaram, sym::Error],
None,
vec![],
PathKind::Local,
))),
Box::new(Path(Path::new_(vec![typaram, sym::Error], vec![], PathKind::Local))),
],
PathKind::Std,
)),
attributes: Vec::new(),
is_unsafe: false,
unify_fieldless_variants: false,
combine_substructure: combine_substructure(Box::new(|a, b, c| {
decodable_substructure(a, b, c, krate)
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_builtin_macros/src/deriving/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@ pub fn expand_deriving_default(
path: Path::new(vec![kw::Default, sym::Default]),
additional_bounds: Vec::new(),
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: false,
methods: vec![MethodDef {
name: kw::Default,
generics: Bounds::empty(),
explicit_self: None,
explicit_self: false,
args: Vec::new(),
ret_ty: Self_,
attributes: attrs,
is_unsafe: false,
unify_fieldless_variants: false,
combine_substructure: combine_substructure(Box::new(|cx, trait_span, substr| {
match substr.fields {
Expand Down
Loading

0 comments on commit c0cf363

Please sign in to comment.