Auto merge of rust-lang#135634 - joboet:trivial-clone, r=<try>

stop specializing on `Copy`

fixes rust-lang#132442

`std` specializes on `Copy` to optimize certain library functions such as `clone_from_slice`. This is unsound, however, as the `Copy` implementation may not be always applicable because of lifetime bounds, which specialization does not take into account; the result being that values are copied even though they are not `Copy`. For instance, this code:
```rust
struct SometimesCopy<'a>(&'a Cell<bool>);

impl<'a> Clone for SometimesCopy<'a> {
    fn clone(&self) -> Self {
        self.0.set(true);
        Self(self.0)
    }
}

impl Copy for SometimesCopy<'static> {}

let clone_called = Cell::new(false);
// As SometimesCopy<'clone_called> is not 'static, this must run `clone`,
// setting the value to `true`.
let _ = [SometimesCopy(&clone_called)].clone();
assert!(clone_called.get());
```
should not panic, but does ([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=6be7a48cad849d8bd064491616fdb43c)).

To solve this, this PR introduces a new `unsafe` trait: `TrivialClone`. This trait may be implemented whenever the `Clone` implementation is equivalent to copying the value (so e.g. `fn clone(&self) -> Self { *self }`). Because of lifetime erasure, there is no way for the `Clone` implementation to observe lifetime bounds, meaning that even if the `TrivialClone` has stricter bounds than the `Clone` implementation, its invariant still holds. Therefore, it is sound to specialize on `TrivialClone`.

I've changed all `Copy` specializations in the standard library to specialize on `TrivialClone` instead. Unfortunately, the unsound `#[rustc_unsafe_specialization_marker]` attribute on `Copy` cannot be removed in this PR as `hashbrown` still depends on it. I'll make a PR updating `hashbrown` once this lands.

With `Copy` no longer being considered for specialization, this change alone would result in the standard library optimizations not being applied for user types unaware of `TrivialClone`. To avoid this and restore the optimizations in most cases, I have changed the expansion of `#[derive(Clone)]`: Currently, whenever both `Clone` and `Copy` are derived, the `clone` method performs a copy of the value. With this PR, the derive macro also adds a `TrivialClone` implementation to make this case observable using specialization. I anticipate that most users will use `#[derive(Clone, Copy)]` whenever both are applicable, so most users will still profit from the library optimizations.

Unfortunately, Hyrum's law applies to this PR: there are some popular crates which rely on the precise specialization behaviour of `core` to implement "specialization at home", e.g. [`libAFL`](https://github.com/AFLplusplus/LibAFL/blob/89cff637025c1652c24e8d97a30a2e3d01f187a4/libafl_bolts/src/tuples.rs#L27-L49). I have no remorse for breaking such horrible code, but perhaps we should open other, better ways to satisfy their needs – for example by dropping the `'static` bound on `TypeId::of`...

Author: bors

compiler/rustc_builtin_macros/src/deriving/bounds.rs

@@ -1,4 +1,4 @@
-use rustc_ast::MetaItem;
+use rustc_ast::{MetaItem, Safety};
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_span::Span;
 
@@ -23,6 +23,8 @@ pub(crate) fn expand_deriving_copy(
         methods: Vec::new(),
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
 
     trait_def.expand(cx, mitem, item, push);
@@ -46,6 +48,8 @@ pub(crate) fn expand_deriving_const_param_ty(
         methods: Vec::new(),
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
 
     trait_def.expand(cx, mitem, item, push);
@@ -60,6 +64,8 @@ pub(crate) fn expand_deriving_const_param_ty(
         methods: Vec::new(),
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
 
     trait_def.expand(cx, mitem, item, push);
@@ -83,6 +89,8 @@ pub(crate) fn expand_deriving_unsized_const_param_ty(
         methods: Vec::new(),
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
 
     trait_def.expand(cx, mitem, item, push);

compiler/rustc_builtin_macros/src/deriving/clone.rs

@@ -1,7 +1,7 @@
-use rustc_ast::{self as ast, Generics, ItemKind, MetaItem, VariantData};
+use rustc_ast::{self as ast, Generics, ItemKind, MetaItem, Safety, VariantData};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_expand::base::{Annotatable, ExtCtxt};
-use rustc_span::{Ident, Span, kw, sym};
+use rustc_span::{DUMMY_SP, Ident, Span, kw, sym};
 use thin_vec::{ThinVec, thin_vec};
 
 use crate::deriving::generic::ty::*;
@@ -68,6 +68,28 @@ pub(crate) fn expand_deriving_clone(
         _ => cx.dcx().span_bug(span, "`#[derive(Clone)]` on trait item or impl item"),
     }
 
