From fba94ac3d2314ee0eec1750a2600b1da0f208e8c Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Tue, 5 Apr 2022 19:57:55 +1000 Subject: [PATCH 1/4] Specialize `derive(Clone)` to use `Copy` when applicable --- .../src/deriving/bounds.rs | 1 + .../src/deriving/clone.rs | 95 ++++++++++++------- .../src/deriving/cmp/eq.rs | 1 + .../src/deriving/cmp/ord.rs | 1 + .../src/deriving/cmp/partial_eq.rs | 1 + .../src/deriving/cmp/partial_ord.rs | 1 + .../src/deriving/debug.rs | 1 + .../src/deriving/decodable.rs | 1 + .../src/deriving/default.rs | 1 + .../src/deriving/encodable.rs | 1 + .../src/deriving/generic/mod.rs | 10 +- .../rustc_builtin_macros/src/deriving/hash.rs | 1 + compiler/rustc_span/src/symbol.rs | 2 + library/core/src/clone.rs | 36 +++++++ 14 files changed, 114 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/bounds.rs b/compiler/rustc_builtin_macros/src/deriving/bounds.rs index 12ef166b8b051..094cc7e9640b3 100644 --- a/compiler/rustc_builtin_macros/src/deriving/bounds.rs +++ b/compiler/rustc_builtin_macros/src/deriving/bounds.rs @@ -17,6 +17,7 @@ pub fn expand_deriving_copy( span, attributes: Vec::new(), path: path_std!(marker::Copy), + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs index c952fc0a866f8..d2ca8f9ea82ba 100644 --- a/compiler/rustc_builtin_macros/src/deriving/clone.rs +++ b/compiler/rustc_builtin_macros/src/deriving/clone.rs @@ -15,27 +15,18 @@ pub fn expand_deriving_clone( item: &Annotatable, push: &mut dyn 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. - // - the item is a union with Copy fields - // Unions with generic parameters still can derive Clone because they require Copy - // for deriving, Clone alone is not enough. - // Wherever Clone is implemented for fields is irrelevant so we don't assert it. let bounds; let substructure; + let is_union; let is_shallow; match *item { Annotatable::Item(ref annitem) => match annitem.kind { ItemKind::Struct(_, Generics { ref params, .. }) | ItemKind::Enum(_, Generics { ref params, .. }) => { + // FIXME: although the use of specialization already removes the need for checking whether + // the type already derives `Copy`, `rustc_scalar_valid_range_*` types derive + // `Clone` AND `Copy` and cannot be constructed unless unsafe blocks surround the expression, + // thus this part is preserved. let container_id = cx.current_expansion.id.expn_data().parent.expect_local(); let has_derive_copy = cx.resolver.has_derive_copy(container_id); if has_derive_copy @@ -46,27 +37,27 @@ pub fn expand_deriving_clone( bounds = vec![]; is_shallow = true; substructure = combine_substructure(Box::new(|c, s, sub| { - cs_clone_shallow("Clone", c, s, sub, false) + cs_clone_shallow(c, s, sub, false) })); } else { bounds = vec![]; is_shallow = false; - substructure = - combine_substructure(Box::new(|c, s, sub| cs_clone("Clone", c, s, sub))); + substructure = combine_substructure(Box::new(|c, s, sub| cs_clone(c, s, sub))); } + is_union = false; } ItemKind::Union(..) => { bounds = vec![Literal(path_std!(marker::Copy))]; is_shallow = true; - substructure = combine_substructure(Box::new(|c, s, sub| { - cs_clone_shallow("Clone", c, s, sub, true) - })); + substructure = + combine_substructure(Box::new(|c, s, sub| cs_clone_shallow(c, s, sub, true))); + is_union = true; } _ => { bounds = vec![]; is_shallow = false; - substructure = - combine_substructure(Box::new(|c, s, sub| cs_clone("Clone", c, s, sub))); + is_union = false; + substructure = combine_substructure(Box::new(|c, s, sub| cs_clone(c, s, sub))); } }, @@ -78,7 +69,8 @@ pub fn expand_deriving_clone( let trait_def = TraitDef { span, attributes: Vec::new(), - path: path_std!(clone::Clone), + path: if is_union { path_std!(clone::Clone) } else { path_std!(clone::DerivedClone) }, + bound_current_trait: false, additional_bounds: bounds, generics: Bounds::empty(), is_unsafe: false, @@ -89,7 +81,7 @@ pub fn expand_deriving_clone( explicit_self: borrowed_explicit_self(), args: Vec::new(), ret_ty: Self_, - attributes: attrs, + attributes: attrs.clone(), is_unsafe: false, unify_fieldless_variants: false, combine_substructure: substructure, @@ -97,11 +89,47 @@ pub fn expand_deriving_clone( associated_types: Vec::new(), }; - trait_def.expand_ext(cx, mitem, item, push, is_shallow) + trait_def.expand_ext(cx, mitem, item, push, is_shallow); + + if !is_union { + TraitDef { + span, + attributes: Vec::new(), + path: path_std!(clone::Clone), + bound_current_trait: true, + additional_bounds: vec![], + generics: Bounds::empty(), + is_unsafe: false, + supports_unions: false, + methods: vec![MethodDef { + name: sym::clone, + generics: Bounds::empty(), + explicit_self: borrowed_explicit_self(), + args: Vec::new(), + ret_ty: Self_, + attributes: attrs, + is_unsafe: false, + unify_fieldless_variants: false, + combine_substructure: combine_substructure(Box::new(|c, s, _| { + c.expr_call( + s, + c.expr_path(c.path(s, c.std_path(&[sym::clone, sym::try_copy]))), + vec![ + c.expr_self(s), + c.expr_path( + c.path(s, c.std_path(&[sym::clone, sym::DerivedClone, sym::clone])), + ), + ], + ) + })), + }], + associated_types: vec![], + } + .expand_ext(cx, mitem, item, push, true) + } } fn cs_clone_shallow( - name: &str, cx: &mut ExtCtxt<'_>, trait_span: Span, substr: &Substructure<'_>, @@ -149,7 +177,7 @@ fn cs_clone_shallow( } _ => cx.span_bug( trait_span, - &format!("unexpected substructure in shallow `derive({})`", name), + &format!("unexpected substructure in shallow `derive(Clone)`"), ), } } @@ -157,12 +185,7 @@ fn cs_clone_shallow( cx.expr_block(cx.block(trait_span, stmts)) } -fn cs_clone( - name: &str, - cx: &mut ExtCtxt<'_>, - trait_span: Span, - substr: &Substructure<'_>, -) -> P { +fn cs_clone(cx: &mut ExtCtxt<'_>, trait_span: Span, substr: &Substructure<'_>) -> P { let ctor_path; let all_fields; let fn_path = cx.std_path(&[sym::clone, sym::Clone, sym::clone]); @@ -184,10 +207,10 @@ fn cs_clone( vdata = &variant.data; } EnumNonMatchingCollapsed(..) => { - cx.span_bug(trait_span, &format!("non-matching enum variants in `derive({})`", name,)) + cx.span_bug(trait_span, "non-matching enum variants in `derive(Clone)`") } StaticEnum(..) | StaticStruct(..) => { - cx.span_bug(trait_span, &format!("associated function in `derive({})`", name)) + cx.span_bug(trait_span, "associated function in `derive(Clone)`") } } @@ -199,7 +222,7 @@ fn cs_clone( let Some(ident) = field.name else { cx.span_bug( trait_span, - &format!("unnamed field in normal struct in `derive({})`", name,), + "unnamed field in normal struct in `derive(Clone)`", ); }; let call = subcall(cx, field); diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs index 54ab88dc3ffc9..66e83aa411cc9 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs @@ -25,6 +25,7 @@ pub fn expand_deriving_eq( span, attributes: Vec::new(), path: path_std!(cmp::Eq), + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs index 2b3ac0a86c16c..bf3094fdaafde 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs @@ -21,6 +21,7 @@ pub fn expand_deriving_ord( span, attributes: Vec::new(), path: path_std!(cmp::Ord), + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs index eead8b37024c0..4982c2436c582 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs @@ -100,6 +100,7 @@ pub fn expand_deriving_partial_eq( span, attributes: Vec::new(), path: path_std!(cmp::PartialEq), + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs index d28ac822a1ed9..fa57df5e5e666 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs @@ -44,6 +44,7 @@ pub fn expand_deriving_partial_ord( span, attributes: vec![], path: path_std!(cmp::PartialOrd), + bound_current_trait: true, additional_bounds: vec![], generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/debug.rs b/compiler/rustc_builtin_macros/src/deriving/debug.rs index ecf70da6d96c5..295310b712bf7 100644 --- a/compiler/rustc_builtin_macros/src/deriving/debug.rs +++ b/compiler/rustc_builtin_macros/src/deriving/debug.rs @@ -27,6 +27,7 @@ pub fn expand_deriving_debug( span, attributes: Vec::new(), path: path_std!(fmt::Debug), + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/decodable.rs b/compiler/rustc_builtin_macros/src/deriving/decodable.rs index 1d892b20729d5..bd2e84a9ddbd5 100644 --- a/compiler/rustc_builtin_macros/src/deriving/decodable.rs +++ b/compiler/rustc_builtin_macros/src/deriving/decodable.rs @@ -24,6 +24,7 @@ pub fn expand_deriving_rustc_decodable( span, attributes: Vec::new(), path: Path::new_(vec![krate, sym::Decodable], None, vec![], PathKind::Global), + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index ca83941f600ca..f2d3edfdc5de8 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -28,6 +28,7 @@ pub fn expand_deriving_default( span, attributes: Vec::new(), path: Path::new(vec![kw::Default, sym::Default]), + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/encodable.rs b/compiler/rustc_builtin_macros/src/deriving/encodable.rs index c5f3a9d3379a7..8f2a4070b58a9 100644 --- a/compiler/rustc_builtin_macros/src/deriving/encodable.rs +++ b/compiler/rustc_builtin_macros/src/deriving/encodable.rs @@ -109,6 +109,7 @@ pub fn expand_deriving_rustc_encodable( span, attributes: Vec::new(), path: Path::new_(vec![krate, sym::Encodable], None, vec![], PathKind::Global), + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index f87f4726d1c69..2a1a238390753 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -205,6 +205,8 @@ pub struct TraitDef<'a> { /// Path of the trait, including any type parameters pub path: Path, + pub bound_current_trait: bool, + /// Additional bounds required of any type parameters of the type, /// other than the current trait pub additional_bounds: Vec, @@ -591,7 +593,7 @@ impl<'a> TraitDef<'a> { cx.trait_bound(p.to_path(cx, self.span, type_ident, generics)) }).chain( // require the current trait - iter::once(cx.trait_bound(trait_path.clone())) + self.bound_current_trait.then(|| cx.trait_bound(trait_path.clone())) ).chain( // also add in any bounds from the declaration param.bounds.iter().cloned() @@ -671,8 +673,10 @@ impl<'a> TraitDef<'a> { .map(|p| cx.trait_bound(p.to_path(cx, self.span, type_ident, generics))) .collect(); - // require the current trait - bounds.push(cx.trait_bound(trait_path.clone())); + if self.bound_current_trait { + // require the current trait + bounds.push(cx.trait_bound(trait_path.clone())); + } let predicate = ast::WhereBoundPredicate { span: self.span, diff --git a/compiler/rustc_builtin_macros/src/deriving/hash.rs b/compiler/rustc_builtin_macros/src/deriving/hash.rs index f1d46f03bad8f..026bec8a8974a 100644 --- a/compiler/rustc_builtin_macros/src/deriving/hash.rs +++ b/compiler/rustc_builtin_macros/src/deriving/hash.rs @@ -24,6 +24,7 @@ pub fn expand_deriving_hash( span, attributes: Vec::new(), path, + bound_current_trait: true, additional_bounds: Vec::new(), generics: Bounds::empty(), is_unsafe: false, diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index f5803aaa0786e..e36f413899628 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -168,6 +168,7 @@ symbols! { Decoder, Default, Deref, + DerivedClone, DirBuilder, Display, DoubleEndedIterator, @@ -1412,6 +1413,7 @@ symbols! { truncf32, truncf64, try_blocks, + try_copy, try_from, try_into, try_trait_v2, diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index cfdc51c71ee06..af0c8aa2ee790 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -172,6 +172,42 @@ pub struct AssertParamIsCopy { _field: crate::marker::PhantomData, } +#[doc(hidden)] +#[unstable( + feature = "derive_clone_copy", + reason = "deriving hack, should not be public", + issue = "none" +)] +pub trait DerivedClone: Sized { + fn clone(&self) -> Self; +} + +#[doc(hidden)] +#[unstable( + feature = "derive_clone_copy", + reason = "deriving hack, should not be public", + issue = "none" +)] +pub fn try_copy(x: &T, clone: fn(&T) -> T) -> T { + trait TryCopy { + fn try_copy(&self, clone: fn(&Self) -> Self) -> Self; + } + + impl TryCopy for X { + default fn try_copy(&self, clone: fn(&Self) -> Self) -> Self { + clone(self) + } + } + + impl TryCopy for X { + fn try_copy(&self, _clone: fn(&Self) -> Self) -> Self { + *self + } + } + + TryCopy::try_copy(x, clone) +} + /// Implementations of `Clone` for primitive types. /// /// Implementations that cannot be described in Rust From 45c50ff42bd3411707d997cc8a8093209118486b Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Tue, 5 Apr 2022 20:04:22 +1000 Subject: [PATCH 2/4] Remove `is_union` and use `is_shallow` --- compiler/rustc_builtin_macros/src/deriving/clone.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs index d2ca8f9ea82ba..79ae27342c89f 100644 --- a/compiler/rustc_builtin_macros/src/deriving/clone.rs +++ b/compiler/rustc_builtin_macros/src/deriving/clone.rs @@ -44,19 +44,16 @@ pub fn expand_deriving_clone( is_shallow = false; substructure = combine_substructure(Box::new(|c, s, sub| cs_clone(c, s, sub))); } - is_union = false; } ItemKind::Union(..) => { bounds = vec![Literal(path_std!(marker::Copy))]; is_shallow = true; substructure = combine_substructure(Box::new(|c, s, sub| cs_clone_shallow(c, s, sub, true))); - is_union = true; } _ => { bounds = vec![]; is_shallow = false; - is_union = false; substructure = combine_substructure(Box::new(|c, s, sub| cs_clone(c, s, sub))); } }, @@ -69,7 +66,7 @@ pub fn expand_deriving_clone( let trait_def = TraitDef { span, attributes: Vec::new(), - path: if is_union { path_std!(clone::Clone) } else { path_std!(clone::DerivedClone) }, + path: if is_shallow { path_std!(clone::Clone) } else { path_std!(clone::DerivedClone) }, bound_current_trait: false, additional_bounds: bounds, generics: Bounds::empty(), @@ -91,7 +88,7 @@ pub fn expand_deriving_clone( trait_def.expand_ext(cx, mitem, item, push, is_shallow); - if !is_union { + if !is_shallow { TraitDef { span, attributes: Vec::new(), From e90c1f6e3a5f94fcbac8db0ffed4b53ed51b8bf3 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Tue, 5 Apr 2022 20:06:22 +1000 Subject: [PATCH 3/4] Add Clone bounds for derive --- compiler/rustc_builtin_macros/src/deriving/clone.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs index 79ae27342c89f..cf63223bc7eb2 100644 --- a/compiler/rustc_builtin_macros/src/deriving/clone.rs +++ b/compiler/rustc_builtin_macros/src/deriving/clone.rs @@ -17,7 +17,6 @@ pub fn expand_deriving_clone( ) { let bounds; let substructure; - let is_union; let is_shallow; match *item { Annotatable::Item(ref annitem) => match annitem.kind { @@ -40,7 +39,7 @@ pub fn expand_deriving_clone( cs_clone_shallow(c, s, sub, false) })); } else { - bounds = vec![]; + bounds = vec![Literal(path_std!(clone::Clone))]; is_shallow = false; substructure = combine_substructure(Box::new(|c, s, sub| cs_clone(c, s, sub))); } @@ -52,7 +51,7 @@ pub fn expand_deriving_clone( combine_substructure(Box::new(|c, s, sub| cs_clone_shallow(c, s, sub, true))); } _ => { - bounds = vec![]; + bounds = vec![Literal(path_std!(clone::Clone))]; is_shallow = false; substructure = combine_substructure(Box::new(|c, s, sub| cs_clone(c, s, sub))); } From 95183ec529e176f033bc074fd4f137c9e203661d Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Tue, 5 Apr 2022 20:33:06 +1000 Subject: [PATCH 4/4] Fix dead_code lint --- compiler/rustc_passes/src/dead.rs | 5 +++++ library/core/src/clone.rs | 1 + 2 files changed, 6 insertions(+) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index c777074df4641..f000ff50362ac 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -268,6 +268,11 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { } return true; } + + // ignore generated `DerivedClone` but do not expose this to users. + if Some(trait_of) == self.tcx.get_diagnostic_item(sym::DerivedClone) { + return true; + } } } diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index af0c8aa2ee790..1c8c1f27a62dc 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -178,6 +178,7 @@ pub struct AssertParamIsCopy { reason = "deriving hack, should not be public", issue = "none" )] +#[rustc_diagnostic_item = "DerivedClone"] pub trait DerivedClone: Sized { fn clone(&self) -> Self; }