Improve shallow Clone deriving

Author: petrochenkov

src/libcore/clone.rs

@@ -106,10 +106,23 @@ pub trait Clone : Sized {
     }
 }
 
-// FIXME(aburka): this method is used solely by #[derive] to
-// assert that every component of a type implements Clone.
+// FIXME(aburka): these structs are used solely by #[derive] to
+// assert that every component of a type implements Clone or Copy.
 //
-// This should never be called by user code.
+// These structs should never appear in user code.
+#[doc(hidden)]
+#[allow(missing_debug_implementations)]
+#[unstable(feature = "derive_clone_copy",
+           reason = "deriving hack, should not be public",
+           issue = "0")]
+pub struct AssertParamIsClone<T: Clone + ?Sized> { _field: ::marker::PhantomData<T> }
+#[doc(hidden)]
+#[allow(missing_debug_implementations)]
+#[unstable(feature = "derive_clone_copy",
+           reason = "deriving hack, should not be public",
+           issue = "0")]
+pub struct AssertParamIsCopy<T: Copy + ?Sized> { _field: ::marker::PhantomData<T> }
+#[cfg(stage0)]
 #[doc(hidden)]
 #[inline(always)]
 #[unstable(feature = "derive_clone_copy",

src/libsyntax_ext/deriving/clone.rs

@@ -11,20 +11,14 @@
 use deriving::generic::*;
 use deriving::generic::ty::*;
 
-use syntax::ast::{Expr, Generics, ItemKind, MetaItem, VariantData};
+use syntax::ast::{self, Expr, Generics, ItemKind, MetaItem, VariantData};
 use syntax::attr;
 use syntax::ext::base::{Annotatable, ExtCtxt};
 use syntax::ext::build::AstBuilder;
-use syntax::parse::token::InternedString;
+use syntax::parse::token::{keywords, InternedString};
 use syntax::ptr::P;
 use syntax_pos::Span;
 
