Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider adding Box::uninitialized function #46406

Closed
malbarbo opened this issue Nov 30, 2017 · 5 comments
Closed

Consider adding Box::uninitialized function #46406

malbarbo opened this issue Nov 30, 2017 · 5 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@malbarbo
Copy link
Contributor

The reasons for such a function are the same as for std::mem::uninitialized. I saw somewhere a discussion about deprecating std::mem::uninitialized, but I cannot remember where. If this is the case, how to avoid the cost of initializing a huge structure in the heap that will be initialized again in a ffi function call (without writing c code)?

Also, it would be great to have Box::uninitialized_from_value that take a reference to a value and return a uninitialized boxed value that could store a copy of the original value. This function is useful to implement clone for structures containing unsized types. For example:

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
struct A<T: ?Sized> {
    a: u8,
    b: T,
}

#[derive(Debug, Eq, PartialEq)]
struct X {
    inner: Box<A<[u32]>>,
}

impl Clone for X {
    fn clone(&self) -> X {
        unsafe {
            let mut inner = Box::uninitialized_from_value(&*self.inner);
            inner.a = self.inner.a;
            inner.b.copy_from_slice(&self.inner.b);            
            X { inner: inner }
        }
    }
}

fn main() {
    let x = X {
        inner: Box::new(A {
            a: 10,
            b: [1, 2, 3, 4, 5],
        }),
    };
    let y = x.clone();
    assert_eq!(x, y);
}

If instead of [u32], we had a generic [T] we would had to be very careful in the clone implementation, but it would be possible do to so using the stable compiler. The issue is that without Box::uninitialized_from_value I think it would be impossible to create an efficient implementation for X::clone (generic over the array item) without using std:heap and the nightly compiler.

Here is a possible implementation (I'm not sure it is correct):

#![feature(allocator_api)]

use std::ptr;
use std::heap::{Alloc, Heap, Layout};
use std::mem;

trait Uninitialized<T: ?Sized> {
    unsafe fn uninitialized_from_value(value: &T) -> Self;
}

impl<T: ?Sized> Uninitialized<T> for Box<T> {
    unsafe fn uninitialized_from_value(value: &T) -> Self {
        let layout = Layout::for_value(value);
        let ptr = Heap.alloc(layout).unwrap_or_else(|e| Heap.oom(e));
        // Initialize b with value so b has the right DST extra field if T is ?Sized
        let mut b: Box<T> = mem::transmute(value);
        // Change the pointer to the newly allocated memory
        ptr::write(&mut b as *mut _ as *mut *mut u8, ptr);
        b
    }
}

This code is based on Rc::allocate_for_ptr (which is similar to Arc::allocate_for_ptr). Box::uninitialized_from_value could be used in Rc::allocate_for_ptr, which indicates that there are uses cases for such a function.

@nagisa
Copy link
Member

nagisa commented Nov 30, 2017

We eventually expect to introduce union MaybeUninitialized<T> { d: T, u: () } as a replacement for mem::uninitialized. Box::new(MaybeUninitialized { u: () }) would then be essentially be what you’re proposing.

@malbarbo
Copy link
Contributor Author

malbarbo commented Nov 30, 2017

How would it work for T: ?Sized?. I've read https://internals.rust-lang.org/t/mem-uninitialized-and-trap-representations/4167/38 but nothing is said about unsized types.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Nov 30, 2017

It doesn't work for unsized T, but neither does mem::uninitialized. For that matter, Box::new can't be used for any unsized data, initialized or not. Pointers to unsized values are already very difficult to construct in general and often require special support from the pointer type in question.

That said, unlike some other fat pointer shenanigans, this is already possible to implement externally as you've shown – except the really fishy transmute/ptr::write business. So it'll be possible on stable once allocators are stable, which will hopefully be soon, and thus I don't see much need to add a new API to std. It might have merit as a convenience function but that's harder to argue since this is a relatively niche use case and wildly unsafe even with the convenience API.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Nov 30, 2017

On second thought, getting the metadata for the fat pointer right is pretty tricky. The transmute/ptr::write thing is definitely UB (relies on layout of Box<T> for unsized T), and I don't know how to avoid that in current Rust. A proper solution, but not on the horizon yet, would be something like rust-lang/rfcs#1524 – a way to get the metadata out of the &T and combine it with the freshly allocated data pointer into a new fat pointer which can then be fed to Box::from_raw.

Edit: But to be clear this is just for the general case of unsized box contents. For slices, the metadata is just the length and there's slice::from_raw_parts. Although technically that creates a short-lived reference to invalid data <.<

@frewsxcv frewsxcv added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 5, 2017
@XAMPPRocky XAMPPRocky added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Feb 26, 2018
@Mark-Simulacrum
Copy link
Member

Closed/fixed by #63291.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants