stop specializing on Copy by joboet · Pull Request #135634 · rust-lang/rust (original) (raw)

fixes #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:

struct SometimesCopy<'a>(&'a Cell);

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

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. 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...