stop specializing on Copy

[joboet]

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

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

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Changes to the code generated for builtin derived traits.

cc @nnethercote

[Mark-Simulacrum]

Going to nominate for libs-api (and libs) since this is both a breaking change (allowed since fixing soundness). I feel like I recall an RFC or some other discussion about us explicitly saying libraries shouldn't do the unsound thing here, but I don't know what that was. https://rust-lang.github.io/rfcs/1521-copy-clone-semantics.html is a bit related but not directly :)

[the8472]

RFC 1521 could be interpreted so. Since it requires that Clone is equivalent to Copy when both are implemented.

Since SometimesCopy implements both (at least sometimes) they must be equivalent. And since cannot tell 'static and non-'static apart they must always be equivalent. Therefore the Clone implementation is wrong.

---

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.

I still don't think this is unsound in itself. So far all demonstrations of unsoundness required some other unsafe code to turn this into a miscompilation. E.g. the WeirdCow in #132442 or the TrustedLen impl in #89948 both require unsafe to exploit this.

Noratrieb also argues that lifetime-conditional Copy currently is unsupported in MIR.

So ISTM that this could be a documentation shortcoming and a compiler/lang issue that such implementations should be prevented but aren't.

That said, I agree that the current situation is brittle.

[scottmcm]

Without saying anything about specialization on Copy, there's definitely been past land discussion of splitting the "memcpyable" part of Clone from the "don't need to write .clone()" part. Something like TrivialClone would probably be what that would need as well, and would -- as you mention in the docs in the PR -- be nice for allowing memcpying of non-Copy types like legacy::Range.

But that gets back to needing, as the8472 said, a way to actually block lifetime-bad implementations before it could be stable.

[the8472]

We discussed this during today's libs-API meeting. We currently are not aware of any safe code that is unsound due to these specializations and there were concerns about performance regressions for user types that manually implement Copy.

So we're leaning towards keeping the implementations as they are and instead improving things in other ways such as adding compiler warnings or improving the Copy documentation or unsafe-code-guidelines.

---

We'd like input from T-types whether they agree with this assessment and if something should be changed on the language side, e.g. by forbidding or at least warning on lifetime-conditional implementations, similar to how Drop impls must have the same bounds as the type it's implemented on.

A compiler-team member has indicated that lifetime-dependent Copy impls are de-facto unsupported.

[BoxyUwU]

