Skip to content

Commit

Permalink
Rollup merge of #104662 - nnethercote:tweak-deriving-for-packed-non-c…
Browse files Browse the repository at this point in the history 
…opy, r=jackh726

Streamline deriving on packed structs.

The current approach to field accesses in derived code:
- Normal case: `&self.0`
- In a packed struct that derives `Copy`: `&{self.0}`
- In a packed struct that doesn't derive `Copy`: `let Self(ref x) = *self`

The `let` pattern used in the third case is equivalent to the simpler field access in the first case. This commit changes the third case to use a field access.

The commit also combines two boolean arguments (`is_packed` and `always_copy`) into a single field (`copy_fields`) earlier, to save passing both around.

r? ``@jackh726``
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Nov 21, 2022
2 parents 5b92892 + a6e09a1 commit 439a8e6
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 87 deletions.
94 changes: 24 additions & 70 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,8 @@ impl<'a> TraitDef<'a> {
_ => unreachable!(),
};
let container_id = cx.current_expansion.id.expn_data().parent.expect_local();
let always_copy = has_no_type_params && cx.resolver.has_derive_copy(container_id);
let copy_fields =
is_packed && has_no_type_params && cx.resolver.has_derive_copy(container_id);

let newitem = match item.kind {
ast::ItemKind::Struct(ref struct_def, ref generics) => self.expand_struct_def(
Expand All @@ -457,16 +458,14 @@ impl<'a> TraitDef<'a> {
item.ident,
generics,
from_scratch,
is_packed,
always_copy,
copy_fields,
),
ast::ItemKind::Enum(ref enum_def, ref generics) => {
// We ignore `is_packed`/`always_copy` here, because
// `repr(packed)` enums cause an error later on.
// We ignore `is_packed` here, because `repr(packed)`
// enums cause an error later on.
//
// This can only cause further compilation errors
// downstream in blatantly illegal code, so it
// is fine.
// downstream in blatantly illegal code, so it is fine.
self.expand_enum_def(cx, enum_def, item.ident, generics, from_scratch)
}
ast::ItemKind::Union(ref struct_def, ref generics) => {
Expand All @@ -477,8 +476,7 @@ impl<'a> TraitDef<'a> {
item.ident,
generics,
from_scratch,
is_packed,
always_copy,
copy_fields,
)
} else {
cx.span_err(mitem.span, "this trait cannot be derived for unions");
Expand Down Expand Up @@ -748,8 +746,7 @@ impl<'a> TraitDef<'a> {
type_ident: Ident,
generics: &Generics,
from_scratch: bool,
is_packed: bool,
always_copy: bool,
copy_fields: bool,
) -> P<ast::Item> {
let field_tys: Vec<P<ast::Ty>> =
struct_def.fields().iter().map(|field| field.ty.clone()).collect();
Expand Down Expand Up @@ -777,8 +774,7 @@ impl<'a> TraitDef<'a> {
type_ident,
&selflike_args,
&nonselflike_args,
is_packed,
always_copy,
copy_fields,
)
};

Expand Down Expand Up @@ -1016,19 +1012,9 @@ impl<'a> MethodDef<'a> {
/// }
/// }
/// ```
/// If the struct doesn't impl `Copy`, we use let-destructuring with `ref`:
/// ```
/// # struct A { x: u8, y: u8 }
/// impl PartialEq for A {
/// fn eq(&self, other: &A) -> bool {
/// let Self { x: ref __self_0_0, y: ref __self_0_1 } = *self;
/// let Self { x: ref __self_1_0, y: ref __self_1_1 } = *other;
/// *__self_0_0 == *__self_1_0 && *__self_0_1 == *__self_1_1
/// }
/// }
/// ```
/// This latter case only works if the fields match the alignment required
/// by the `packed(N)` attribute. (We'll get errors later on if not.)
/// If the struct doesn't impl `Copy`, we use the normal `&self.x`. This
/// only works if the fields match the alignment required by the
/// `packed(N)` attribute. (We'll get errors later on if not.)
fn expand_struct_method_body<'b>(
&self,
cx: &mut ExtCtxt<'_>,
Expand All @@ -1037,51 +1023,19 @@ impl<'a> MethodDef<'a> {
type_ident: Ident,
selflike_args: &[P<Expr>],
nonselflike_args: &[P<Expr>],
is_packed: bool,
always_copy: bool,
copy_fields: bool,
) -> BlockOrExpr {
let span = trait_.span;
assert!(selflike_args.len() == 1 || selflike_args.len() == 2);

let mk_body = |cx, selflike_fields| {
self.call_substructure_method(
cx,
trait_,
type_ident,
nonselflike_args,
&Struct(struct_def, selflike_fields),
)
};

if !is_packed {
let selflike_fields =
trait_.create_struct_field_access_fields(cx, selflike_args, struct_def, false);
mk_body(cx, selflike_fields)
} else if always_copy {
let selflike_fields =
trait_.create_struct_field_access_fields(cx, selflike_args, struct_def, true);
mk_body(cx, selflike_fields)
} else {
// Packed and not copy. Need to use ref patterns.
let prefixes: Vec<_> =
(0..selflike_args.len()).map(|i| format!("__self_{}", i)).collect();
let selflike_fields = trait_.create_struct_pattern_fields(cx, struct_def, &prefixes);
let mut body = mk_body(cx, selflike_fields);

let struct_path = cx.path(span, vec![Ident::new(kw::SelfUpper, type_ident.span)]);
let patterns =
trait_.create_struct_patterns(cx, struct_path, struct_def, &prefixes, ByRef::Yes);

// Do the let-destructuring.
let mut stmts: Vec<_> = iter::zip(selflike_args, patterns)
.map(|(selflike_arg_expr, pat)| {
let selflike_arg_expr = cx.expr_deref(span, selflike_arg_expr.clone());
cx.stmt_let_pat(span, pat, selflike_arg_expr)
})
.collect();
stmts.extend(std::mem::take(&mut body.0));
BlockOrExpr(stmts, body.1)
}
let selflike_fields =
trait_.create_struct_field_access_fields(cx, selflike_args, struct_def, copy_fields);
self.call_substructure_method(
cx,
trait_,
type_ident,
nonselflike_args,
&Struct(struct_def, selflike_fields),
)
}

fn expand_static_struct_method_body(
Expand Down Expand Up @@ -1531,7 +1485,7 @@ impl<'a> TraitDef<'a> {
cx: &mut ExtCtxt<'_>,
selflike_args: &[P<Expr>],
struct_def: &'a VariantData,
copy: bool,
copy_fields: bool,
) -> Vec<FieldInfo> {
self.create_fields(struct_def, |i, struct_field, sp| {
selflike_args
Expand All @@ -1550,7 +1504,7 @@ impl<'a> TraitDef<'a> {
}),
),
);
if copy {
if copy_fields {
field_expr = cx.expr_block(
cx.block(struct_field.span, vec![cx.stmt_expr(field_expr)]),
);
Expand Down
23 changes: 6 additions & 17 deletions src/test/ui/deriving/deriving-all-codegen.stdout
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,16 +463,14 @@ struct PackedNonCopy(u8);
impl ::core::clone::Clone for PackedNonCopy {
#[inline]
fn clone(&self) -> PackedNonCopy {
let Self(ref __self_0_0) = *self;
PackedNonCopy(::core::clone::Clone::clone(__self_0_0))
PackedNonCopy(::core::clone::Clone::clone(&self.0))
}
}
#[automatically_derived]
impl ::core::fmt::Debug for PackedNonCopy {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
let Self(ref __self_0_0) = *self;
::core::fmt::Formatter::debug_tuple_field1_finish(f, "PackedNonCopy",
&__self_0_0)
&&self.0)
}
}
#[automatically_derived]
Expand All @@ -485,20 +483,15 @@ impl ::core::default::Default for PackedNonCopy {
#[automatically_derived]
impl ::core::hash::Hash for PackedNonCopy {
fn hash<__H: ::core::hash::Hasher>(&self, state: &mut __H) -> () {
let Self(ref __self_0_0) = *self;
::core::hash::Hash::hash(__self_0_0, state)
::core::hash::Hash::hash(&self.0, state)
}
}
#[automatically_derived]
impl ::core::marker::StructuralPartialEq for PackedNonCopy { }
#[automatically_derived]
impl ::core::cmp::PartialEq for PackedNonCopy {
#[inline]
fn eq(&self, other: &PackedNonCopy) -> bool {
let Self(ref __self_0_0) = *self;
let Self(ref __self_1_0) = *other;
*__self_0_0 == *__self_1_0
}
fn eq(&self, other: &PackedNonCopy) -> bool { self.0 == other.0 }
}
#[automatically_derived]
impl ::core::marker::StructuralEq for PackedNonCopy { }
Expand All @@ -516,18 +509,14 @@ impl ::core::cmp::PartialOrd for PackedNonCopy {
#[inline]
fn partial_cmp(&self, other: &PackedNonCopy)
-> ::core::option::Option<::core::cmp::Ordering> {
let Self(ref __self_0_0) = *self;
let Self(ref __self_1_0) = *other;
::core::cmp::PartialOrd::partial_cmp(__self_0_0, __self_1_0)
::core::cmp::PartialOrd::partial_cmp(&self.0, &other.0)
}
}
#[automatically_derived]
impl ::core::cmp::Ord for PackedNonCopy {
#[inline]
fn cmp(&self, other: &PackedNonCopy) -> ::core::cmp::Ordering {
let Self(ref __self_0_0) = *self;
let Self(ref __self_1_0) = *other;
::core::cmp::Ord::cmp(__self_0_0, __self_1_0)
::core::cmp::Ord::cmp(&self.0, &other.0)
}
}

Expand Down

0 comments on commit 439a8e6

Please sign in to comment.