Open scottmcm opened 3 years ago
I'm happy to put some work into fixing this, if people can suggest what their preferred approach would be.
@scottmcm in this example, could you use drop_in_place
?
@jyn514 But that would drop it -- what I want here is more like forget_in_place
.
I may have opened a duplicate issue for the same issue? https://github.com/rust-lang/rust/issues/79754
Triage: looks like this is still happening on nightly 2024-04-05.
It's great that
ManuallyDrop
helps people do the right thing by pre-leaking things they plan to move elsewhere, helping to avoid double-free issues.But unfortunately
ManuallyDrop::new
gets codegened as copying the whole thing into a local variable, and LLVM frequently cannot remove it. This is particularly frustrating for large types where the ABI passes them by pointer, as they get copied from that pointer to the stack (which, in addition to the copy, means more stack manipulation and__rust_probestack
call than necessary). Demo: https://rust.godbolt.org/z/Y8o7MGNote in particular the following (generated with nightly 2020-12-07 in the godbolt link):
There's no good reason for either of those locals, and this is comparatively easy mode -- panic=abort and the monomorphization is only using
Copy
types that definitely don't need dropping.At the ABI level this is
fn demo(*mut [i64; 400], *const [u32; 400])
, so there ought to be a way to write this function in Rust such that it would produce the obvious no-stack-needed implementation ofout[i] = static_cast<_>(in[i]);
. But I can't find one.Undeveloped musings on possible fixes:
ManuallyDrop
as its own type in codegen (it's already a lang item anyway)Edit: FWIW,
-Z mir-opt-level=3 -Z unsound-mir-opts=yes
doesn't fix this either (as of 2020-12-12)I started looking into this as part of figuring out what was happening in https://github.com/rust-lang/rust/pull/75571
Finally decided to open this after this CI failure (which I'd made sure worked locally), https://github.com/rust-lang/rust/runs/1532668348
MaybeUninit
has similar issues, but that might be covered by https://github.com/rust-lang/rust/issues/61011