Remove useless 'static bounds on Box allocator

[Jules-Bertholet]

#79327 added 'static bounds to the allocator parameter for various Box + Pin APIs to ensure soundness. But it was a bit overzealous, some of the bounds aren't actually needed.

[rustbot]

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

[zetanumbers]

This would be incorrect and violate pin's drop guarantee which in turn can break existing correct code assuming this rule.

[Jules-Bertholet]

@zetanumbers can you show an example about how removing the 'static bounds touched by this PR specifically leads to unsoundness? (This PR doesn't remove all the 'static bounds added by #79327, only a few of them.)

[thomcc]

I'm going to be away for a few months, so I'm rerolling my PRs so that folks don't have to wait for me. Sorry/thanks.

r? libs

[cuviper]

@zetanumbers, can you elaborate on that unsoundness?

Or maybe @TimDiekmann can take a look as the person who first added this?

[zetanumbers]

I've tried to elaborate on unsoundness of this change given my knowledge in this comment: #120092 (comment)

So #79327 was justified because otherwise it violates pin's drop guarantee, which says that memory that holds pointee of a pinned (smart) pointer is allowed to be deallocated only after dropping the pointee, unless pointee is !Unpin. This guarantee to some extend can allow, for example, to send a pointer to a buffer allocated inside of the future's local state to write some data to it. It is also needed for hypothetical scoped async tasks and scoped thread join handles in particular to be able to safely capture references to future's local state (see proposal's zulip thread).

[Amanieu]

In other words: leaking a Box with a non-'static allocator would result in the underlying memory being freed without dropping the pinned object. This violates the pin drop guarantee.

[cuviper]

Then we should capture that information in code comments!

[Jules-Bertholet]

Again, does anyone have a concrete example of how this PR specifically would allow unsoundness? Because as far as I can tell, no such example can be constructed.

[Amanieu]

After spending some time reading through the Unpin docs, I think this PR is actually fine. Unpin only indicates that a type does not have any address-sensitive state, which is always true for Box even if the allocator is not static. This is obvious if you look at &'a T which is Unpin for all lifetimes.

The other 2 impls are just forwarding impls based on the first one, so they are also fine.

@bors r+

[bors]

📌 Commit 1265af0 has been approved by Amanieu

It is now in the queue for this repository.

[bors]

⌛ Testing commit 1265af0 with merge c5f69bd...

[bors]

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing c5f69bd to master...

[rust-timer]

Finished benchmarking commit (c5f69bd): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.5%	[-2.5%, -2.5%]	1
Improvements ✅ (secondary)	-3.0%	[-4.0%, -2.0%]	4
All ❌✅ (primary)	-2.5%	[-2.5%, -2.5%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 649.976s -> 650.169s (0.03%)
Artifact size: 310.93 MiB -> 310.96 MiB (0.01%)