Auto merge of #31414 - durka:clone-copy, r=alexcrichton

special-case #[derive(Copy, Clone)] with a shallow clone

If a type is Copy then its Clone implementation can be a no-op. Currently `#[derive(Clone)]` generates a deep clone anyway. This can lead to lots of code bloat.

This PR detects the case where Copy and Clone are both being derived (the general case of "is this type Copy" can't be determined by a syntax extension) and generates the shallow Clone impl. Right now this can only be done if there are no type parameters (see #31085 (comment)), but this restriction can be removed after specialization.

Fixes #31085.

Author: bors

src/libcore/clone.rs

@@ -75,6 +75,17 @@ pub trait Clone : Sized {
     }
 }
 
+// FIXME(aburka): this method is used solely by #[derive] to
+// assert that every component of a type implements Clone.
+//
+// This should never be called by user code.
+#[doc(hidden)]
+#[inline(always)]
+#[unstable(feature = "derive_clone_copy",
+           reason = "deriving hack, should not be public",
+           issue = "0")]
+pub fn assert_receiver_is_clone<T: Clone + ?Sized>(_: &T) {}
+
 #[stable(feature = "rust1", since = "1.0.0")]
 impl<'a, T: ?Sized> Clone for &'a T {
     /// Returns a shallow copy of the reference.

src/libsyntax_ext/deriving/clone.rs

@@ -11,26 +11,68 @@
 use deriving::generic::*;
 use deriving::generic::ty::*;
 
-use syntax::ast::{MetaItem, Expr, VariantData};
+use syntax::ast::{Expr, ItemKind, Generics, MetaItem, VariantData};
+use syntax::attr::{self, AttrMetaMethods};
 use syntax::codemap::Span;
 use syntax::ext::base::{ExtCtxt, Annotatable};
 use syntax::ext::build::AstBuilder;
 use syntax::parse::token::InternedString;
 use syntax::ptr::P;
 
+#[derive(PartialEq)]
+enum Mode { Deep, Shallow }
+
 pub fn expand_deriving_clone(cx: &mut ExtCtxt,
                              span: Span,
                              mitem: &MetaItem,
                              item: &Annotatable,
                              push: &mut FnMut(Annotatable))
 {
+    // check if we can use a short form
+    //
+    // 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.
+    let bounds;
+    let substructure;
+    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, "derive_Copy") => {
+
+                    bounds = vec![Literal(path_std!(cx, core::marker::Copy))];
+                    substructure = combine_substructure(Box::new(|c, s, sub| {
+                        cs_clone("Clone", c, s, sub, Mode::Shallow)
+                    }));
+                }
+
+                _ => {
+                    bounds = vec![];
+                    substructure = combine_substructure(Box::new(|c, s, sub| {
+                        cs_clone("Clone", c, s, sub, Mode::Deep)
+                    }));
+                }
+            }
+        }
+
+        _ => cx.span_bug(span, "#[derive(Clone)] on trait item or impl item")
+    }
+
     let inline = cx.meta_word(span, InternedString::new("inline"));
     let attrs = vec!(cx.attribute(span, inline));
     let trait_def = TraitDef {
         span: span,
         attributes: Vec::new(),
         path: path_std!(cx, core::clone::Clone),
-        additional_bounds: Vec::new(),
+        additional_bounds: bounds,
         generics: LifetimeBounds::empty(),
         is_unsafe: false,
         methods: vec!(
@@ -42,9 +84,7 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt,
                 ret_ty: Self_,
                 attributes: attrs,
                 is_unsafe: false,
-                combine_substructure: combine_substructure(Box::new(|c, s, sub| {
-                    cs_clone("Clone", c, s, sub)
-                })),
+                combine_substructure: substructure,
             }
         ),
         associated_types: Vec::new(),
@@ -56,14 +96,24 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt,
 fn cs_clone(
     name: &str,
     cx: &mut ExtCtxt, trait_span: Span,
-    substr: &Substructure) -> P<Expr> {
+    substr: &Substructure,
+    mode: Mode) -> P<Expr> {
     let ctor_path;
     let all_fields;
-    let fn_path = cx.std_path(&["clone", "Clone", "clone"]);
+    let fn_path = match mode {
+        Mode::Shallow => cx.std_path(&["clone", "assert_receiver_is_clone"]),
+        Mode::Deep  => cx.std_path(&["clone", "Clone", "clone"]),
+    };
     let subcall = |field: &FieldInfo| {
         let args = vec![cx.expr_addr_of(field.span, field.self_.clone())];
 
-        cx.expr_call_global(field.span, fn_path.clone(), args)
+        let span = if mode == Mode::Shallow {
+            // set the expn ID so we can call the unstable method
+            Span { expn_id: cx.backtrace(), .. trait_span }
+        } else {
+            field.span
+        };
+        cx.expr_call_global(span, fn_path.clone(), args)
     };
 
     let vdata;
@@ -89,29 +139,41 @@ fn cs_clone(
         }
     }
 
