]> git.lizzy.rs Git - rust.git/commitdiff
shallow Clone for #[derive(Copy,Clone)]
authorAlex Burka <aburka@seas.upenn.edu>
Thu, 4 Feb 2016 00:40:59 +0000 (19:40 -0500)
committerAlex Burka <aburka@seas.upenn.edu>
Tue, 26 Apr 2016 17:49:29 +0000 (13:49 -0400)
Changes #[derive(Copy, Clone)] to use a faster impl of Clone when
both derives are present, and there are no generics in the type.

The faster impl is simply returning *self (which works because the
type is also Copy). See the comments in libsyntax_ext/deriving/clone.rs
for more details.

There are a few types which are Copy but not Clone, in violation
of the definition of Copy. These include large arrays and tuples. The
very existence of these types is arguably a bug, but in order for this
optimization not to change the applicability of #[derive(Copy, Clone)],
the faster Clone impl also injects calls to a new function,
core::clone::assert_receiver_is_clone, to verify that all members are
actually Clone.

This is not a breaking change, because pursuant to RFC 1521, any type
that implements Copy should not do any observable work in its Clone
impl.

src/libcore/clone.rs
src/libsyntax_ext/deriving/clone.rs
src/libsyntax_ext/deriving/decodable.rs
src/libsyntax_ext/deriving/encodable.rs
src/libsyntax_ext/deriving/generic/mod.rs
src/test/compile-fail/deriving-copyclone.rs [new file with mode: 0644]
src/test/run-pass/copy-out-of-array-1.rs
src/test/run-pass/deriving-copyclone.rs [new file with mode: 0644]
src/test/run-pass/hrtb-opt-in-copy.rs
src/test/run-pass/issue-13264.rs
src/test/run-pass/issue-3121.rs

index a793502e58d371e440d4743c1f54dced7f3d6331..ad2a205a82376c7471460168d33043327bc0bda4 100644 (file)
@@ -75,6 +75,17 @@ fn clone_from(&mut self, source: &Self) {
     }
 }
 
+// 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.
index 0085182d1ac2af018d5bff4821b4b8265d177eb3..c81198d4729fb55e5949793b5be7c9f9039a66cb 100644 (file)
 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)
+                }
+            }
         }
     }
 }
index 49f14c937e953771e442fd3f315733dfcadee607..9387cf05ac7d142c8d0eebc3319a20c0bba94a91 100644 (file)
@@ -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,
index a05bd7869b2a9ccd1065c375c6fb4f28af1b748c..daa352275d84e7a27268359fc0a7619f08c3cf63 100644 (file)
@@ -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,
index b8ba1a58f2163924fc95b5265abde5bb18f71a4f..56df0ff0e5b91b59c80c2a2cae68873f75360166 100644 (file)
@@ -858,6 +858,7 @@ fn create_method(&self,
                      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);
 
@@ -991,6 +992,7 @@ fn expand_struct_method_body<'b>(&self,
             body = cx.expr_match(trait_.span, arg_expr.clone(),
                                      vec!( cx.arm(trait_.span, vec!(pat.clone()), body) ))
         }
+
         body
     }
 
diff --git a/src/test/compile-fail/deriving-copyclone.rs b/src/test/compile-fail/deriving-copyclone.rs
new file mode 100644 (file)
index 0000000..92fb7c5
--- /dev/null
@@ -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 });
+}
+
index 5c5765454d457adefd99ef00949d845a7302099d..54147c73ff0ff014d02576be5600f261fe8423d4 100644 (file)
@@ -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 }
 
diff --git a/src/test/run-pass/deriving-copyclone.rs b/src/test/run-pass/deriving-copyclone.rs
new file mode 100644 (file)
index 0000000..96d0406
--- /dev/null
@@ -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));
+}
+
index b40f4d27a9c4d63a86e0b6d953fb37122682d2dc..f0214d3f37b6f2f152924848e881e199989f9a19 100644 (file)
@@ -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;
index 7acabf31c85a17587ed16823161e47b8545becf3..383c1aef23459e2e4716871a1709593ba132968a 100644 (file)
@@ -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 {
index 777e5bf7a6dedfe37f1cbd02b7ddad32eb11a3dc..6e9ee7fb15c34620e6445dfe9f3463f89e6551fa 100644 (file)
@@ -8,8 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// pretty-expanded FIXME #23616
-
 #![allow(unknown_features)]
 #![feature(box_syntax)]