Skip to content

Commit 9249e6a

Browse files
committed
shallow Clone for #[derive(Copy,Clone)]
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.
1 parent 42ea682 commit 9249e6a

File tree

11 files changed

+203
-40
lines changed

11 files changed

+203
-40
lines changed

src/libcore/clone.rs

+11
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,17 @@ pub trait Clone : Sized {
7575
}
7676
}
7777

78+
// FIXME(aburka): this method is used solely by #[derive] to
79+
// assert that every component of a type implements Clone.
80+
//
81+
// This should never be called by user code.
82+
#[doc(hidden)]
83+
#[inline(always)]
84+
#[unstable(feature = "derive_clone_copy",
85+
reason = "deriving hack, should not be public",
86+
issue = "0")]
87+
pub fn assert_receiver_is_clone<T: Clone + ?Sized>(_: &T) {}
88+
7889
#[stable(feature = "rust1", since = "1.0.0")]
7990
impl<'a, T: ?Sized> Clone for &'a T {
8091
/// Returns a shallow copy of the reference.

src/libsyntax_ext/deriving/clone.rs

+92-30
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,68 @@
1111
use deriving::generic::*;
1212
use deriving::generic::ty::*;
1313

14-
use syntax::ast::{MetaItem, Expr, VariantData};
14+
use syntax::ast::{Expr, ItemKind, Generics, MetaItem, VariantData};
15+
use syntax::attr::{self, AttrMetaMethods};
1516
use syntax::codemap::Span;
1617
use syntax::ext::base::{ExtCtxt, Annotatable};
1718
use syntax::ext::build::AstBuilder;
1819
use syntax::parse::token::InternedString;
1920
use syntax::ptr::P;
2021

22+
#[derive(PartialEq)]
23+
enum Mode { Deep, Shallow }
24+
2125
pub fn expand_deriving_clone(cx: &mut ExtCtxt,
2226
span: Span,
2327
mitem: &MetaItem,
2428
item: &Annotatable,
2529
push: &mut FnMut(Annotatable))
2630
{
31+
// check if we can use a short form
32+
//
33+
// the short form is `fn clone(&self) -> Self { *self }`
34+
//
35+
// we can use the short form if:
36+
// - the item is Copy (unfortunately, all we can check is whether it's also deriving Copy)
37+
// - there are no generic parameters (after specialization this limitation can be removed)
38+
// if we used the short form with generics, we'd have to bound the generics with
39+
// Clone + Copy, and then there'd be no Clone impl at all if the user fills in something
40+
// that is Clone but not Copy. and until specialization we can't write both impls.
41+
let bounds;
42+
let substructure;
43+
match *item {
44+
Annotatable::Item(ref annitem) => {
45+
match annitem.node {
46+
ItemKind::Struct(_, Generics { ref ty_params, .. }) |
47+
ItemKind::Enum(_, Generics { ref ty_params, .. })
48+
if ty_params.is_empty()
49+
&& attr::contains_name(&annitem.attrs, "derive_Copy") => {
50+
51+
bounds = vec![Literal(path_std!(cx, core::marker::Copy))];
52+
substructure = combine_substructure(Box::new(|c, s, sub| {
53+
cs_clone("Clone", c, s, sub, Mode::Shallow)
54+
}));
55+
}
56+
57+
_ => {
58+
bounds = vec![];
59+
substructure = combine_substructure(Box::new(|c, s, sub| {
60+
cs_clone("Clone", c, s, sub, Mode::Deep)
61+
}));
62+
}
63+
}
64+
}
65+
66+
_ => cx.span_bug(span, "#[derive(Clone)] on trait item or impl item")
67+
}
68+
2769
let inline = cx.meta_word(span, InternedString::new("inline"));
2870
let attrs = vec!(cx.attribute(span, inline));
2971
let trait_def = TraitDef {
3072
span: span,
3173
attributes: Vec::new(),
3274
path: path_std!(cx, core::clone::Clone),
33-
additional_bounds: Vec::new(),
75+
additional_bounds: bounds,
3476
generics: LifetimeBounds::empty(),
3577
is_unsafe: false,
3678
methods: vec!(
@@ -42,9 +84,7 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt,
4284
ret_ty: Self_,
4385
attributes: attrs,
4486
is_unsafe: false,
45-
combine_substructure: combine_substructure(Box::new(|c, s, sub| {
46-
cs_clone("Clone", c, s, sub)
47-
})),
87+
combine_substructure: substructure,
4888
}
4989
),
5090
associated_types: Vec::new(),
@@ -56,14 +96,24 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt,
5696
fn cs_clone(
5797
name: &str,
5898
cx: &mut ExtCtxt, trait_span: Span,
59-
substr: &Substructure) -> P<Expr> {
99+
substr: &Substructure,
100+
mode: Mode) -> P<Expr> {
60101
let ctor_path;
61102
let all_fields;
62-
let fn_path = cx.std_path(&["clone", "Clone", "clone"]);
103+
let fn_path = match mode {
104+
Mode::Shallow => cx.std_path(&["clone", "assert_receiver_is_clone"]),
105+
Mode::Deep => cx.std_path(&["clone", "Clone", "clone"]),
106+
};
63107
let subcall = |field: &FieldInfo| {
64108
let args = vec![cx.expr_addr_of(field.span, field.self_.clone())];
65109

66-
cx.expr_call_global(field.span, fn_path.clone(), args)
110+
let span = if mode == Mode::Shallow {
111+
// set the expn ID so we can call the unstable method
112+
Span { expn_id: cx.backtrace(), .. trait_span }
113+
} else {
114+
field.span
115+
};
116+
cx.expr_call_global(span, fn_path.clone(), args)
67117
};
68118

69119
let vdata;
@@ -89,29 +139,41 @@ fn cs_clone(
89139
}
90140
}
91141

92-
match *vdata {
93-
VariantData::Struct(..) => {
94-
let fields = all_fields.iter().map(|field| {
95-
let ident = match field.name {
96-
Some(i) => i,
97-
None => {
98-
cx.span_bug(trait_span,
99-
&format!("unnamed field in normal struct in \
100-
`derive({})`", name))
101-
}
102-
};
103-
cx.field_imm(field.span, ident, subcall(field))
104-
}).collect::<Vec<_>>();
105-
106-
cx.expr_struct(trait_span, ctor_path, fields)
142+
match mode {
143+
Mode::Shallow => {
144+
cx.expr_block(cx.block(trait_span,
145+
all_fields.iter()
146+
.map(subcall)
147+
.map(|e| cx.stmt_expr(e))
148+
.collect(),
149+
Some(cx.expr_deref(trait_span, cx.expr_self(trait_span)))))
107150
}
108-
VariantData::Tuple(..) => {
109-
let subcalls = all_fields.iter().map(subcall).collect();
110-
let path = cx.expr_path(ctor_path);
111-
cx.expr_call(trait_span, path, subcalls)
112-
}
113-
VariantData::Unit(..) => {
114-
cx.expr_path(ctor_path)
151+
Mode::Deep => {
152+
match *vdata {
153+
VariantData::Struct(..) => {
154+
let fields = all_fields.iter().map(|field| {
155+
let ident = match field.name {
156+
Some(i) => i,
157+
None => {
158+
cx.span_bug(trait_span,
159+
&format!("unnamed field in normal struct in \
160+
`derive({})`", name))
161+
}
162+
};
163+
cx.field_imm(field.span, ident, subcall(field))
164+
}).collect::<Vec<_>>();
165+
166+
cx.expr_struct(trait_span, ctor_path, fields)
167+
}
168+
VariantData::Tuple(..) => {
169+
let subcalls = all_fields.iter().map(subcall).collect();
170+
let path = cx.expr_path(ctor_path);
171+
cx.expr_call(trait_span, path, subcalls)
172+
}
173+
VariantData::Unit(..) => {
174+
cx.expr_path(ctor_path)
175+
}
176+
}
115177
}
116178
}
117179
}

src/libsyntax_ext/deriving/decodable.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ fn expand_deriving_decodable_imp(cx: &mut ExtCtxt,
7474
},
7575
explicit_self: None,
7676
args: vec!(Ptr(Box::new(Literal(Path::new_local(typaram))),
77-
Borrowed(None, Mutability::Mutable))),
77+
Borrowed(None, Mutability::Mutable))),
7878
ret_ty: Literal(Path::new_(
7979
pathvec_std!(cx, core::result::Result),
8080
None,

src/libsyntax_ext/deriving/encodable.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ fn expand_deriving_encodable_imp(cx: &mut ExtCtxt,
150150
},
151151
explicit_self: borrowed_explicit_self(),
152152
args: vec!(Ptr(Box::new(Literal(Path::new_local(typaram))),
153-
Borrowed(None, Mutability::Mutable))),
153+
Borrowed(None, Mutability::Mutable))),
154154
ret_ty: Literal(Path::new_(
155155
pathvec_std!(cx, core::result::Result),
156156
None,

src/libsyntax_ext/deriving/generic/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,7 @@ impl<'a> MethodDef<'a> {
858858
explicit_self: ast::ExplicitSelf,
859859
arg_types: Vec<(Ident, P<ast::Ty>)> ,
860860
body: P<Expr>) -> ast::ImplItem {
861+
861862
// create the generics that aren't for Self
862863
let fn_generics = self.generics.to_generics(cx, trait_.span, type_ident, generics);
863864

@@ -991,6 +992,7 @@ impl<'a> MethodDef<'a> {
991992
body = cx.expr_match(trait_.span, arg_expr.clone(),
992993
vec!( cx.arm(trait_.span, vec!(pat.clone()), body) ))
993994
}
995+
994996
body
995997
}
996998

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// this will get a no-op Clone impl
12+
#[derive(Copy, Clone)]
13+
struct A {
14+
a: i32,
15+
b: i64
16+
}
17+
18+
// this will get a deep Clone impl
19+
#[derive(Copy, Clone)]
20+
struct B<T> {
21+
a: i32,
22+
b: T
23+
}
24+
25+
struct C; // not Copy or Clone
26+
#[derive(Clone)] struct D; // Clone but not Copy
27+
28+
fn is_copy<T: Copy>(_: T) {}
29+
fn is_clone<T: Clone>(_: T) {}
30+
31+
fn main() {
32+
// A can be copied and cloned
33+
is_copy(A { a: 1, b: 2 });
34+
is_clone(A { a: 1, b: 2 });
35+
36+
// B<i32> can be copied and cloned
37+
is_copy(B { a: 1, b: 2 });
38+
is_clone(B { a: 1, b: 2 });
39+
40+
// B<C> cannot be copied or cloned
41+
is_copy(B { a: 1, b: C }); //~ERROR Copy
42+
is_clone(B { a: 1, b: C }); //~ERROR Clone
43+
44+
// B<D> can be cloned but not copied
45+
is_copy(B { a: 1, b: D }); //~ERROR Copy
46+
is_clone(B { a: 1, b: D });
47+
}
48+

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

-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
//
1313
// (Compare with compile-fail/move-out-of-array-1.rs)
1414

15-
// pretty-expanded FIXME #23616
16-
1715
#[derive(Copy, Clone)]
1816
struct C { _x: u8 }
1917

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
//! Test that #[derive(Copy, Clone)] produces a shallow copy
12+
//! even when a member violates RFC 1521
13+
14+
use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering};
15+
16+
/// A struct that pretends to be Copy, but actually does something
17+
/// in its Clone impl
18+
#[derive(Copy)]
19+
struct Liar;
20+
21+
/// Static cooperating with the rogue Clone impl
22+
static CLONED: AtomicBool = ATOMIC_BOOL_INIT;
23+
24+
impl Clone for Liar {
25+
fn clone(&self) -> Self {
26+
// this makes Clone vs Copy observable
27+
CLONED.store(true, Ordering::SeqCst);
28+
29+
*self
30+
}
31+
}
32+
33+
/// This struct is actually Copy... at least, it thinks it is!
34+
#[derive(Copy, Clone)]
35+
struct Innocent(Liar);
36+
37+
impl Innocent {
38+
fn new() -> Self {
39+
Innocent(Liar)
40+
}
41+
}
42+
43+
fn main() {
44+
let _ = Innocent::new().clone();
45+
// if Innocent was byte-for-byte copied, CLONED will still be false
46+
assert!(!CLONED.load(Ordering::SeqCst));
47+
}
48+

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

-2
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
// did not consider that a match (something I would like to revise in
1717
// a later PR).
1818

19-
// pretty-expanded FIXME #23616
20-
2119
#![allow(dead_code)]
2220

2321
use std::marker::PhantomData;

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

-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// pretty-expanded FIXME #23616
12-
1311
use std::ops::Deref;
1412

1513
struct Root {

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

-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// pretty-expanded FIXME #23616
12-
1311
#![allow(unknown_features)]
1412
#![feature(box_syntax)]
1513

0 commit comments

Comments
 (0)