Skip to content

Commit 8f55218

Browse files
borsbors
committed
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.
2 parents 897199a + 9249e6a commit 8f55218

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
@@ -857,6 +857,7 @@ impl<'a> MethodDef<'a> {
857857
explicit_self: ast::ExplicitSelf,
858858
arg_types: Vec<(Ident, P<ast::Ty>)> ,
859859
body: P<Expr>) -> ast::ImplItem {
860+
860861
// create the generics that aren't for Self
861862
let fn_generics = self.generics.to_generics(cx, trait_.span, type_ident, generics);
862863

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

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

+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+
// 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

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

+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)