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.

Author: arielb1

src/libsyntax_ext/deriving/generic/mod.rs

@@ -393,34 +393,47 @@ 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,
                   push: &mut FnMut(Annotatable)) {
         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,
                       push: &mut FnMut(Annotatable),
                       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");
@@ -674,7 +687,8 @@ impl<'a> TraitDef<'a> {
                          struct_def: &'a VariantData,
                          type_ident: Ident,
                          generics: &Generics,
-                         from_scratch: bool)
+                         from_scratch: bool,
+                         use_temporaries: bool)
                          -> P<ast::Item> {
         let field_tys: Vec<P<ast::Ty>> = struct_def.fields()
             .iter()
@@ -700,7 +714,8 @@ impl<'a> TraitDef<'a> {
                                                          struct_def,
                                                          type_ident,
                                                          &self_args[..],
-                                                         &nonself_args[..])
+                                                         &nonself_args[..],
+                                                         use_temporaries)
                 };
 
                 method_def.create_method(cx,
@@ -957,14 +972,31 @@ 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,
                                      trait_: &TraitDef<'b>,
                                      struct_def: &'b VariantData,
                                      type_ident: Ident,
                                      self_args: &[P<Expr>],
-                                     nonself_args: &[P<Expr>])
+                                     nonself_args: &[P<Expr>],
+                                     use_temporaries: bool)
                                      -> P<Expr> {
 
         let mut raw_fields = Vec::new(); // Vec<[fields of self],
@@ -976,7 +1008,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);
         }
@@ -1139,7 +1172,6 @@ impl<'a> MethodDef<'a> {
                                   self_args: Vec<P<Expr>>,
                                   nonself_args: &[P<Expr>])
                                   -> P<Expr> {
-
         let sp = trait_.span;
         let variants = &enum_def.variants;
 
@@ -1511,12 +1543,18 @@ impl<'a> TraitDef<'a> {
     fn create_subpatterns(&self,
                           cx: &mut ExtCtxt,
                           field_paths: Vec<ast::SpannedIdent>,
-                          mutbl: ast::Mutability)
+                          mutbl: ast::Mutability,
+                          use_temporaries: bool)
                           -> Vec<P<ast::Pat>> {
         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()
     }
@@ -1527,8 +1565,10 @@ impl<'a> TraitDef<'a> {
          struct_path: ast::Path,
          struct_def: &'a VariantData,
          prefix: &str,
-         mutbl: ast::Mutability)
-         -> (P<ast::Pat>, Vec<(Span, Option<Ident>, P<Expr>, &'a [ast::Attribute])>) {
+         mutbl: ast::Mutability,
+         use_temporaries: bool)
+         -> (P<ast::Pat>, Vec<(Span, Option<Ident>, P<Expr>, &'a [ast::Attribute])>)
+    {
         let mut paths = Vec::new();
         let mut ident_exprs = Vec::new();
         for (i, struct_field) in struct_def.fields().iter().enumerate() {
@@ -1538,12 +1578,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()
@@ -1587,7 +1633,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)
     }
 }
 

src/libsyntax_pos/span_encoding.rs

@@ -19,30 +19,16 @@ use hygiene::SyntaxContext;
 
 use rustc_data_structures::fx::FxHashMap;
 use std::cell::RefCell;
-use std::hash::{Hash, Hasher};
 
 /// A compressed span.
 /// Contains either fields of `SpanData` inline if they are small, or index into span interner.
 /// The primary goal of `Span` is to be as small as possible and fit into other structures
 /// (that's why it uses `packed` as well). Decoding speed is the second priority.
 /// See `SpanData` for the info on span fields in decoded representation.
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, PartialEq, Eq, Hash)]
 #[repr(packed)]
 pub struct Span(u32);
 
-impl PartialEq for Span {
-    #[inline]
-    fn eq(&self, other: &Self) -> bool { ({self.0}) == ({other.0}) }
-}
-
-impl Eq for Span {}
-
-impl Hash for Span {
-    fn hash<H: Hasher>(&self, s: &mut H) {
-        {self.0}.hash(s)
-    }
-}
-
 /// Dummy span, both position and length are zero, syntax context is zero as well.
 /// This span is kept inline and encoded with format 0.
 pub const DUMMY_SP: Span = Span(0);

src/test/compile-fail/deriving-with-repr-packed-not-copy.rs

@@ -0,0 +1,28 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// check that derive on a packed struct with non-Copy fields
+// correctly. This can't be made to work perfectly because
+// we can't just use the field from the struct as it might
+// not be aligned.
+
+#[derive(PartialEq)]
+struct Y(usize);
+
+#[derive(PartialEq)]
+//~^ ERROR cannot move out of borrowed
+//~| ERROR cannot move out of borrowed
+//~| ERROR cannot move out of borrowed
+//~| ERROR cannot move out of borrowed
+#[repr(packed)]
+struct X(Y);
+
+fn main() {
+}

src/test/run-pass/deriving-with-repr-packed.rs

@@ -0,0 +1,45 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// check that derive on a packed struct does not call field
+// methods with a misaligned field.
+
+use std::mem;
+
+#[derive(Copy, Clone)]
+struct Aligned(usize);
+
+#[inline(never)]
+fn check_align(ptr: *const Aligned) {
+    assert_eq!(ptr as usize % mem::align_of::<Aligned>(),
+               0);
+}
+
+impl PartialEq for Aligned {
+    fn eq(&self, other: &Self) -> bool {
+        check_align(self);
+        check_align(other);
+        self.0 == other.0
+    }
+}
+
+#[repr(packed)]
+#[derive(PartialEq)]
+struct Packed(Aligned, Aligned);
+
+#[derive(PartialEq)]
+#[repr(C)]
+struct Dealigned<T>(u8, T);
+
+fn main() {
+    let d1 = Dealigned(0, Packed(Aligned(1), Aligned(2)));
+    let ck = d1 == d1;
+    assert!(ck);
+}