Mapping and collecting an owned Vec to an item with the same size should reuse its allocation

[jhpratt]

#[repr(i8)]
pub enum Foo {
    A,
    B,
    C,
}

pub fn convert(vec: Vec<Foo>) -> Vec<i8> {
    vec.into_iter().map(|foo| foo as i8).collect()
    // unsafe { std::mem::transmute(vec) }
}

As it stands currently (on both stable and nightly), a new Vec is created in memory. As the source and target types are the same size in memory, I see no reason the compiler couldn't just map the value in-place, returning the same allocated ref (but with different semantics).

The commented out line is to indicate what I think should be the case for a trivial case where memory representations are identical. Obviously it wouldn't be quite as simple for most cases.

[leonardo-m]

On the topic:
https://www.joachim-breitner.de/blog/610-Adding_safe_coercions_to_Haskell

http://research.microsoft.com/en-us/um/people/simonpj/papers/ext-f/coercible.pdf

[hanna-kruppe]

I don't expect compiler optimizations can reasonably and reliably address either the transmute case nor the case where some non-trivial transformation happens but memory layout stays the same, but perhaps library code can help. The transmute scenario is probably most reliably addressed by rust-lang/rfcs#2756 plus (plus the results of the Safe Transmute project group (https://github.com/rust-lang/project-safe-transmute) if you want something safe). The other case, user-defined transform-in-place, might be addressed to some extent but special-casing in iter::Map<vec::IntoIter>::collect. This requires specialization at minimum. A much simpler solution would be adding an API to Vec that checks for layout match and then directly transforms in-place. I'm sure that idea has been proposed before, perhaps on discourse?

[nbdd0121]

Vec::map_in_place was unstable before Rust 1.2.0, deprecated in Rust 1.3.0 and removed since.

You might want to use something like https://docs.rs/map_in_place.

[jhpratt]

@nbdd0121 Nice find. That's actually pretty much the exact API that I'd love to have, and I think the performance difference (allocations specifically) show that it would be a sensible addition. Of course, rustc could also accomplish this with specialization, but Vec::map_in_place is both clearer and would provide guarantees. While it would panic back then, similar checks to transmute could take place to prevent compilation outright if the guarantee isn't maintained.

---

Edit: Here's a quick proof of concept that I believe does what I want.

trait MapVecInPlace<T> {
    fn map_in_place<U>(self, f: impl FnMut(T) -> U) -> Vec<U>;
}

impl<T> MapVecInPlace<T> for Vec<T> {
    fn map_in_place<U>(mut self, mut f: impl FnMut(T) -> U) -> Vec<U> {
        assert_eq!(size_of::<T>(), size_of::<U>());
        assert_eq!(align_of::<T>(), align_of::<U>());

        let ptr_range_start = self.as_mut_ptr();
        // Safety: Both pointers are of the same type T, and no wrapping is involved.
        let ptr_range_end = unsafe { ptr_range_start.add(self.len()) };

        let mut ptr = ptr_range_start;
        while ptr != ptr_range_end {
            // Safety: The value is within the range of pointers where memory is initialized.
            let t_value = unsafe { ptr.read() };
            let u_value = f(t_value);
            // Safety: We have a `U` returned from `f`. As we're writing this value, we need to
            // cast this pointer. This is safe because size and alignment have already been
            // checked.
            unsafe { ptr.cast::<U>().write(u_value) };

            // Safety: No wrapping is involved, as we're operating within the original vector.
            ptr = unsafe { ptr.add(1) };
        }

        // Safety: We've transformed all T's to U's. Size and alignment is guaranteed to be the
        // same due to asserts.
        unsafe { transmute(self) }
    }
}

[nbdd0121]

What you have is not safe.

#[repr(u8)]
#[derive(Debug)]
enum Test {
    A = 1,
    B = 2,
}

impl Drop for Test {
    fn drop(&mut self) {}
}

let v = vec![Test::A, Test::B];
let v2 = v.map_in_place(|x| match x {
    Test::A => 0u8,
    Test::B => panic!(),
});

Miri gives:

error: Undefined Behavior: enum value has invalid tag: 0x00

[jhpratt]

Right. You'd want to call drop on the T value, at the least. I'm personally not familiar enough with unsafe to create a rock-solid implementation — my goal was just to create a baseline for others to work from if they want to. How to avoid the behavior you found…I'm not sure.

[the8472]

I have a PR for this, but it regresses compile times and I haven't had the time yet to dig into why. #70793

[jhpratt]

@the8472 Does the now-merged PR resolve this issue? If so, I'll close it.

[the8472]

That it does, https://rust.godbolt.org/z/3WK5M4

[jhpratt]

Thanks for the confirmation! Looks like the move still isn't optimized out (as it is when unsafe is used), but that's an orthogonal issue.