From 10564b739182855d1a7f9ed2fa6be5b77eb13816 Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 11 Jan 2025 21:22:01 +0100 Subject: [PATCH 1/6] (almost) get rid of the unsound `#[rustc_unsafe_specialization_marker]` on `Copy`, introduce `TrivialClone` --- library/alloc/src/boxed/convert.rs | 5 ++- library/alloc/src/lib.rs | 1 + library/alloc/src/rc.rs | 8 +++- library/alloc/src/slice.rs | 6 ++- library/alloc/src/sync.rs | 8 +++- library/alloc/src/vec/mod.rs | 7 +-- library/alloc/src/vec/spec_extend.rs | 3 +- library/core/src/array/mod.rs | 9 ++-- library/core/src/clone.rs | 46 +++++++++++++++++++ library/core/src/clone/uninit.rs | 5 ++- library/core/src/marker.rs | 8 +--- library/core/src/mem/maybe_uninit.rs | 13 +++++- library/core/src/slice/mod.rs | 66 +++++++++++++++++----------- library/core/src/slice/specialize.rs | 9 +++- 14 files changed, 142 insertions(+), 52 deletions(-) diff --git a/library/alloc/src/boxed/convert.rs b/library/alloc/src/boxed/convert.rs index 255cefb1e78fb..52dcf7efde07d 100644 --- a/library/alloc/src/boxed/convert.rs +++ b/library/alloc/src/boxed/convert.rs @@ -1,4 +1,5 @@ use core::any::Any; +use core::clone::TrivialClone; use core::error::Error; use core::mem; use core::pin::Pin; @@ -75,11 +76,13 @@ impl BoxFromSlice for Box<[T]> { } #[cfg(not(no_global_oom_handling))] -impl BoxFromSlice for Box<[T]> { +impl BoxFromSlice 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() diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 1bb0f76106472..746703e8caf78 100644 --- a/library/alloc/src/lib.rs +++ b/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)] diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 09206c2f8b290..6974354dd807e 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -245,6 +245,7 @@ use core::any::Any; use core::cell::Cell; #[cfg(not(no_global_oom_handling))] use core::clone::CloneToUninit; +use core::clone::TrivialClone; use core::cmp::Ordering; use core::hash::{Hash, Hasher}; use core::intrinsics::abort; @@ -2143,7 +2144,8 @@ impl 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 { @@ -2233,9 +2235,11 @@ impl RcFromSlice for Rc<[T]> { } #[cfg(not(no_global_oom_handling))] -impl RcFromSlice for Rc<[T]> { +impl RcFromSlice 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) } } } diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index 1cedead7aa243..cebc9588814b5 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -13,6 +13,7 @@ #![cfg_attr(test, allow(unused_imports, dead_code))] use core::borrow::{Borrow, BorrowMut}; +use core::clone::TrivialClone; #[cfg(not(no_global_oom_handling))] use core::cmp::Ordering::{self, Less}; #[cfg(not(no_global_oom_handling))] @@ -88,6 +89,7 @@ use crate::vec::Vec; #[allow(unreachable_pub)] // cfg(test) pub above pub(crate) mod hack { use core::alloc::Allocator; + use core::clone::TrivialClone; use crate::boxed::Box; use crate::vec::Vec; @@ -156,7 +158,7 @@ pub(crate) mod hack { } #[cfg(not(no_global_oom_handling))] - impl ConvertVec for T { + impl ConvertVec for T { #[inline] fn to_vec(s: &[Self], alloc: A) -> Vec { let mut v = Vec::with_capacity_in(s.len(), alloc); @@ -872,7 +874,7 @@ impl SpecCloneIntoVec for [T] { } #[cfg(not(no_global_oom_handling))] -impl SpecCloneIntoVec for [T] { +impl SpecCloneIntoVec for [T] { fn clone_into(&self, target: &mut Vec) { target.clear(); target.extend_from_slice(self); diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index dba1449347ac0..3b3785017274d 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -11,6 +11,7 @@ use core::any::Any; #[cfg(not(no_global_oom_handling))] use core::clone::CloneToUninit; +use core::clone::TrivialClone; use core::cmp::Ordering; use core::hash::{Hash, Hasher}; use core::intrinsics::abort; @@ -2044,7 +2045,8 @@ impl Arc<[T]> { /// Copy elements from slice into newly allocated `Arc<[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]) -> Arc<[T]> { unsafe { @@ -2136,9 +2138,11 @@ impl ArcFromSlice for Arc<[T]> { } #[cfg(not(no_global_oom_handling))] -impl ArcFromSlice for Arc<[T]> { +impl ArcFromSlice for Arc<[T]> { #[inline] fn from_slice(v: &[T]) -> Self { + // SAFETY: `T` implements `TrivialClone`, so this is sound and equivalent + // to the above. unsafe { Arc::copy_from_slice(v) } } } diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 55cd0569538a8..556eb2fb77c51 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -53,6 +53,7 @@ #![stable(feature = "rust1", since = "1.0.0")] +use core::clone::TrivialClone; #[cfg(not(no_global_oom_handling))] use core::cmp; use core::cmp::Ordering; @@ -3225,7 +3226,7 @@ impl ExtendFromWithinSpec for Vec { } #[cfg(not(no_global_oom_handling))] -impl ExtendFromWithinSpec for Vec { +impl ExtendFromWithinSpec for Vec { unsafe fn spec_extend_from_within(&mut self, src: Range) { let count = src.len(); { @@ -3238,8 +3239,8 @@ impl ExtendFromWithinSpec for Vec { // SAFETY: // - Both pointers are created from unique slice references (`&mut [_]`) // so they are valid and do not overlap. - // - Elements are :Copy so it's OK to copy them, without doing - // anything with the original values + // - Elements implement `TrivialClone` so this is equivalent to calling + // `clone` on every one of them. // - `count` is equal to the len of `source`, so source is valid for // `count` reads // - `.reserve(count)` guarantees that `spare.len() >= count` so spare diff --git a/library/alloc/src/vec/spec_extend.rs b/library/alloc/src/vec/spec_extend.rs index b98db669059f9..f0faabd2b9509 100644 --- a/library/alloc/src/vec/spec_extend.rs +++ b/library/alloc/src/vec/spec_extend.rs @@ -1,3 +1,4 @@ +use core::clone::TrivialClone; use core::iter::TrustedLen; use core::slice::{self}; @@ -53,7 +54,7 @@ where impl<'a, T: 'a, A: Allocator> SpecExtend<&'a T, slice::Iter<'a, T>> for Vec where - T: Copy, + T: TrivialClone, { #[track_caller] fn spec_extend(&mut self, iterator: slice::Iter<'a, T>) { diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 28329bb090845..16d332e5c10d4 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -5,10 +5,10 @@ #![stable(feature = "core_array", since = "1.35.0")] use crate::borrow::{Borrow, BorrowMut}; +use crate::clone::TrivialClone; use crate::cmp::Ordering; use crate::convert::Infallible; use crate::error::Error; -use crate::fmt; use crate::hash::{self, Hash}; use crate::intrinsics::transmute_unchecked; use crate::iter::{UncheckedIterator, repeat_n}; @@ -18,6 +18,7 @@ use crate::ops::{ }; use crate::ptr::{null, null_mut}; use crate::slice::{Iter, IterMut}; +use crate::{fmt, ptr}; mod ascii; mod drain; @@ -437,10 +438,12 @@ impl SpecArrayClone for T { } } -impl SpecArrayClone for T { +impl SpecArrayClone for T { #[inline] fn clone(array: &[T; N]) -> [T; N] { - *array + // SAFETY: `TrivialClone` implies that this is equivalent to calling + // `Clone` on every element. + unsafe { ptr::read(array) } } } diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index 00300328b64c1..f4cf8ad3b90fb 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -176,6 +176,32 @@ pub trait Clone: Sized { } } +/// Indicates that the `Clone` implementation is identical to copying the value. +/// +/// This is used for some optimizations in the standard library, which specializes +/// on this trait to select faster implementations of functions such as +/// [`clone_from_slice`](slice::clone_from_slice). It is automatically implemented +/// when using `#[derive(Clone, Copy)]`. +/// +/// Note that this trait does not imply that the type is `Copy`, because e.g. +/// `core::ops::Range` could soundly implement this trait. +/// +/// # Safety +/// `Clone::clone` must be equivalent to copying the value, otherwise calling functions +/// such as `slice::clone_from_slice` can have undefined behaviour. +#[unstable( + feature = "trivial_clone", + reason = "this isn't part of any API guarantee", + issue = "none" +)] +// SAFETY: +// It is sound to specialize on this because the `clone` implementation cannot be +// lifetime-dependent. Therefore, if `TrivialClone` is implemented for any lifetime, +// its invariant holds whenever `Clone` is implemented, even if the actual +// `TrivialClone` bound would not be satisfied because of lifetime bounds. +#[rustc_unsafe_specialization_marker] +pub unsafe trait TrivialClone: Clone {} + /// Derive macro generating an impl of the trait `Clone`. #[rustc_builtin_macro] #[stable(feature = "builtin_macro_prelude", since = "1.38.0")] @@ -327,6 +353,8 @@ unsafe impl CloneToUninit for crate::bstr::ByteStr { /// are implemented in `traits::SelectionContext::copy_clone_conditions()` /// in `rustc_trait_selection`. mod impls { + use super::TrivialClone; + macro_rules! impl_clone { ($($t:ty)*) => { $( @@ -337,6 +365,9 @@ mod impls { *self } } + + #[unstable(feature = "trivial_clone", issue = "none")] + unsafe impl TrivialClone for $t {} )* } } @@ -356,6 +387,12 @@ mod impls { } } + #[unstable(feature = "trivial_clone", issue = "none")] + unsafe impl TrivialClone for ! {} + + #[unstable(feature = "trivial_clone", issue = "none")] + unsafe impl TrivialClone for () {} + #[stable(feature = "rust1", since = "1.0.0")] impl Clone for *const T { #[inline(always)] @@ -364,6 +401,9 @@ mod impls { } } + #[unstable(feature = "trivial_clone", issue = "none")] + unsafe impl TrivialClone for *const T {} + #[stable(feature = "rust1", since = "1.0.0")] impl Clone for *mut T { #[inline(always)] @@ -372,6 +412,9 @@ mod impls { } } + #[unstable(feature = "trivial_clone", issue = "none")] + unsafe impl TrivialClone for *mut T {} + /// Shared references can be cloned, but mutable references *cannot*! #[stable(feature = "rust1", since = "1.0.0")] impl Clone for &T { @@ -382,6 +425,9 @@ mod impls { } } + #[unstable(feature = "trivial_clone", issue = "none")] + unsafe impl TrivialClone for &T {} + /// Shared references can be cloned, but mutable references *cannot*! #[stable(feature = "rust1", since = "1.0.0")] impl !Clone for &mut T {} diff --git a/library/core/src/clone/uninit.rs b/library/core/src/clone/uninit.rs index 8b738bec796de..8d1185067eb88 100644 --- a/library/core/src/clone/uninit.rs +++ b/library/core/src/clone/uninit.rs @@ -1,3 +1,4 @@ +use super::TrivialClone; use crate::mem::{self, MaybeUninit}; use crate::ptr; @@ -49,9 +50,9 @@ unsafe impl CopySpec for T { } } -// Specialized implementation for types that are [`Copy`], not just [`Clone`], +// Specialized implementation for types that are [`TrivialClone`], not just [`Clone`], // and can therefore be copied bitwise. -unsafe impl CopySpec for T { +unsafe impl CopySpec for T { #[inline] unsafe fn clone_one(src: &Self, dst: *mut Self) { // SAFETY: The safety conditions of clone_to_uninit() are a superset of those of diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 029c8b356d074..72cb04e5a3ccd 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -409,12 +409,8 @@ marker_impls! { /// [impls]: #implementors #[stable(feature = "rust1", since = "1.0.0")] #[lang = "copy"] -// FIXME(matthewjasper) This allows copying a type that doesn't implement -// `Copy` because of unsatisfied lifetime bounds (copying `A<'_>` when only -// `A<'static>: Copy` and `A<'_>: Clone`). -// We have this attribute here for now only because there are quite a few -// existing specializations on `Copy` that already exist in the standard -// library, and there's no way to safely have this behavior right now. +// This is unsound, but required by `hashbrown` +// FIXME(joboet): change `hashbrown` to use `TrivialClone` #[rustc_unsafe_specialization_marker] #[rustc_diagnostic_item = "Copy"] pub trait Copy: Clone { diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index 0d8c3ef906bf0..75ae09cad1159 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -1,4 +1,5 @@ use crate::any::type_name; +use crate::clone::TrivialClone; use crate::mem::ManuallyDrop; use crate::{fmt, intrinsics, ptr, slice}; @@ -272,6 +273,10 @@ impl Clone for MaybeUninit { } } +// SAFETY: the clone implementation is a copy, see above. +#[unstable(feature = "trivial_clone", issue = "none")] +unsafe impl TrivialClone for MaybeUninit where MaybeUninit: Clone {} + #[stable(feature = "maybe_uninit_debug", since = "1.41.0")] impl fmt::Debug for MaybeUninit { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -1613,8 +1618,12 @@ impl SpecFill for [MaybeUninit] { } } -impl SpecFill for [MaybeUninit] { +impl SpecFill for [MaybeUninit] { fn spec_fill(&mut self, value: T) { - self.fill(MaybeUninit::new(value)); + // SAFETY: because `T` is `TrivialClone`, this is equivalent to calling + // `T::clone` for every element. Notably, `TrivialClone` also implies + // that the `clone` implementation will not panic, so we can avoid + // initialization guards and such. + self.fill_with(|| MaybeUninit::new(unsafe { ptr::read(&value) })); } } diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index fe9d7c10db28c..1d1f9e9f0cd92 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -6,6 +6,7 @@ #![stable(feature = "rust1", since = "1.0.0")] +use crate::clone::TrivialClone; use crate::cmp::Ordering::{self, Equal, Greater, Less}; use crate::intrinsics::{exact_div, unchecked_sub}; use crate::mem::{self, SizedTypeProperties}; @@ -3721,30 +3722,8 @@ impl [T] { where T: Copy, { - // The panic code path was put into a cold function to not bloat the - // call site. - #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)] - #[cfg_attr(feature = "panic_immediate_abort", inline)] - #[track_caller] - const fn len_mismatch_fail(dst_len: usize, src_len: usize) -> ! { - const_panic!( - "copy_from_slice: source slice length does not match destination slice length", - "copy_from_slice: source slice length ({src_len}) does not match destination slice length ({dst_len})", - src_len: usize, - dst_len: usize, - ) - } - - if self.len() != src.len() { - len_mismatch_fail(self.len(), src.len()); - } - - // SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was - // checked to have the same length. The slices cannot overlap because - // mutable references are exclusive. - unsafe { - ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len()); - } + // SAFETY: `T` implements `Copy`. + unsafe { copy_from_slice_impl(self, src) } } /// Copies elements from one part of the slice to another part of itself, @@ -4909,6 +4888,38 @@ impl [f64] { } } +/// Copies `src` to `dest`. +/// +/// # Safety +/// `T` must implement one of `Copy` or `TrivialClone`. +#[track_caller] +const unsafe fn copy_from_slice_impl(dest: &mut [T], src: &[T]) { + // The panic code path was put into a cold function to not bloat the + // call site. + #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)] + #[cfg_attr(feature = "panic_immediate_abort", inline)] + #[track_caller] + const fn len_mismatch_fail(dst_len: usize, src_len: usize) -> ! { + const_panic!( + "copy_from_slice: source slice length does not match destination slice length", + "copy_from_slice: source slice length ({src_len}) does not match destination slice length ({dst_len})", + src_len: usize, + dst_len: usize, + ) + } + + if dest.len() != src.len() { + len_mismatch_fail(dest.len(), src.len()); + } + + // SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was + // checked to have the same length. The slices cannot overlap because + // mutable references are exclusive. + unsafe { + ptr::copy_nonoverlapping(src.as_ptr(), dest.as_mut_ptr(), dest.len()); + } +} + trait CloneFromSpec { fn spec_clone_from(&mut self, src: &[T]); } @@ -4933,11 +4944,14 @@ where impl CloneFromSpec for [T] where - T: Copy, + T: TrivialClone, { #[track_caller] fn spec_clone_from(&mut self, src: &[T]) { - self.copy_from_slice(src); + // SAFETY: `T` implements `TrivialClone`. + unsafe { + copy_from_slice_impl(self, src); + } } } diff --git a/library/core/src/slice/specialize.rs b/library/core/src/slice/specialize.rs index 80eb590587f99..5ff529168c041 100644 --- a/library/core/src/slice/specialize.rs +++ b/library/core/src/slice/specialize.rs @@ -1,3 +1,6 @@ +use crate::clone::TrivialClone; +use crate::ptr; + pub(super) trait SpecFill { fn spec_fill(&mut self, value: T); } @@ -14,10 +17,12 @@ impl SpecFill for [T] { } } -impl SpecFill for [T] { +impl SpecFill for [T] { fn spec_fill(&mut self, value: T) { for item in self.iter_mut() { - *item = value; + // SAFETY: `TrivialClone` indicates that this is equivalent to + // calling `Clone::clone` + *item = unsafe { ptr::read(&value) }; } } } From bbb2834b6daf7f8908a8a7cc3b5b093dfc9a419a Mon Sep 17 00:00:00 2001 From: joboet Date: Fri, 17 Jan 2025 16:19:10 +0100 Subject: [PATCH 2/6] add a `TrivialClone` implementation when deriving both `Clone` and `Copy` --- .../src/deriving/bounds.rs | 6 ++++- .../src/deriving/clone.rs | 24 +++++++++++++++++-- .../src/deriving/cmp/eq.rs | 3 ++- .../src/deriving/cmp/ord.rs | 3 ++- .../src/deriving/cmp/partial_eq.rs | 4 +++- .../src/deriving/cmp/partial_ord.rs | 3 ++- .../src/deriving/debug.rs | 3 ++- .../src/deriving/default.rs | 4 ++-- .../src/deriving/generic/mod.rs | 7 ++++-- .../rustc_builtin_macros/src/deriving/hash.rs | 3 ++- compiler/rustc_span/src/symbol.rs | 1 + library/core/src/clone.rs | 2 +- ...duplicate-derive-copy-clone-diagnostics.rs | 1 + ...icate-derive-copy-clone-diagnostics.stderr | 12 +++++++++- tests/ui/deriving/deriving-all-codegen.stdout | 16 +++++++++++++ 15 files changed, 77 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/bounds.rs b/compiler/rustc_builtin_macros/src/deriving/bounds.rs index a98e9c6d1c7fc..ca0b46da98462 100644 --- a/compiler/rustc_builtin_macros/src/deriving/bounds.rs +++ b/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,7 @@ pub(crate) fn expand_deriving_copy( methods: Vec::new(), associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push); @@ -46,6 +47,7 @@ pub(crate) fn expand_deriving_const_param_ty( methods: Vec::new(), associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push); @@ -60,6 +62,7 @@ pub(crate) fn expand_deriving_const_param_ty( methods: Vec::new(), associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push); @@ -83,6 +86,7 @@ pub(crate) fn expand_deriving_unsized_const_param_ty( methods: Vec::new(), associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push); diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs index c3656e8244fe0..06951adef3726 100644 --- a/compiler/rustc_builtin_macros/src/deriving/clone.rs +++ b/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,25 @@ 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), + }; + + trivial_def.expand_ext(cx, mitem, item, push, true); + } + let trait_def = TraitDef { span, path: path_std!(clone::Clone), @@ -87,6 +106,7 @@ pub(crate) fn expand_deriving_clone( }], associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand_ext(cx, mitem, item, push, is_simple) diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs index eca79e4dc4897..046be78e4567c 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs +++ b/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,7 @@ pub(crate) fn expand_deriving_eq( }], associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand_ext(cx, mitem, item, push, true) } diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs index 1ed44c20bc61e..e0c836c7f3378 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs +++ b/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,7 @@ pub(crate) fn expand_deriving_ord( }], associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push) 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 4b93b3414c76b..d9988fc1b55ea 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs +++ b/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,7 @@ pub(crate) fn expand_deriving_partial_eq( methods: Vec::new(), associated_types: Vec::new(), is_const: false, + safety: Safety::Default, }; structural_trait_def.expand(cx, mitem, item, push); @@ -110,6 +111,7 @@ pub(crate) fn expand_deriving_partial_eq( methods, associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push) } 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 7958e037555d5..ef0a81910dcd9 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs +++ b/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,7 @@ pub(crate) fn expand_deriving_partial_ord( methods: vec![partial_cmp_def], associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push) } diff --git a/compiler/rustc_builtin_macros/src/deriving/debug.rs b/compiler/rustc_builtin_macros/src/deriving/debug.rs index 8ab21986e68a0..385e516f83e17 100644 --- a/compiler/rustc_builtin_macros/src/deriving/debug.rs +++ b/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,7 @@ pub(crate) fn expand_deriving_debug( }], associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push) } diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index 1fe567e23f455..6b85836b476e2 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/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,7 @@ pub(crate) fn expand_deriving_default( }], associated_types: Vec::new(), is_const, + safety: Safety::Default, }; trait_def.expand(cx, mitem, item, push) } diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 234ec8582165b..3726f469ef275 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/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 as attr; use rustc_expand::base::{Annotatable, ExtCtxt}; @@ -220,6 +220,9 @@ pub(crate) struct TraitDef<'a> { pub associated_types: Vec<(Ident, Ty)>, pub is_const: bool, + + /// The safety of the `impl`. + pub safety: Safety, } pub(crate) struct MethodDef<'a> { @@ -788,7 +791,7 @@ impl<'a> TraitDef<'a> { 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 }, diff --git a/compiler/rustc_builtin_macros/src/deriving/hash.rs b/compiler/rustc_builtin_macros/src/deriving/hash.rs index 6e6dbe19e4d1a..54f82d5ce96de 100644 --- a/compiler/rustc_builtin_macros/src/deriving/hash.rs +++ b/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,7 @@ pub(crate) fn expand_deriving_hash( }], associated_types: Vec::new(), is_const, + safety: Safety::Default, }; hash_trait_def.expand(cx, mitem, item, push); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index bb59b4c40bd7f..cb82d2ec6dcdf 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -346,6 +346,7 @@ symbols! { ToString, TokenStream, Trait, + TrivialClone, Try, TryCaptureGeneric, TryCapturePrintable, diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index f4cf8ad3b90fb..11bab59a65aea 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -205,7 +205,7 @@ pub unsafe trait TrivialClone: Clone {} /// Derive macro generating an impl of the trait `Clone`. #[rustc_builtin_macro] #[stable(feature = "builtin_macro_prelude", since = "1.38.0")] -#[allow_internal_unstable(core_intrinsics, derive_clone_copy)] +#[allow_internal_unstable(core_intrinsics, derive_clone_copy, trivial_clone)] pub macro Clone($item:item) { /* compiler built-in */ } diff --git a/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.rs b/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.rs index c4fb620fea4f3..bc88a0e131a8e 100644 --- a/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.rs +++ b/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.rs @@ -6,6 +6,7 @@ #[derive(Copy, Clone)] //~^ ERROR conflicting implementations of trait `Clone` for type `E` //~| ERROR conflicting implementations of trait `Copy` for type `E` +//~| ERROR conflicting implementations of trait `TrivialClone` for type `E` enum E {} fn main() {} diff --git a/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.stderr b/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.stderr index f8e1db33f5363..749315748e772 100644 --- a/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.stderr +++ b/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.stderr @@ -8,6 +8,16 @@ LL | #[derive(Copy, Clone)] | = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) +error[E0119]: conflicting implementations of trait `TrivialClone` for type `E` + --> $DIR/duplicate-derive-copy-clone-diagnostics.rs:6:16 + | +LL | #[derive(Copy, Clone)] + | ----- first implementation here +LL | #[derive(Copy, Clone)] + | ^^^^^ conflicting implementation for `E` + | + = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0119]: conflicting implementations of trait `Clone` for type `E` --> $DIR/duplicate-derive-copy-clone-diagnostics.rs:6:16 | @@ -18,6 +28,6 @@ LL | #[derive(Copy, Clone)] | = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0119`. diff --git a/tests/ui/deriving/deriving-all-codegen.stdout b/tests/ui/deriving/deriving-all-codegen.stdout index 6503c87099040..35ea8503be9d7 100644 --- a/tests/ui/deriving/deriving-all-codegen.stdout +++ b/tests/ui/deriving/deriving-all-codegen.stdout @@ -25,6 +25,8 @@ extern crate std; // Empty struct. struct Empty; #[automatically_derived] +unsafe impl ::core::clone::TrivialClone for Empty { } +#[automatically_derived] impl ::core::clone::Clone for Empty { #[inline] fn clone(&self) -> Empty { *self } @@ -85,6 +87,8 @@ struct Point { y: u32, } #[automatically_derived] +unsafe impl ::core::clone::TrivialClone for Point { } +#[automatically_derived] impl ::core::clone::Clone for Point { #[inline] fn clone(&self) -> Point { @@ -170,6 +174,8 @@ struct PackedPoint { y: u32, } #[automatically_derived] +unsafe impl ::core::clone::TrivialClone for PackedPoint { } +#[automatically_derived] impl ::core::clone::Clone for PackedPoint { #[inline] fn clone(&self) -> PackedPoint { @@ -262,6 +268,8 @@ struct Big { b8: u32, } #[automatically_derived] +unsafe impl ::core::clone::TrivialClone for Big { } +#[automatically_derived] impl ::core::clone::Clone for Big { #[inline] fn clone(&self) -> Big { @@ -763,6 +771,8 @@ impl Enum0 { *self } @@ -958,6 +968,8 @@ enum Fieldless { C, } #[automatically_derived] +unsafe impl ::core::clone::TrivialClone for Fieldless { } +#[automatically_derived] impl ::core::clone::Clone for Fieldless { #[inline] fn clone(&self) -> Fieldless { *self } @@ -1040,6 +1052,8 @@ enum Mixed { }, } #[automatically_derived] +unsafe impl ::core::clone::TrivialClone for Mixed { } +#[automatically_derived] impl ::core::clone::Clone for Mixed { #[inline] fn clone(&self) -> Mixed { @@ -1437,6 +1451,8 @@ pub union Union { pub i: i32, } #[automatically_derived] +unsafe impl ::core::clone::TrivialClone for Union { } +#[automatically_derived] impl ::core::clone::Clone for Union { #[inline] fn clone(&self) -> Union { From e40fce56f127970da42c4df0695aae092c40307f Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 11 Feb 2025 11:16:45 +0100 Subject: [PATCH 3/6] alloc: remove test of unsound specialization behaviour --- library/alloc/tests/vec.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index fe1db56414e0c..927be0a9e630f 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -2345,20 +2345,6 @@ fn test_vec_swap() { assert_eq!(n, 0); } -#[test] -fn test_extend_from_within_spec() { - #[derive(Copy)] - struct CopyOnly; - - impl Clone for CopyOnly { - fn clone(&self) -> Self { - panic!("extend_from_within must use specialization on copy"); - } - } - - vec![CopyOnly, CopyOnly].extend_from_within(..); -} - #[test] fn test_extend_from_within_clone() { let mut v = vec![String::from("sssss"), String::from("12334567890"), String::from("c")]; From f2d28fe38a9fd1e2bb0bb7af401cd22ce987c119 Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 11 Feb 2025 11:42:22 +0100 Subject: [PATCH 4/6] make `TrivialClone` a `#[marker]`-trait to keep it from appearing in error messages --- library/core/src/clone.rs | 4 ++++ .../duplicate-derive-copy-clone-diagnostics.rs | 1 - .../duplicate-derive-copy-clone-diagnostics.stderr | 12 +----------- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index 11bab59a65aea..60d4c6a4f04e4 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -200,6 +200,10 @@ pub trait Clone: Sized { // its invariant holds whenever `Clone` is implemented, even if the actual // `TrivialClone` bound would not be satisfied because of lifetime bounds. #[rustc_unsafe_specialization_marker] +// If `#[derive(Clone, Clone, Copy)]` is written, there will be multiple +// implementations of `TrivialClone`. To keep it from appearing in error +// messages, make it a `#[marker]` trait. +#[marker] pub unsafe trait TrivialClone: Clone {} /// Derive macro generating an impl of the trait `Clone`. diff --git a/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.rs b/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.rs index bc88a0e131a8e..c4fb620fea4f3 100644 --- a/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.rs +++ b/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.rs @@ -6,7 +6,6 @@ #[derive(Copy, Clone)] //~^ ERROR conflicting implementations of trait `Clone` for type `E` //~| ERROR conflicting implementations of trait `Copy` for type `E` -//~| ERROR conflicting implementations of trait `TrivialClone` for type `E` enum E {} fn main() {} diff --git a/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.stderr b/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.stderr index 749315748e772..f8e1db33f5363 100644 --- a/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.stderr +++ b/tests/ui/derives/duplicate-derive-copy-clone-diagnostics.stderr @@ -8,16 +8,6 @@ LL | #[derive(Copy, Clone)] | = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) -error[E0119]: conflicting implementations of trait `TrivialClone` for type `E` - --> $DIR/duplicate-derive-copy-clone-diagnostics.rs:6:16 - | -LL | #[derive(Copy, Clone)] - | ----- first implementation here -LL | #[derive(Copy, Clone)] - | ^^^^^ conflicting implementation for `E` - | - = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) - error[E0119]: conflicting implementations of trait `Clone` for type `E` --> $DIR/duplicate-derive-copy-clone-diagnostics.rs:6:16 | @@ -28,6 +18,6 @@ LL | #[derive(Copy, Clone)] | = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0119`. From 80e881f0d21c76fd3ad60a933477674c682557f7 Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 11 Feb 2025 13:01:33 +0100 Subject: [PATCH 5/6] fix build of `alloc` with `no_global_oom_handling` --- library/alloc/src/boxed/convert.rs | 1 + library/alloc/src/rc.rs | 3 +-- library/alloc/src/slice.rs | 2 ++ library/alloc/src/sync.rs | 1 + library/alloc/src/vec/mod.rs | 1 + 5 files changed, 6 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/boxed/convert.rs b/library/alloc/src/boxed/convert.rs index 52dcf7efde07d..b0d9489e587b1 100644 --- a/library/alloc/src/boxed/convert.rs +++ b/library/alloc/src/boxed/convert.rs @@ -1,4 +1,5 @@ use core::any::Any; +#[cfg(not(no_global_oom_handling))] use core::clone::TrivialClone; use core::error::Error; use core::mem; diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 6974354dd807e..4864f59e38138 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -244,8 +244,7 @@ use core::any::Any; use core::cell::Cell; #[cfg(not(no_global_oom_handling))] -use core::clone::CloneToUninit; -use core::clone::TrivialClone; +use core::clone::{CloneToUninit, TrivialClone}; use core::cmp::Ordering; use core::hash::{Hash, Hasher}; use core::intrinsics::abort; diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index cebc9588814b5..8fa9083bbb656 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -13,6 +13,7 @@ #![cfg_attr(test, allow(unused_imports, dead_code))] use core::borrow::{Borrow, BorrowMut}; +#[cfg(not(no_global_oom_handling))] use core::clone::TrivialClone; #[cfg(not(no_global_oom_handling))] use core::cmp::Ordering::{self, Less}; @@ -89,6 +90,7 @@ use crate::vec::Vec; #[allow(unreachable_pub)] // cfg(test) pub above pub(crate) mod hack { use core::alloc::Allocator; + #[cfg(not(no_global_oom_handling))] use core::clone::TrivialClone; use crate::boxed::Box; diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 3b3785017274d..8897698fc4427 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -11,6 +11,7 @@ use core::any::Any; #[cfg(not(no_global_oom_handling))] use core::clone::CloneToUninit; +#[cfg(not(no_global_oom_handling))] use core::clone::TrivialClone; use core::cmp::Ordering; use core::hash::{Hash, Hasher}; diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 556eb2fb77c51..4b95df426fcb4 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -53,6 +53,7 @@ #![stable(feature = "rust1", since = "1.0.0")] +#[cfg(not(no_global_oom_handling))] use core::clone::TrivialClone; #[cfg(not(no_global_oom_handling))] use core::cmp; From d31fbe3055ba2a001bee4143aeb6d4400ded4b70 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 12 Feb 2025 14:23:53 +0100 Subject: [PATCH 6/6] core: implement `TrivialClone` for arrays and `Option` --- library/core/src/array/mod.rs | 3 +++ library/core/src/option.rs | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 16d332e5c10d4..d0914848853a4 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -427,6 +427,9 @@ impl Clone for [T; N] { } } +#[unstable(feature = "trivial_clone", issue = "none")] +unsafe impl TrivialClone for [T; N] {} + trait SpecArrayClone: Clone { fn clone(array: &[Self; N]) -> [Self; N]; } diff --git a/library/core/src/option.rs b/library/core/src/option.rs index a9f06b92ad5dd..3c90735570617 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -556,6 +556,7 @@ #![stable(feature = "rust1", since = "1.0.0")] +use crate::clone::TrivialClone; use crate::iter::{self, FusedIterator, TrustedLen}; use crate::ops::{self, ControlFlow, Deref, DerefMut}; use crate::panicking::{panic, panic_display}; @@ -2050,6 +2051,9 @@ where } } +#[unstable(feature = "trivial_clone", issue = "none")] +unsafe impl TrivialClone for Option {} + #[stable(feature = "rust1", since = "1.0.0")] impl Default for Option { /// Returns [`None`][Option::None].