Try detecting mixed derive/derivative with Clone and Copy

Author: mcarton

src/attr.rs

@@ -41,13 +41,17 @@ pub struct InputClone {
     bounds: Option<Vec<syn::WherePredicate>>,
     /// Whether the implementation should have an explicit `clone_from`.
     pub clone_from: bool,
+    /// Whether the `rustc_copy_clone_marker` was found.
+    pub rustc_copy_clone_marker: bool,
 }
 
 #[derive(Debug, Default)]
 /// Represent the `derivative(Clone(…))` attributes on an input.
 pub struct InputCopy {
     /// The `bound` attribute if present and the corresponding bounds.
     bounds: Option<Vec<syn::WherePredicate>>,
+    /// Wether the input also derive `Clone` (ie. `derive(Clone)`, but not `derivative(Clone)`)
+    derives_clone: bool,
 }
 
 #[derive(Debug, Default)]
@@ -168,21 +172,40 @@ impl Input {
         for_all_attr! {
             for (name, values) in attrs;
             "Clone" => {
+                let mut clone = input.clone.take().unwrap_or_default();
+
+                println!("{:?}", attrs);
+                clone.rustc_copy_clone_marker = attrs
+                    .iter()
+                    .any(|attr| attr.value.name() == "rustc_copy_clone_marker");
+
                 match_attributes! {
-                    let Some(clone) = input.clone;
                     for value in values;
                     "bound" => try!(parse_bound(&mut clone.bounds, value)),
                     "clone_from" => {
                         clone.clone_from = try!(parse_boolean_meta_item(&value, true, "clone_from"));
                     }
                 }
+
+                input.clone = Some(clone);
             }
             "Copy" => {
+                let mut copy = input.copy.take().unwrap_or_default();
+
+                for attr in attrs {
+                    if let syn::MetaItem::List(ref name, ref traits) = attr.value {
+                        if name == "derive" && traits.iter().any(|mi| mi.name() == "Clone") {
+                            copy.derives_clone = true;
+                        }
+                    }
+                }
+
                 match_attributes! {
-                    let Some(copy) = input.copy;
                     for value in values;
                     "bound" => try!(parse_bound(&mut copy.bounds, value)),
                 }
+
+                input.copy = Some(copy);
             }
             "Debug" => {
                 match_attributes! {
@@ -238,6 +261,10 @@ impl Input {
         self.copy.as_ref().map_or(None, |d| d.bounds.as_ref().map(Vec::as_slice))
     }
 
+    pub fn derives_clone(&self) -> bool {
+        self.copy.as_ref().map_or(false, |d| d.derives_clone)
+    }
+
     pub fn debug_bound(&self) -> Option<&[syn::WherePredicate]> {
         self.debug.as_ref().map_or(None, |d| d.bounds.as_ref().map(Vec::as_slice))
     }
@@ -254,6 +281,10 @@ impl Input {
         self.eq.as_ref().map_or(None, |d| d.bounds.as_ref().map(Vec::as_slice))
     }
 
+    pub fn rustc_copy_clone_marker(&self) -> bool {
+        self.clone.as_ref().map_or(false, |d| d.rustc_copy_clone_marker)
+    }
+
     pub fn partial_eq_bound(&self) -> Option<&[syn::WherePredicate]> {
         self.partial_eq.as_ref().map_or(None, |d| d.bounds.as_ref().map(Vec::as_slice))
     }

src/clone.rs

@@ -6,9 +6,13 @@ use syn::{self, aster};
 use utils;
 
 /// Derive `Copy` for `input`.
-pub fn derive_copy(input: &ast::Input) -> quote::Tokens {
+pub fn derive_copy(input: &ast::Input) -> Result<quote::Tokens, String> {
     let name = &input.ident;
 
+    if input.attrs.derives_clone() {
+        return Err("`#[derivative(Copy)]` can't be used with `#[derive(Clone)]`".into());
+    }
+
     let copy_trait_path = copy_trait_path();
     let impl_generics = utils::build_impl_generics(
         input,
@@ -25,10 +29,10 @@ pub fn derive_copy(input: &ast::Input) -> quote::Tokens {
                              .build()
                              .build();
 
-    quote! {
+    Ok(quote! {
         #[allow(unused_qualifications)]
         impl #impl_generics #copy_trait_path for #ty #where_clause {}
-    }
+    })
 }
 
 /// Derive `Clone` for `input`.
@@ -51,7 +55,8 @@ pub fn derive_clone(input: &ast::Input) -> quote::Tokens {
                              .build()
                              .build();
 
-    if input.attrs.copy.is_some() && input.generics.ty_params.is_empty() {
+    let is_copy = input.attrs.rustc_copy_clone_marker() || input.attrs.copy.is_some();
+    if is_copy && input.generics.ty_params.is_empty() {
         quote! {
             #[allow(unused_qualifications)]
             impl #impl_generics #clone_trait_path for #ty #where_clause {

src/lib.rs

@@ -25,7 +25,7 @@ fn derive_impls(input: &ast::Input) -> Result<quote::Tokens, String> {
         tokens.append(&clone::derive_clone(input).to_string());
     }
     if input.attrs.copy.is_some() {
-        tokens.append(&clone::derive_copy(input).to_string());
+        tokens.append(&try!(clone::derive_copy(input)).to_string());
     }
     if input.attrs.debug.is_some() {
         tokens.append(&debug::derive(input).to_string());

tests/compile-fail/derive-copy-clone.rs

@@ -0,0 +1,14 @@
+#![feature(proc_macro)]
+
+#[macro_use]
+extern crate derivative;
+
+#[derive(Derivative)]
+//~^ ERROR custom derive attribute panicked
+//~| HELP `#[derivative(Copy)]` can't be used with `#[derive(Clone)]`
+#[derivative(Copy)]
+#[derive(Clone)]
+struct OurTheir1;
+
+fn main() {
+}

tests/run-pass/rustc-deriving-copyclone.rs

@@ -46,6 +46,15 @@ struct OurOur1(Liar);
 #[derivative(Clone, Copy)]
 struct OurOur2(Liar);
 
+#[derive(Copy)]
+#[derive(Derivative)]
+#[derivative(Clone)]
+struct TheirOur1(Liar);
+#[derive(Copy)]
+#[derive(Derivative)]
+#[derivative(Clone)]
+struct TheirOur2(Liar);
+
 fn main() {
     let _ = TheirTheir(Liar).clone();
     assert!(!CLONED.load(Ordering::SeqCst), "TheirTheir");
@@ -54,4 +63,12 @@ fn main() {
     assert!(!CLONED.load(Ordering::SeqCst), "OurOur1");
     let _ = OurOur2(Liar).clone();
     assert!(!CLONED.load(Ordering::SeqCst), "OurOur2");
+
+    // Ideally this would work the same, just testing that the behaviour does not change:
+    CLONED.store(false, Ordering::SeqCst);
+    let _ = TheirOur1(Liar).clone();
+    assert!(CLONED.load(Ordering::SeqCst), "TheirOur1");
+    CLONED.store(false, Ordering::SeqCst);
+    let _ = TheirOur2(Liar).clone();
+    assert!(CLONED.load(Ordering::SeqCst), "TheirOur2");
 }