Documentation for mem::forget implies that it may invalidate raw pointers to heap allocations

[pnkfelix]

(Transferred from rust-lang/rust#79320 )

Someone pointed out that the documentation for mem::forget may be a bit too broad, in that it could be interpreted as saying that heap allocations owned by a value passed to mem::forget may be invalidated.

I think the UCG WG is currently the best group of people to work out precisely what the rules are here, and what the best mental model is, (if you haven't already, that is).

I'm closing rust-lang/rust#79320 because I don't think the work will be tracked usefully in that context.

[RalfJung]

Actually, this code is unsound under Stacked Borrows:

let s = Box::new(32);
let s_ptr = s.as_ptr();
std::mem::forget(s);
let lowercase_e = unsafe { *s_ptr };

That is because passing s (of type Box) to mem::forget (which is just a regular function) re-asserts its uniqueness, thus invalidating any other pointers to that memory.

The example in rust-lang/rust#79320 used String, which does not currently assert uniqueness the way Box does, but I am not sure if that is something we want to guarantee.

The docs recommend using ManuallyDrop (among other factors) for this reason.

[Lokathor]

That's a lot more to do with Box being an unsafe code hazard than anything else.

Wouldn't using ManuallyDrop::new have the same problem of forget? You're passing the value to a function either way.

[RalfJung]

The mode of use is different though:

let s = Box::new(32);
let s = ManuallyDrop::new(s);
let s_ptr = s.as_ptr();
let lowercase_e = unsafe { *s_ptr };

This pattern works fine, and the standard lib uses it quite a bit.

[Lokathor]

Ah, the manually drop moves before the pointer getting, right then.

[saethlin]

The example in rust-lang/rust#79320 used String, which does not currently assert uniqueness the way Box does, but I am not sure if that is something we want to guarantee.

I think that adding Unique to Vec has dubious optimization value and will probably invalidate a lot of code. I'm happy to gather some data on this if someone can implement the change in Miri or offer guidance on how I could do that.

Separately, is there any reason to keep mem::forget? It seems to me like it's just an aliasing footgun and it can always be replaced with ManuallyDrop.

[Lokathor]

I've certainly used forget to drop a thing immediately, which ManuallyDrop doesn't do as ergonomically. And it would be a breaking change to remove, and there's not much obvious value in even deprecating it because you make a lot of code less obvious.

[saethlin]

Isn't the whole point of forget that the value isn't dropped?

[Lokathor]

Oh wait, I confused it with drop. Ignore everything I just said.

[RalfJung]

I think that adding Unique to Vec has dubious optimization value and will probably invalidate a lot of code. I'm happy to gather some data on this if someone can implement the change in Miri or offer guidance on how I could do that.

That requires a big redesign of SB I think -- but one that I want to do anyway since it also helps with some other issues.