+    // If the clone method is just copying the value, also mark the type as
+    // `TrivialClone` to allow some library optimizations.
+    if is_simple {
+        let trivial_def = TraitDef {
+            span,
+            path: path_std!(clone::TrivialClone),
+            skip_path_as_bound: false,
+            needs_copy_as_bound_if_packed: true,
+            additional_bounds: bounds.clone(),
+            supports_unions: true,
+            methods: Vec::new(),
+            associated_types: Vec::new(),
+            is_const,
+            safety: Safety::Unsafe(DUMMY_SP),
+            // `TrivialClone` is not part of an API guarantee, so it shouldn't
+            // appear in rustdoc output.
+            document: false,
+        };
+
+        trivial_def.expand_ext(cx, mitem, item, push, true);
+    }
+
     let trait_def = TraitDef {
         span,
         path: path_std!(clone::Clone),
@@ -87,6 +109,8 @@ pub(crate) fn expand_deriving_clone(
         }],
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
 
     trait_def.expand_ext(cx, mitem, item, push, is_simple)

compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs

@@ -1,4 +1,4 @@
-use rustc_ast::{self as ast, MetaItem};
+use rustc_ast::{self as ast, MetaItem, Safety};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_span::{Span, sym};
@@ -43,6 +43,8 @@ pub(crate) fn expand_deriving_eq(
         }],
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
     trait_def.expand_ext(cx, mitem, item, push, true)
 }

compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs

@@ -1,4 +1,4 @@
-use rustc_ast::MetaItem;
+use rustc_ast::{MetaItem, Safety};
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_span::{Ident, Span, sym};
 use thin_vec::thin_vec;
@@ -34,6 +34,8 @@ pub(crate) fn expand_deriving_ord(
         }],
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
 
     trait_def.expand(cx, mitem, item, push)

compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs

@@ -1,5 +1,5 @@
 use rustc_ast::ptr::P;
-use rustc_ast::{BinOpKind, BorrowKind, Expr, ExprKind, MetaItem, Mutability};
+use rustc_ast::{BinOpKind, BorrowKind, Expr, ExprKind, MetaItem, Mutability, Safety};
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_span::{Span, sym};
 use thin_vec::thin_vec;
@@ -84,6 +84,8 @@ pub(crate) fn expand_deriving_partial_eq(
         methods: Vec::new(),
         associated_types: Vec::new(),
         is_const: false,
+        safety: Safety::Default,
+        document: true,
     };
     structural_trait_def.expand(cx, mitem, item, push);
 
@@ -110,6 +112,8 @@ pub(crate) fn expand_deriving_partial_eq(
         methods,
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
     trait_def.expand(cx, mitem, item, push)
 }

compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs

@@ -1,4 +1,4 @@
-use rustc_ast::{ExprKind, ItemKind, MetaItem, PatKind};
+use rustc_ast::{ExprKind, ItemKind, MetaItem, PatKind, Safety};
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_span::{Ident, Span, sym};
 use thin_vec::thin_vec;
@@ -64,6 +64,8 @@ pub(crate) fn expand_deriving_partial_ord(
         methods: vec![partial_cmp_def],
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
     trait_def.expand(cx, mitem, item, push)
 }

compiler/rustc_builtin_macros/src/deriving/debug.rs

@@ -1,4 +1,4 @@
-use rustc_ast::{self as ast, EnumDef, MetaItem};
+use rustc_ast::{self as ast, EnumDef, MetaItem, Safety};
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_session::config::FmtDebug;
 use rustc_span::{Ident, Span, Symbol, sym};
@@ -41,6 +41,8 @@ pub(crate) fn expand_deriving_debug(
         }],
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
     trait_def.expand(cx, mitem, item, push)
 }

compiler/rustc_builtin_macros/src/deriving/default.rs

@@ -1,8 +1,7 @@
 use core::ops::ControlFlow;
 
-use rustc_ast as ast;
 use rustc_ast::visit::visit_opt;
-use rustc_ast::{EnumDef, VariantData, attr};
+use rustc_ast::{self as ast, EnumDef, Safety, VariantData, attr};
 use rustc_expand::base::{Annotatable, DummyResult, ExtCtxt};
 use rustc_span::{ErrorGuaranteed, Ident, Span, kw, sym};
 use smallvec::SmallVec;
@@ -51,6 +50,8 @@ pub(crate) fn expand_deriving_default(
         }],
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
     trait_def.expand(cx, mitem, item, push)
 }

compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

