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

Add: From<&[T]> for Rc<[T]> + From<&str> for Rc<str> #1844

Closed
Centril opened this issue Jan 4, 2017 · 9 comments
Closed

Add: From<&[T]> for Rc<[T]> + From<&str> for Rc<str> #1844

Centril opened this issue Jan 4, 2017 · 9 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@Centril
Copy link
Contributor

Centril commented Jan 4, 2017

liballoc already has the following defined for Rc<str>:

impl Rc<str> {
    /// Constructs a new `Rc<str>` from a string slice.
    #[doc(hidden)]
    #[unstable(feature = "rustc_private",
               reason = "for internal use in rustc",
               issue = "0")]
    pub fn __from_str(value: &str) -> Rc<str> {
        unsafe {
            // Allocate enough space for `RcBox<str>`.
            let aligned_len = 2 + (value.len() + size_of::<usize>() - 1) / size_of::<usize>();
            let vec = RawVec::<usize>::with_capacity(aligned_len);
            let ptr = vec.ptr();
            forget(vec);
            // Initialize fields of `RcBox<str>`.
            *ptr.offset(0) = 1; // strong: Cell::new(1)
            *ptr.offset(1) = 1; // weak: Cell::new(1)
            ptr::copy_nonoverlapping(value.as_ptr(), ptr.offset(2) as *mut u8, value.len());
            // Combine the allocation address and the string length into a fat pointer to `RcBox`.
            let rcbox_ptr: *mut RcBox<str> = mem::transmute([ptr as usize, value.len()]);
            assert!(aligned_len * size_of::<usize>() == size_of_val(&*rcbox_ptr));
            Rc { ptr: Shared::new(rcbox_ptr) }
        }
    }
}

but it is gated under the feature rustc_private, which will never be stabilized.

It would be useful to provide From<str> for Rc<[T]> and From<&str> for Rc<str> implementations where the latter has use cases such as string interning (for example using: HashSet<Rc<str>>).

A quick implementation would be:

impl<'a, T> From<&'a [T]> for Rc<[T]> {
    /// Constructs a new `Rc<[T]>` from a shared slice [`&[T]`][slice].
    /// All elements in the slice are copied and the length is exactly that of
    /// the given [slice].
    ///
    /// # Examples
    ///
    /// ```
    /// use std::rc::Rc;
    ///
    /// let arr = [1, 2, 3];
    /// let rc  = Rc::from(arr);
    /// assert_eq!(rc.as_ref(), &arr);   // The elements match.
    /// assert_eq!(rc.len(), arr.len()); // The length is the same.
    /// ```
    ///
    /// Using the [`Into`][Into] trait:
    ///
    /// ```
    /// use std::rc::Rc;
    ///
    /// let arr          = [1, 2, 3];
    /// let rc: Rc<[u8]> = arr.as_ref().into();
    /// assert_eq!(rc.as_ref(), &arr);   // The elements match.
    /// assert_eq!(rc.len(), arr.len()); // The length is the same.
    /// ```
    ///
    /// [Into]: ../../std/convert/trait.Into.html
    /// [slice]: ../../std/primitive.slice.html
    #[inline]
    fn from(slice: &'a [T]) -> Self {
        // Compute space to allocate for `RcBox<[T]>`.
        let vptr   = slice.as_ptr();
        let vlen   = slice.len();
        let susize = size_of::<usize>();
        let aligned_len = 1 + (size_of_val(slice) + susize - 1) / susize;

        unsafe {
            // Allocate enough space for `RcBox<[T]>`.
            let vec = RawVec::<usize>::with_capacity(aligned_len);
            let ptr = vec.ptr();
            forget(vec);

            // Initialize fields of `RcBox<[T]>`.
            ptr::write(ptr,           1); // strong: Cell::new(1)
            ptr::write(ptr.offset(1), 1); // weak:   Cell::new(1)
            ptr::copy_nonoverlapping(vptr, ptr.offset(1) as *mut T, vlen);

            // Combine the allocation address and the string length into a
            // fat pointer to `RcBox`.
            let rcbox_ptr: *mut RcBox<[T]> = mem::transmute((ptr, vlen));
            debug_assert_eq!(aligned_len * susize, size_of_val(&*rcbox_ptr));
            Rc { ptr: Shared::new(rcbox_ptr) }
        }
    }
}

impl<'a> From<&'a str> for Rc<str> {
    /// Constructs a new `Rc<str>` from a [string slice].
    /// The underlying bytes are copied from it.
    ///
    /// # Examples
    ///
    /// ```
    /// use std::rc::Rc;
    ///
    /// let slice = "hello world!";
    /// let rc: Rc<str> = Rc::from(slice);
    /// assert_eq!(rc.as_ref(), slice);    // The elements match.
    /// assert_eq!(rc.len(), slice.len()); // The length is the same.
    /// ```
    ///
    /// Using the [`Into`][Into] trait:
    ///
    /// ```
    /// use std::rc::Rc;
    ///
    /// let slice = "hello world!";
    /// let rc: Rc<str> = slice.into();
    /// assert_eq!(rc.as_ref(), slice);    // The elements match.
    /// assert_eq!(rc.len(), slice.len()); // The length is the same.
    /// ```
    ///
    /// [Into]: ../../std/convert/trait.Into.html
    /// [string slice]: ../../std/primitive.str.html
    fn from(value: &'a str) -> Self {
        // This is safe since the input was valid utf8.
        unsafe { mem::transmute(Rc::<[u8]>::from(value.as_bytes())) }
    }
}
@Centril Centril changed the title Add: From<[T]> for Rc<[T]> + From<&str> for Rc<str> Add: From<&[T]> for Rc<[T]> + From<&str> for Rc<str> Jan 4, 2017
@seanmonstar
Copy link
Contributor

It seems that with the recent conversation about field re-ordering, this could fail if random ordering were implemented, unless Rc were to get repr(C) or some future repr(linear).

@Centril
Copy link
Contributor Author

Centril commented Jan 4, 2017

@seanmonstar Yes, that'd be the case, but then, __from_str would fail as well, no?

@Stebalien
Copy link
Contributor

Non-allocating From<String> and From<Vec<T>> would also be nice.

@Centril
Copy link
Contributor Author

Centril commented Jan 4, 2017

@Stebalien unless I'm mistaken, since the strong: Cell<usize> and weak: : Cell<usize> refcounts are part of the RcBox that gets heap allocated... then the allocated space for String would either have to be reallocated, or you'd have to have separate allocations for the refcounts and for the data, which would add overhead to the rest of Rc. The same should apply to Vec<T>.

@seanmonstar Maybe repr(C) is reasonable here - and should be backwards compatible since fields aren't reordered yet (iirc).

@Stebalien
Copy link
Contributor

Stebalien commented Jan 4, 2017 via email

@strega-nil
Copy link

strega-nil commented Jan 4, 2017

Also do note that T: Copy is required.

edit: okay, I guess T: Clone also works.

@apasel422
Copy link
Contributor

Future allocator trait support may make things like this easier to do safely.

@Centril
Copy link
Contributor Author

Centril commented Jan 5, 2017

@apasel422 but that should be mainly an implementation detail, no? i.e: it doesn't change the API...

@Centril
Copy link
Contributor Author

Centril commented Jan 5, 2017

Closing this now that the RFC has been made.

@Centril Centril closed this as completed Jan 5, 2017
@Centril Centril added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

5 participants