From dee8a71cd5221536c319ca8c14108e93521092f5 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 2 Oct 2017 15:15:23 +0200 Subject: fix #[derive] implementation for repr(packed) structs Fix the derive implementation for repr(packed) structs to move the fields out instead of calling functions on references to each subfield. That's it, `#[derive(PartialEq)]` on a packed struct now does: ```Rust fn eq(&self, other: &Self) { let field_0 = self.0; let other_field_0 = other.0; &field_0 == &other_field_0 } ``` Instead of ```Rust fn eq(&self, other: &Self) { let ref field_0 = self.0; let ref other_field_0 = other.0; &*field_0 == &*other_field_0 } ``` Taking (unaligned) references to each subfield is undefined, unsound and is an error with MIR effectck, so it had to be prevented. This causes a borrowck error when a `repr(packed)` struct has a non-Copy field (and therefore is a [breaking-change]), but I don't see a sound way to avoid that error. --- src/libsyntax_ext/deriving/generic/mod.rs | 80 ++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 16 deletions(-) (limited to 'src/libsyntax_ext') diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 18897047538..5abf524313a 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -393,7 +393,7 @@ fn find_type_parameters(ty: &ast::Ty, } impl<'a> TraitDef<'a> { - pub fn expand(&self, + pub fn expand(self, cx: &mut ExtCtxt, mitem: &ast::MetaItem, item: &'a Annotatable, @@ -401,7 +401,7 @@ impl<'a> TraitDef<'a> { self.expand_ext(cx, mitem, item, push, false); } - pub fn expand_ext(&self, + pub fn expand_ext(self, cx: &mut ExtCtxt, mitem: &ast::MetaItem, item: &'a Annotatable, @@ -409,18 +409,31 @@ impl<'a> TraitDef<'a> { from_scratch: bool) { match *item { Annotatable::Item(ref item) => { + let is_packed = item.attrs.iter().any(|attr| { + attr::find_repr_attrs(&cx.parse_sess.span_diagnostic, attr) + .contains(&attr::ReprPacked) + }); + let use_temporaries = is_packed; let newitem = match item.node { ast::ItemKind::Struct(ref struct_def, ref generics) => { - self.expand_struct_def(cx, &struct_def, item.ident, generics, from_scratch) + self.expand_struct_def(cx, &struct_def, item.ident, generics, from_scratch, + use_temporaries) } ast::ItemKind::Enum(ref enum_def, ref generics) => { + // We ignore `use_temporaries` 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. self.expand_enum_def(cx, enum_def, &item.attrs, item.ident, generics, from_scratch) } ast::ItemKind::Union(ref struct_def, ref generics) => { if self.supports_unions { self.expand_struct_def(cx, &struct_def, item.ident, - generics, from_scratch) + generics, from_scratch, + use_temporaries) } else { cx.span_err(mitem.span, "this trait cannot be derived for unions"); @@ -675,7 +688,8 @@ impl<'a> TraitDef<'a> { struct_def: &'a VariantData, type_ident: Ident, generics: &Generics, - from_scratch: bool) + from_scratch: bool, + use_temporaries: bool) -> P { let field_tys: Vec> = struct_def.fields() .iter() @@ -701,7 +715,8 @@ impl<'a> TraitDef<'a> { struct_def, type_ident, &self_args[..], - &nonself_args[..]) + &nonself_args[..], + use_temporaries) }; method_def.create_method(cx, @@ -958,6 +973,22 @@ impl<'a> MethodDef<'a> { /// } /// } /// } + /// + /// // or if A is repr(packed) - note fields are matched by-value + /// // instead of by-reference. + /// impl PartialEq for A { + /// fn eq(&self, __arg_1: &A) -> bool { + /// match *self { + /// A {x: __self_0_0, y: __self_0_1} => { + /// match __arg_1 { + /// A {x: __self_1_0, y: __self_1_1} => { + /// __self_0_0.eq(&__self_1_0) && __self_0_1.eq(&__self_1_1) + /// } + /// } + /// } + /// } + /// } + /// } /// ``` fn expand_struct_method_body<'b>(&self, cx: &mut ExtCtxt, @@ -965,7 +996,8 @@ impl<'a> MethodDef<'a> { struct_def: &'b VariantData, type_ident: Ident, self_args: &[P], - nonself_args: &[P]) + nonself_args: &[P], + use_temporaries: bool) -> P { let mut raw_fields = Vec::new(); // Vec<[fields of self], @@ -977,7 +1009,8 @@ impl<'a> MethodDef<'a> { struct_path, struct_def, &format!("__self_{}", i), - ast::Mutability::Immutable); + ast::Mutability::Immutable, + use_temporaries); patterns.push(pat); raw_fields.push(ident_expr); } @@ -1140,7 +1173,6 @@ impl<'a> MethodDef<'a> { self_args: Vec>, nonself_args: &[P]) -> P { - let sp = trait_.span; let variants = &enum_def.variants; @@ -1512,12 +1544,18 @@ impl<'a> TraitDef<'a> { fn create_subpatterns(&self, cx: &mut ExtCtxt, field_paths: Vec, - mutbl: ast::Mutability) + mutbl: ast::Mutability, + use_temporaries: bool) -> Vec> { field_paths.iter() .map(|path| { + let binding_mode = if use_temporaries { + ast::BindingMode::ByValue(ast::Mutability::Immutable) + } else { + ast::BindingMode::ByRef(mutbl) + }; cx.pat(path.span, - PatKind::Ident(ast::BindingMode::ByRef(mutbl), (*path).clone(), None)) + PatKind::Ident(binding_mode, (*path).clone(), None)) }) .collect() } @@ -1528,8 +1566,10 @@ impl<'a> TraitDef<'a> { struct_path: ast::Path, struct_def: &'a VariantData, prefix: &str, - mutbl: ast::Mutability) - -> (P, Vec<(Span, Option, P, &'a [ast::Attribute])>) { + mutbl: ast::Mutability, + use_temporaries: bool) + -> (P, Vec<(Span, Option, P, &'a [ast::Attribute])>) + { let mut paths = Vec::new(); let mut ident_exprs = Vec::new(); for (i, struct_field) in struct_def.fields().iter().enumerate() { @@ -1539,12 +1579,18 @@ impl<'a> TraitDef<'a> { span: sp, node: ident, }); - let val = cx.expr_deref(sp, cx.expr_path(cx.path_ident(sp, ident))); + let val = cx.expr_path(cx.path_ident(sp, ident)); + let val = if use_temporaries { + val + } else { + cx.expr_deref(sp, val) + }; let val = cx.expr(sp, ast::ExprKind::Paren(val)); + ident_exprs.push((sp, struct_field.ident, val, &struct_field.attrs[..])); } - let subpats = self.create_subpatterns(cx, paths, mutbl); + let subpats = self.create_subpatterns(cx, paths, mutbl, use_temporaries); let pattern = match *struct_def { VariantData::Struct(..) => { let field_pats = subpats.into_iter() @@ -1588,7 +1634,9 @@ impl<'a> TraitDef<'a> { let variant_ident = variant.node.name; let sp = variant.span.with_ctxt(self.span.ctxt()); let variant_path = cx.path(sp, vec![enum_ident, variant_ident]); - self.create_struct_pattern(cx, variant_path, &variant.node.data, prefix, mutbl) + let use_temporaries = false; // enums can't be repr(packed) + self.create_struct_pattern(cx, variant_path, &variant.node.data, prefix, mutbl, + use_temporaries) } } -- cgit 1.4.1-3-g733a5