-    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))
-                    }
-                };
-                cx.field_imm(field.span, ident, subcall(field))
-            }).collect::<Vec<_>>();
-
-            cx.expr_struct(trait_span, ctor_path, fields)
+    match mode {
+        Mode::Shallow => {
+            cx.expr_block(cx.block(trait_span,
+                                   all_fields.iter()
+                                             .map(subcall)
+                                             .map(|e| cx.stmt_expr(e))
+                                             .collect(),
+                                   Some(cx.expr_deref(trait_span, cx.expr_self(trait_span)))))
         }
-        VariantData::Tuple(..) => {
-            let subcalls = all_fields.iter().map(subcall).collect();
-            let path = cx.expr_path(ctor_path);
-            cx.expr_call(trait_span, path, subcalls)
-        }
-        VariantData::Unit(..) => {
-            cx.expr_path(ctor_path)
+        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))
+                            }
+                        };
+                        cx.field_imm(field.span, ident, subcall(field))
+                    }).collect::<Vec<_>>();
+
+                    cx.expr_struct(trait_span, ctor_path, fields)
+                }
+                VariantData::Tuple(..) => {
+                    let subcalls = all_fields.iter().map(subcall).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/decodable.rs

@@ -74,7 +74,7 @@ fn expand_deriving_decodable_imp(cx: &mut ExtCtxt,
                 },
                 explicit_self: None,
                 args: vec!(Ptr(Box::new(Literal(Path::new_local(typaram))),
-                            Borrowed(None, Mutability::Mutable))),
+                           Borrowed(None, Mutability::Mutable))),
                 ret_ty: Literal(Path::new_(
                     pathvec_std!(cx, core::result::Result),
                     None,

src/libsyntax_ext/deriving/encodable.rs

@@ -150,7 +150,7 @@ fn expand_deriving_encodable_imp(cx: &mut ExtCtxt,
                 },
                 explicit_self: borrowed_explicit_self(),
                 args: vec!(Ptr(Box::new(Literal(Path::new_local(typaram))),
-                            Borrowed(None, Mutability::Mutable))),
+                           Borrowed(None, Mutability::Mutable))),
                 ret_ty: Literal(Path::new_(
                     pathvec_std!(cx, core::result::Result),
                     None,

src/libsyntax_ext/deriving/generic/mod.rs

@@ -857,6 +857,7 @@ impl<'a> MethodDef<'a> {
                      explicit_self: ast::ExplicitSelf,
                      arg_types: Vec<(Ident, P<ast::Ty>)> ,
                      body: P<Expr>) -> ast::ImplItem {
+
         // create the generics that aren't for Self
         let fn_generics = self.generics.to_generics(cx, trait_.span, type_ident, generics);
 
@@ -990,6 +991,7 @@ impl<'a> MethodDef<'a> {
             body = cx.expr_match(trait_.span, arg_expr.clone(),
                                      vec!( cx.arm(trait_.span, vec!(pat.clone()), body) ))
         }
+
         body
     }
 

src/test/compile-fail/deriving-copyclone.rs

@@ -0,0 +1,48 @@
+// Copyright 2016 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.
+
+// this will get a no-op Clone impl
+#[derive(Copy, Clone)]
+struct A {
+    a: i32,
+    b: i64
+}
+
+// this will get a deep Clone impl
+#[derive(Copy, Clone)]
+struct B<T> {
+    a: i32,
+    b: T
+}
+
+struct C; // not Copy or Clone
+#[derive(Clone)] struct D; // Clone but not Copy
+
+fn is_copy<T: Copy>(_: T) {}
+fn is_clone<T: Clone>(_: T) {}
+
+fn main() {
+    // A can be copied and cloned
+    is_copy(A { a: 1, b: 2 });
+    is_clone(A { a: 1, b: 2 });
+
+    // B<i32> can be copied and cloned
+    is_copy(B { a: 1, b: 2 });
+    is_clone(B { a: 1, b: 2 });
+
+    // B<C> cannot be copied or cloned
+    is_copy(B { a: 1, b: C }); //~ERROR Copy
+    is_clone(B { a: 1, b: C }); //~ERROR Clone
+
+    // B<D> can be cloned but not copied
+    is_copy(B { a: 1, b: D }); //~ERROR Copy
+    is_clone(B { a: 1, b: D });
+}
+

src/test/run-pass/copy-out-of-array-1.rs

@@ -12,8 +12,6 @@
 //
 // (Compare with compile-fail/move-out-of-array-1.rs)
 
-// pretty-expanded FIXME #23616
-
 #[derive(Copy, Clone)]
 struct C { _x: u8 }
 

src/test/run-pass/deriving-copyclone.rs

@@ -0,0 +1,48 @@
+// Copyright 2016 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.
+
+//! Test that #[derive(Copy, Clone)] produces a shallow copy
+//! even when a member violates RFC 1521
+
+use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering};
+
+/// A struct that pretends to be Copy, but actually does something
+/// in its Clone impl
+#[derive(Copy)]
+struct Liar;
+
+/// Static cooperating with the rogue Clone impl
+static CLONED: AtomicBool = ATOMIC_BOOL_INIT;
+
+impl Clone for Liar {
+    fn clone(&self) -> Self {
+        // this makes Clone vs Copy observable
+        CLONED.store(true, Ordering::SeqCst);
+
+        *self
+    }
+}
+
+/// This struct is actually Copy... at least, it thinks it is!
+#[derive(Copy, Clone)]
+struct Innocent(Liar);
+
+impl Innocent {
+    fn new() -> Self {
+        Innocent(Liar)
+    }
+}
+
+fn main() {
+    let _ = Innocent::new().clone();
+    // if Innocent was byte-for-byte copied, CLONED will still be false
+    assert!(!CLONED.load(Ordering::SeqCst));
+}
+

src/test/run-pass/hrtb-opt-in-copy.rs

@@ -16,8 +16,6 @@
 // did not consider that a match (something I would like to revise in
 // a later PR).
 
-// pretty-expanded FIXME #23616
-
 #![allow(dead_code)]
 
 use std::marker::PhantomData;

src/test/run-pass/issue-13264.rs

@@ -8,8 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// pretty-expanded FIXME #23616
-
 use std::ops::Deref;
 
 struct Root {