Comments: Learn Unsafe Rust from My Mistakes

[geo-ant]

Comments for this article go here :)

[tbu-]

I went through a similar journey when I first started using Rust. In fact, I contributed the original map_in_place to the standard library (rust-lang/rust#16853). Back then, it was a lot easier to add methods to the standard library. ^^

I'm happy to hear that this function is now unnecessary as it now works for somewhat arbitrary iterator chains, as proposed by @alexcrichton on the original PR: rust-lang/rust#15302 (comment).

The optimization you allude to happens in the following file: library/alloc/src/vec/in_place_collect.rs. It took me a couple of minutes to find. :) It was apparently added in rust-lang/rust#70793.

Thanks for your interesting blog post. :)

[geo-ant]

@tbu- Oh cool, it's always nice to hear that other devs shared some of the struggle. Thanks for taking the time to find the sources and provide more context.

[GoldsteinE]

Hi!

I think this example is misleading:

let mut x = 10;
let p1: *mut i32 = &mut x;
let p2: *mut i32 = &mut x;
unsafe {
  // ...
}

The text around it seems to imply that we created two usable pointers, but p1 is actually invalidated by the second mutable borrow and isn’t usable under Stacked Borrows (which can be easily validated with Miri). I think it’s valid under Tree Borrows, but it’s not even stable in Miri, so it’s far from being a commonly accepted memory model for Rust.

[geo-ant]

Hi!

I think this example is misleading:

Hey @GoldsteinE , good point. I should just be able to replace the second line by let p2 = p1 and still have a valid p1, since pointers are clone, right?

[GoldsteinE]

Yeah, that should work (the relevant part is that they’re Copy, not Clone tho).

[geo-ant]

Yeah, that should work (the relevant part is that they’re Copy, not Clone tho).

Oh yeah, sorry that's what I meant to say...

[MalbaCato]

Small nitpic:

As leaking destructors isn't unsound in rust, the "first" version of the function is sound (or at least panic safe). This can be shown by writing a semantically equivalent function using safe rust only, by forcing everything to happen inside ManuallyDrops:

fn safe_map_in_place<T, U, F>(v: Vec<T>, f: F) -> Vec<U>
where
    F: Fn(T) -> U,
{
    let mut v = ManuallyDrop::new(v.into_iter().map(f));
    let mut u = ManuallyDrop::new(Vec::new());
    u.extend(&mut *v);
    ManuallyDrop::into_inner(u)
}

This behaviour is certainly incorrect due to being surprising and would be easy to misuse in the presence of other unsafe code that relied on destructors being ran, but clearly safe while unwinding.

Miri refuses to accept this wasteful attitude towards owned recourses, but with a little bit of trickery (or turning off memory leak tracking), it too sees that nothing bad is actually going on.

[geo-ant]

@MalbaCato hey thanks for the comment, I have questions. First if all I think you're right that soundness does not guarantee that destructors are run. I should have stated that more clearly.

The other thing is that I am staring at your example (which is beautiful btw), but I don't understand why it would guarantee in place mapping? Meaning why would vector u reuse the memory allocated for vector v. Not saying that isn't the case, just saying I don't understand what about the code would guarantee it. Could you explain it a bit more?