@@ -183,7 +183,7 @@ pub(crate) use SubstructureFields::*;
 use rustc_ast::ptr::P;
 use rustc_ast::{
     self as ast, AnonConst, BindingMode, ByRef, EnumDef, Expr, GenericArg, GenericParamKind,
-    Generics, Mutability, PatKind, VariantData,
+    Generics, Mutability, PatKind, Safety, VariantData,
 };
 use rustc_attr_parsing::{AttributeKind, AttributeParser, ReprPacked};
 use rustc_expand::base::{Annotatable, ExtCtxt};
@@ -221,6 +221,12 @@ pub(crate) struct TraitDef<'a> {
     pub associated_types: Vec<(Ident, Ty)>,
 
     pub is_const: bool,
+
+    /// The safety of the `impl`.
+    pub safety: Safety,
+
+    /// Whether the added `impl` should appear in rustdoc output.
+    pub document: bool,
 }
 
 pub(crate) struct MethodDef<'a> {
@@ -784,15 +790,19 @@ impl<'a> TraitDef<'a> {
         let path = cx.path_all(self.span, false, vec![type_ident], self_params);
         let self_type = cx.ty_path(path);
 
-        let attrs = thin_vec![cx.attr_word(sym::automatically_derived, self.span),];
+        let mut attrs = thin_vec![cx.attr_word(sym::automatically_derived, self.span),];
+        if !self.document {
+            attrs.push(cx.attr_nested_word(sym::doc, sym::hidden, self.span));
+        }
+
         let opt_trait_ref = Some(trait_ref);
 
         cx.item(
             self.span,
             Ident::empty(),
             attrs,
             ast::ItemKind::Impl(Box::new(ast::Impl {
-                safety: ast::Safety::Default,
+                safety: self.safety,
                 polarity: ast::ImplPolarity::Positive,
                 defaultness: ast::Defaultness::Final,
                 constness: if self.is_const { ast::Const::Yes(DUMMY_SP) } else { ast::Const::No },

compiler/rustc_builtin_macros/src/deriving/hash.rs

@@ -1,4 +1,4 @@
-use rustc_ast::{MetaItem, Mutability};
+use rustc_ast::{MetaItem, Mutability, Safety};
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_span::{Span, sym};
 use thin_vec::thin_vec;
@@ -41,6 +41,8 @@ pub(crate) fn expand_deriving_hash(
         }],
         associated_types: Vec::new(),
         is_const,
+        safety: Safety::Default,
+        document: true,
     };
 
     hash_trait_def.expand(cx, mitem, item, push);

compiler/rustc_hir/src/lang_items.rs

@@ -172,6 +172,7 @@ language_item_table! {
     Clone,                   sym::clone,               clone_trait,                Target::Trait,          GenericRequirement::None;
     CloneFn,                 sym::clone_fn,            clone_fn,                   Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;
     UseCloned,               sym::use_cloned,          use_cloned_trait,           Target::Trait,          GenericRequirement::None;
+    TrivialClone,            sym::trivial_clone,       trivial_clone_trait,        Target::Trait,          GenericRequirement::None;
     Sync,                    sym::sync,                sync_trait,                 Target::Trait,          GenericRequirement::Exact(0);
     DiscriminantKind,        sym::discriminant_kind,   discriminant_kind_trait,    Target::Trait,          GenericRequirement::None;
     /// The associated item of the `DiscriminantKind` trait.

compiler/rustc_middle/src/ty/context.rs

@@ -719,6 +719,7 @@ bidirectional_lang_item_map! {
     Poll,
     Sized,
     TransmuteTrait,
+    TrivialClone,
     Tuple,
     Unpin,
     Unsize,

compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs

@@ -397,9 +397,11 @@ where
         } else {
             match cx.as_lang_item(trait_def_id) {
                 Some(TraitSolverLangItem::Sized) => G::consider_builtin_sized_candidate(self, goal),
-                Some(TraitSolverLangItem::Copy | TraitSolverLangItem::Clone) => {
-                    G::consider_builtin_copy_clone_candidate(self, goal)
-                }
+                Some(
+                    TraitSolverLangItem::Copy
+                    | TraitSolverLangItem::Clone
+                    | TraitSolverLangItem::TrivialClone,
+                ) => G::consider_builtin_copy_clone_candidate(self, goal),
                 Some(TraitSolverLangItem::Fn) => {
                     G::consider_builtin_fn_trait_candidates(self, goal, ty::ClosureKind::Fn)
                 }

compiler/rustc_span/src/symbol.rs

@@ -364,6 +364,7 @@ symbols! {
         ToString,
         TokenStream,
         Trait,
+        TrivialClone,
         Try,
         TryCaptureGeneric,
         TryCapturePrintable,
@@ -2083,6 +2084,7 @@ symbols! {
         transparent_enums,
         transparent_unions,
         trivial_bounds,
+        trivial_clone,
         truncf128,
         truncf16,
         truncf32,

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

@@ -108,7 +108,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     &mut candidates,
                 );
             } else {
-                if tcx.is_lang_item(def_id, LangItem::Clone) {
+                if tcx.is_lang_item(def_id, LangItem::Clone)
+                    || tcx.is_lang_item(def_id, LangItem::TrivialClone)
+                {
                     // Same builtin conditions as `Copy`, i.e., every type which has builtin support
                     // for `Copy` also has builtin support for `Clone`, and tuples/arrays of `Clone`
                     // types have builtin support for `Clone`.

compiler/rustc_trait_selection/src/traits/select/confirmation.rs

@@ -261,6 +261,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 self.copy_clone_conditions(obligation)
             } else if tcx.is_lang_item(trait_def, LangItem::Clone) {
                 self.copy_clone_conditions(obligation)
+            } else if tcx.is_lang_item(trait_def, LangItem::TrivialClone) {
+                self.copy_clone_conditions(obligation)
             } else if tcx.is_lang_item(trait_def, LangItem::FusedIterator) {
                 self.fused_iterator_conditions(obligation)
             } else {

compiler/rustc_type_ir/src/lang_items.rs

@@ -36,6 +36,7 @@ pub enum TraitSolverLangItem {
     Poll,
     Sized,
     TransmuteTrait,
+    TrivialClone,
     Tuple,
     Unpin,
     Unsize,

library/alloc/src/boxed/convert.rs

@@ -1,4 +1,6 @@
 use core::any::Any;
+#[cfg(not(no_global_oom_handling))]
+use core::clone::TrivialClone;
 use core::error::Error;
 use core::mem;
 use core::pin::Pin;
@@ -75,11 +77,13 @@ impl<T: Clone> BoxFromSlice<T> for Box<[T]> {
 }
 
 #[cfg(not(no_global_oom_handling))]
-impl<T: Copy> BoxFromSlice<T> for Box<[T]> {
+impl<T: TrivialClone> BoxFromSlice<T> for Box<[T]> {
     #[inline]
     fn from_slice(slice: &[T]) -> Self {
         let len = slice.len();
         let buf = RawVec::with_capacity(len);
+        // SAFETY: since `T` implements `TrivialClone`, this is sound and
+        // equivalent to the above.
         unsafe {
             ptr::copy_nonoverlapping(slice.as_ptr(), buf.ptr(), len);
             buf.into_box(slice.len()).assume_init()

library/alloc/src/lib.rs

@@ -147,6 +147,7 @@
 #![feature(std_internals)]
 #![feature(str_internals)]
 #![feature(temporary_niche_types)]
+#![feature(trivial_clone)]
 #![feature(trusted_fused)]
 #![feature(trusted_len)]
 #![feature(trusted_random_access)]

library/alloc/src/rc.rs

@@ -243,9 +243,9 @@
 
 use core::any::Any;
 use core::cell::Cell;
-#[cfg(not(no_global_oom_handling))]
-use core::clone::CloneToUninit;
 use core::clone::UseCloned;
+#[cfg(not(no_global_oom_handling))]
+use core::clone::{CloneToUninit, TrivialClone};
 use core::cmp::Ordering;
 use core::hash::{Hash, Hasher};
 use core::intrinsics::abort;
@@ -2150,7 +2150,8 @@ impl<T> Rc<[T]> {
 
     /// Copy elements from slice into newly allocated `Rc<[T]>`
     ///
-    /// Unsafe because the caller must either take ownership or bind `T: Copy`
+    /// Unsafe because the caller must either take ownership, bind `T: Copy` or
+    /// bind `T: TrivialClone`.
     #[cfg(not(no_global_oom_handling))]
     unsafe fn copy_from_slice(v: &[T]) -> Rc<[T]> {
         unsafe {
@@ -2240,9 +2241,11 @@ impl<T: Clone> RcFromSlice<T> for Rc<[T]> {
 }
 
 #[cfg(not(no_global_oom_handling))]
-impl<T: Copy> RcFromSlice<T> for Rc<[T]> {
+impl<T: TrivialClone> RcFromSlice<T> for Rc<[T]> {
     #[inline]
     fn from_slice(v: &[T]) -> Self {
+        // SAFETY: `T` implements `TrivialClone`, so this is sound and equivalent
+        // to the above.
         unsafe { Rc::copy_from_slice(v) }
     }
 }