Forbidding lifetime dependent copy impls seems like it would be rather breaking (but that's pure speculation, we ought to do a crater run to check if anyone feels strongly we should forbid such impls), though generally I don't feel great about forbidding lifetime dependent copy impls. I also don't think a warning on lifetime dependent copy impls really helps anything for std as warnings cannot be relied upon for soundness and so std's usage of specialization would still be wrong.

In general I would prefer std to not be using specialization in any ways that affect behaviour in any way, it's stably exposing unstable broken parts of the type system in ways that are arguably unsound (allows you to prove trait bounds hold when they do not).

imo what should have happened is that years ago when specialization was found to be unsound all these specializations should have been ripped out regardless of the performance cost and re-added with a PR like this that respects lifetime constraints and treats the unsafe specialization marker attr as something unsafe with invariants to be upheld.

I cant speak for the whole types team but that's atleast my opinion as a types member 🤷‍♀️

---

On a semi-related note, does std still specialize fused iterator stuff in ways that exposes specialization to stable too? I remember that being a thing some years ago but haven't kept up to date with how std is using specialization

[the8472]

On a semi-related note, does std still specialize fused iterator stuff in ways that exposes specialization to stable too?

Yes, but #86765 changed the specialization so that incorrect specializations only result in correctness issues and not soundness ones.

And we have TrusedFused now for cases where it's relevant to soundness.

[lcnr]

The types team discussed this on zulip: https://rust-lang.zulipchat.com/#narrow/channel/326866-t-types.2Fnominated/topic/.23135634.3A.20stop.20specializing.20on.20.60Copy.60

My opinion/summary from there:

• rn specializing on Copy is unsound from a type system pov, even as I don't know of, and can't think of, actual cases whether this results in broken invariants/ub
  • fixing specialization with lifetime dependent impls to be sound won't happen in the near future
  • forbidding lifetime dependent Copy impls is not possible/too much of a breaking change, as they are currently allowed with arbitrary where-bounds

I would like to avoid specializing on Copy. I believe we should land this PR if the approach of having a new trait implemented on derive(Copy) is good enough perf wise (whatever that means)

[bors]

☔ The latest upstream changes (presumably #136448) made this pull request unmergeable. Please resolve the merge conflicts.

[cuviper]

Should we add manual conditional impls for types like Option<T> and [T; N]?
And how about compiler-implemented types like closures and tuples?

I know we're not going to perfectly recover everything that Copy specialization did right, but I think these will be impactful. It's also great that we could go further, like conditional Range<T> and unconditional slice::Iter<'_, T>.

[joboet]

Should we add manual conditional impls for types like Option<T> and [T; N]? And how about compiler-implemented types like closures and tuples?

I know we're not going to perfectly recover everything that Copy specialization did right, but I think these will be impactful. It's also great that we could go further, like conditional Range<T> and unconditional slice::Iter<'_, T>.

Maybe, but let's just try the performance of this first:
@bors try @rust-timer queue

[Mark-Simulacrum]

In general, this seems fine (given FCP completion): r=me with CI fixed.

[joboet]

@bors r=@Mark-Simulacrum rollup=never

[bors]

📌 Commit 16d2b55 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[Zalathar]

@bors ping

[bors]

😪 I'm awake I'm awake

[Zalathar]

@bors retry

[jieyouxu]

(Infra checking bors)
@bors r=Mark-Simulacrum rollup=never

[bors]

💡 This pull request was already approved, no need to approve it again.

[bors]

📌 Commit 16d2b55 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[jieyouxu]

Checking if bors is back
@bors p=1001

[bors]

⌛ Testing commit 16d2b55 with merge 055d0d6...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 055d0d6 to main...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing a7b3715 (parent) -> 055d0d6 (this PR)

Test differences

Show 4834 test diffs

Stage 1

• vec::test_extend_from_within_spec: pass -> [missing] (J1)
• [ui] tests/ui/functions-closures/trivial-clone-closure.rs: [missing] -> pass (J3)

Stage 2

• [ui] tests/ui/functions-closures/trivial-clone-closure.rs: [missing] -> pass (J0)
• vec::test_extend_from_within_spec: pass -> [missing] (J2)

Additionally, 4830 doctest diffs were found. These are ignored, as they are noisy.

Job group index

• J0: aarch64-apple, aarch64-gnu, aarch64-gnu-llvm-20-1, aarch64-msvc-1, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-gnu, x86_64-gnu-debug, x86_64-gnu-gcc, x86_64-gnu-llvm-20, x86_64-gnu-llvm-20-2, x86_64-gnu-llvm-21-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1
• J1: pr-check-2, x86_64-gnu-llvm-20-3, x86_64-gnu-llvm-21-3
• J2: aarch64-apple, aarch64-gnu, aarch64-gnu-llvm-20-1, aarch64-msvc-1, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-gnu, x86_64-gnu-aux, x86_64-gnu-llvm-20-2, x86_64-gnu-llvm-21-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1
• J3: x86_64-gnu-llvm-20-3, x86_64-gnu-llvm-21-3

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 055d0d6aaf937cc11b3d2a5b5725972723b7f3c6 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. pr-check-1: 1475.9s -> 1898.1s (+28.6%)
2. x86_64-gnu-llvm-20-1: 3027.7s -> 3706.2s (+22.4%)
3. armhf-gnu: 4609.2s -> 5482.6s (+19.0%)
4. aarch64-gnu-debug: 3790.8s -> 4433.1s (+16.9%)
5. test-various: 6033.5s -> 7034.8s (+16.6%)
6. x86_64-rust-for-linux: 2713.2s -> 3150.6s (+16.1%)
7. pr-check-2: 2313.2s -> 2668.0s (+15.3%)
8. x86_64-msvc-2: 6251.6s -> 7203.5s (+15.2%)
9. x86_64-gnu-llvm-21-3: 5716.0s -> 6579.0s (+15.1%)
10. x86_64-gnu-llvm-20: 2452.7s -> 2820.7s (+15.0%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (055d0d6): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

• If the regression was expected or you think it can be justified,
  please write a comment with sufficient written justification, and add
  @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
• If you think that you know of a way to resolve the regression, try to create
  a new PR with a fix for the regression.
• If you do not understand the regression or you think that it is just noise,
  you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
  were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.1%, 3.2%]	110
Regressions ❌ (secondary)	0.8%	[0.1%, 1.8%]	50
Improvements ✅ (primary)	-0.9%	[-1.4%, -0.5%]	2
Improvements ✅ (secondary)	-0.4%	[-0.5%, -0.1%]	7
All ❌✅ (primary)	0.7%	[-1.4%, 3.2%]	112

Max RSS (memory usage)

Results (primary 1.0%, secondary -0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.7%	[0.6%, 4.0%]	7
Regressions ❌ (secondary)	1.3%	[1.3%, 1.3%]	1
Improvements ✅ (primary)	-1.4%	[-2.1%, -0.8%]	2
Improvements ✅ (secondary)	-1.4%	[-1.4%, -1.4%]	1
All ❌✅ (primary)	1.0%	[-2.1%, 4.0%]	9

Cycles

Results (primary 3.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.7%	[2.2%, 4.6%]	10
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.7%	[2.2%, 4.6%]	10

Binary size

Results (primary 0.3%, secondary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.0%, 2.1%]	103
Regressions ❌ (secondary)	0.2%	[0.0%, 1.4%]	68
Improvements ✅ (primary)	-0.0%	[-0.0%, -0.0%]	4
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	3
All ❌✅ (primary)	0.3%	[-0.0%, 2.1%]	107

Bootstrap: 476.631s -> 476.7s (0.01%)
Artifact size: 391.32 MiB -> 391.36 MiB (0.01%)