Liballoc IntoIter use as_mut_slice directly

[pickfire]

No description provided.

[rust-highfive]

r? @LukasKalbertodt

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

[cuviper]

Beware, this would undo part of #71148.
cc @bluss @RalfJung

[RalfJung]

@pickfire this code was deliberately written to avoid references, and use raw pointers instead.
What is the reason for reverting parts of #71148?

[pickfire]

@RalfJung I didn't realized it removes part of that. But this is to reduce duplicates, I saw that it could just use the public function directly since it is the same to use &mut x or *mut x, not sure if that's wrong.

[RalfJung]

It is wrong. The old code created *mut [T], the new code instead creates a &mut [T] and then (implicitly) casts that to *mut [T] when passing it to drop_in_place. That is a huge difference: &mut is a safe reference and must be unique (no aliases) and point to initialized data; *mut [T] makes no such promises.

To give a simpler example (too simplistic for Vec but it demonstrates the fundamental difference):

fn okay_function() -> *mut [i32] {
  ptr::slice_from_raw_parts_mut(0 as *mut _, 42)
}

fn very_not_okay_function() -> &mut [i32] {
  slice::from_raw_parts_mut(0 as *mut _, 42)
}

Calling the second function is Undefined Behavior.

[pickfire]

But it is still using ptr::slice_from_raw_parts_mut instead of slice::from_raw_parts_mut, I didn't change that part. I don't get what you said but I also don't understand how it is related to this patch. @RalfJung

[RalfJung]

Yes but you are also creating a reference. When I inline slice::from_raw_parts_mut in my bad example I get:

fn very_not_okay_function() -> &mut [i32] {
  &mut *ptr::slice_from_raw_parts_mut(0 as *mut _, 42)
}

That's your code.

References in Rust are meaningful. Turning a raw pointer to a reference is a big promise you are making to the compiler. In this case, that promise is not justified.

[RalfJung]

Specifically, if the contents of the vector are not properly initialized, your patch violates one of the core rules of Rust: You must not, under any circumstances,

Producing an invalid value, even in private fields and locals. "Producing" a value happens any time a value is assigned to or read from a place, passed to a function/primitive operation or returned from a function/primitive operation. The following values are invalid (at their respective type):
[...]

• A reference or Box that is dangling, unaligned, or points to an invalid value.

[pickfire]

Ah, I think I kinda understand it now, turning a raw pointer to a reference is bad. But I see that both place uses unsafe, and the build still passes, I didn't thought it was bad.

[RalfJung]

"unsafe" doesn't mean "you can do bad things". "unsafe" means "it is your responsibility to make sure that you are not doing bad things".

In safe code, the compiler can ensure you are not doing anything bad. By writing unsafe, you are taking that burden onto yourself. The compiler cannot help you any more. The compiler has to blindly trust that you did not make a mistake. That is also why the build passed.

This is why unsafe is so hard to use, and why we try hard to make sure that people usually do not have to use unsafe. :)

[pickfire]

A reference or Box that is dangling, unaligned, or points to an invalid value.

But we are not pointing to an invalid value here right?

[RalfJung]

It points to the buffer with the Vec content. You don't know what it is in there. Maybe the user put something invalid in there. That was the point of #71148.