-#[derive(PartialEq)]
-enum Mode {
-    Deep,
-    Shallow,
-}
-
 pub fn expand_deriving_clone(cx: &mut ExtCtxt,
                              span: Span,
                              mitem: &MetaItem,
@@ -40,29 +34,38 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt,
     //      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.
+    //      Whever Clone is implemented for fields is irrelevant so we don't assert it.
     let bounds;
-    let unify_fieldless_variants;
     let substructure;
+    let is_shallow;
     match *item {
         Annotatable::Item(ref annitem) => {
             match annitem.node {
                 ItemKind::Struct(_, Generics { ref ty_params, .. }) |
                 ItemKind::Enum(_, Generics { ref ty_params, .. })
-                    if ty_params.is_empty() &&
-                       attr::contains_name(&annitem.attrs, "rustc_copy_clone_marker") => {
-
+                        if attr::contains_name(&annitem.attrs, "rustc_copy_clone_marker") &&
+                           ty_params.is_empty() => {
+                    bounds = vec![];
+                    is_shallow = true;
+                    substructure = combine_substructure(Box::new(|c, s, sub| {
+                        cs_clone_shallow("Clone", c, s, sub, false)
+                    }));
+                }
+                ItemKind::Union(..) => {
                     bounds = vec![Literal(path_std!(cx, core::marker::Copy))];
-                    unify_fieldless_variants = true;
+                    is_shallow = true;
                     substructure = combine_substructure(Box::new(|c, s, sub| {
-                        cs_clone("Clone", c, s, sub, Mode::Shallow)
+                        cs_clone_shallow("Clone", c, s, sub, true)
                     }));
                 }
-
                 _ => {
                     bounds = vec![];
-                    unify_fieldless_variants = false;
+                    is_shallow = false;
                     substructure = combine_substructure(Box::new(|c, s, sub| {
-                        cs_clone("Clone", c, s, sub, Mode::Deep)
+                        cs_clone("Clone", c, s, sub)
                     }));
                 }
             }
@@ -80,7 +83,7 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt,
         additional_bounds: bounds,
         generics: LifetimeBounds::empty(),
         is_unsafe: false,
-        supports_unions: false,
+        supports_unions: true,
         methods: vec![MethodDef {
                           name: "clone",
                           generics: LifetimeBounds::empty(),
@@ -89,37 +92,85 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt,
                           ret_ty: Self_,
                           attributes: attrs,
                           is_unsafe: false,
-                          unify_fieldless_variants: unify_fieldless_variants,
+                          unify_fieldless_variants: false,
                           combine_substructure: substructure,
                       }],
         associated_types: Vec::new(),
     };
 
-    trait_def.expand(cx, mitem, item, push)
+    trait_def.expand_ext(cx, mitem, item, push, is_shallow)
+}
+
+fn cs_clone_shallow(name: &str,
+                    cx: &mut ExtCtxt,
+                    trait_span: Span,
+                    substr: &Substructure,
+                    is_union: bool)
+                    -> P<Expr> {
+    fn assert_ty_bounds(cx: &mut ExtCtxt, stmts: &mut Vec<ast::Stmt>,
+                        ty: P<ast::Ty>, span: Span, helper_name: &str) {
+        // Generate statement `let _: helper_name<ty>;`,
+        // set the expn ID so we can use the unstable struct.
+        let span = super::allow_unstable(cx, span, "derive(Clone)");
+        let assert_path = cx.path_all(span, true,
+                                        cx.std_path(&["clone", helper_name]),
+                                        vec![], vec![ty], vec![]);
+        let local = P(ast::Local {
+            pat: cx.pat_wild(span),
+            ty: Some(cx.ty_path(assert_path)),
+            init: None,
+            id: ast::DUMMY_NODE_ID,
+            span: span,
+            attrs: ast::ThinVec::new(),
+        });
+        let stmt = ast::Stmt {
+            id: ast::DUMMY_NODE_ID,
+            node: ast::StmtKind::Local(local),
+            span: span,
+        };
+        stmts.push(stmt);
+    }
+    fn process_variant(cx: &mut ExtCtxt, stmts: &mut Vec<ast::Stmt>, variant: &VariantData) {
+        for field in variant.fields() {
+            // let _: AssertParamIsClone<FieldTy>;
+            assert_ty_bounds(cx, stmts, field.ty.clone(), field.span, "AssertParamIsClone");
+        }
+    }
+
+    let mut stmts = Vec::new();
+    if is_union {
+        // let _: AssertParamIsCopy<Self>;
+        let self_ty = cx.ty_path(cx.path_ident(trait_span, keywords::SelfType.ident()));
+        assert_ty_bounds(cx, &mut stmts, self_ty, trait_span, "AssertParamIsCopy");
+    } else {
+        match *substr.fields {
+            StaticStruct(vdata, ..) => {
+                process_variant(cx, &mut stmts, vdata);
+            }
+            StaticEnum(enum_def, ..) => {
+                for variant in &enum_def.variants {
+                    process_variant(cx, &mut stmts, &variant.node.data);
+                }
+            }
+            _ => cx.span_bug(trait_span, &format!("unexpected substructure in \
+                                                    shallow `derive({})`", name))
+        }
+    }
+    stmts.push(cx.stmt_expr(cx.expr_deref(trait_span, cx.expr_self(trait_span))));
+    cx.expr_block(cx.block(trait_span, stmts))
 }
 
 fn cs_clone(name: &str,
             cx: &mut ExtCtxt,
             trait_span: Span,
-            substr: &Substructure,
-            mode: Mode)
+            substr: &Substructure)
             -> P<Expr> {
     let ctor_path;
     let all_fields;
-    let fn_path = match mode {
-        Mode::Shallow => cx.std_path(&["clone", "assert_receiver_is_clone"]),
-        Mode::Deep => cx.std_path(&["clone", "Clone", "clone"]),
-    };
+    let fn_path = cx.std_path(&["clone", "Clone", "clone"]);
     let subcall = |cx: &mut ExtCtxt, field: &FieldInfo| {
         let args = vec![cx.expr_addr_of(field.span, field.self_.clone())];
-
-        let span = if mode == Mode::Shallow {
-            // set the expn ID so we can call the unstable method
-            super::allow_unstable(cx, field.span, "derive(Clone)")
-        } else {
-            field.span
-        };
-        cx.expr_call_global(span, fn_path.clone(), args)
+        cx.expr_call_global(field.span, fn_path.clone(), args)
     };
 
     let vdata;
@@ -145,43 +196,31 @@ fn cs_clone(name: &str,
         }
     }
 
-    match mode {
-        Mode::Shallow => {
-            let mut stmts = all_fields.iter().map(|f| {
-                let call = subcall(cx, f);
-                cx.stmt_expr(call)
-            }).collect::<Vec<_>>();
-            stmts.push(cx.stmt_expr(cx.expr_deref(trait_span, cx.expr_self(trait_span))));
-            cx.expr_block(cx.block(trait_span, stmts))
-        }
-        Mode::Deep => {
-            match *vdata {
-                VariantData::Struct(..) => {
-                    let fields = all_fields.iter()
-                        .map(|field| {
-                            let ident = match field.name {
-                                Some(i) => i,
-                                None => {
-                                    cx.span_bug(trait_span,
-                                                &format!("unnamed field in normal struct in \
-                                                     `derive({})`",
-                                                         name))
-                                }
-                            };
-                            let call = subcall(cx, field);
-                            cx.field_imm(field.span, ident, call)
-                        })
-                        .collect::<Vec<_>>();
+    match *vdata {
+        VariantData::Struct(..) => {
+            let fields = all_fields.iter()
+                .map(|field| {
+                    let ident = match field.name {
+                        Some(i) => i,
+                        None => {
+                            cx.span_bug(trait_span,
+                                        &format!("unnamed field in normal struct in \
+                                                `derive({})`",
+                                                    name))
+                        }
+                    };
+                    let call = subcall(cx, field);
+                    cx.field_imm(field.span, ident, call)
+                })
+                .collect::<Vec<_>>();
 
-                    cx.expr_struct(trait_span, ctor_path, fields)
-                }
-                VariantData::Tuple(..) => {
-                    let subcalls = all_fields.iter().map(|f| subcall(cx, f)).collect();
-                    let path = cx.expr_path(ctor_path);
-                    cx.expr_call(trait_span, path, subcalls)
-                }
-                VariantData::Unit(..) => cx.expr_path(ctor_path),
-            }
+            cx.expr_struct(trait_span, ctor_path, fields)
+        }
+        VariantData::Tuple(..) => {
+            let subcalls = all_fields.iter().map(|f| subcall(cx, f)).collect();
+            let path = cx.expr_path(ctor_path);
+            cx.expr_call(trait_span, path, subcalls)
         }
+        VariantData::Unit(..) => cx.expr_path(ctor_path),
     }
 }

src/libsyntax_ext/deriving/generic/mod.rs

@@ -401,18 +401,29 @@ impl<'a> TraitDef<'a> {
                   mitem: &ast::MetaItem,
                   item: &'a Annotatable,
                   push: &mut FnMut(Annotatable)) {
+        self.expand_ext(cx, mitem, item, push, false);
+    }
+
+    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 newitem = match item.node {
                     ast::ItemKind::Struct(ref struct_def, ref generics) => {
-                        self.expand_struct_def(cx, &struct_def, item.ident, generics)
+                        self.expand_struct_def(cx, &struct_def, item.ident, generics, from_scratch)
                     }
                     ast::ItemKind::Enum(ref enum_def, ref generics) => {
-                        self.expand_enum_def(cx, enum_def, &item.attrs, item.ident, generics)
+                        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)
+                            self.expand_struct_def(cx, &struct_def, item.ident,
+                                                   generics, from_scratch)
                         } else {
                             cx.span_err(mitem.span,
                                         "this trait cannot be derived for unions");
@@ -661,7 +672,8 @@ impl<'a> TraitDef<'a> {
                          cx: &mut ExtCtxt,
                          struct_def: &'a VariantData,
                          type_ident: Ident,
-                         generics: &Generics)
+                         generics: &Generics,
+                         from_scratch: bool)
                          -> P<ast::Item> {
         let field_tys: Vec<P<ast::Ty>> = struct_def.fields()
             .iter()
@@ -674,7 +686,7 @@ impl<'a> TraitDef<'a> {
                 let (explicit_self, self_args, nonself_args, tys) =
                     method_def.split_self_nonself_args(cx, self, type_ident, generics);
 
-                let body = if method_def.is_static() {
+                let body = if from_scratch || method_def.is_static() {
                     method_def.expand_static_struct_method_body(cx,
                                                                 self,
                                                                 struct_def,
@@ -709,7 +721,8 @@ impl<'a> TraitDef<'a> {
                        enum_def: &'a EnumDef,
                        type_attrs: &[ast::Attribute],
                        type_ident: Ident,
-                       generics: &Generics)
+                       generics: &Generics,
+                       from_scratch: bool)
                        -> P<ast::Item> {
         let mut field_tys = Vec::new();
 
@@ -727,7 +740,7 @@ impl<'a> TraitDef<'a> {
                 let (explicit_self, self_args, nonself_args, tys) =
                     method_def.split_self_nonself_args(cx, self, type_ident, generics);
 
-                let body = if method_def.is_static() {
+                let body = if from_scratch || method_def.is_static() {
                     method_def.expand_static_enum_method_body(cx,
                                                               self,
                                                               enum_def,

src/test/compile-fail/borrowck/borrowck-union-borrow-nested.rs

@@ -18,16 +18,12 @@ struct S {
     b: u16,
 }
 
+#[derive(Clone, Copy)]
 union U {
     s: S,
     c: u32,
 }
 
-impl Clone for U {
-    fn clone(&self) -> Self { *self }
-}
-impl Copy for U {}
-
 fn main() {
     unsafe {
         {

src/test/compile-fail/borrowck/borrowck-union-borrow.rs

@@ -12,16 +12,12 @@
 
 #![feature(untagged_unions)]
 
+#[derive(Clone, Copy)]
 union U {
     a: u8,
     b: u64,
 }
 
-impl Clone for U {
-    fn clone(&self) -> Self { *self }
-}
-impl Copy for U {}
-
 fn main() {
     unsafe {
         let mut u = U { b: 0 };