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

Box::new_from_ref for making a Box<T> from a &T where T: CloneToUninit + ?Sized (and Rc and Arc) #483

Open
zachs18 opened this issue Nov 15, 2024 · 0 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@zachs18
Copy link

zachs18 commented Nov 15, 2024

Proposal

Problem statement

Given a t: &T, where T: CloneToUninit, it would be nice to be able to make a heap-allocated container containing a clone of t, such as Box<T> or Rc<T>. Currently, such conversions are only implemented ad-hoc for specific types, e.g. impl From<&str> for Box<str>, impl<T: Clone> From<&[T]> for Arc<[T]>, etc.

Motivating examples or use cases

Consider a custom DST MyDst. It is possible to write a From<&MyDst> for Box<MyDst> using manual allocation and unsafe code to clone fields into the new allocation, but this manual allocation would not be necessary if the standard library provided this functionality, and the user would only need to write the unsafe code to implement CloneToUninit for MyDst to clone the fields into the new allocation.

Now that CloneToUninit is dyn-compatible, this also applies to dyn MyTrait where trait MyTrait: CloneToUninit.

Additionally, since Rc/Arc's heap layout is not specified, it is only possible to write a From<&MyDst> for Rc<MyDst> that makes a Box<MyDst> and goes through From<Box<T>> for Rc<T>, making an unnecessary intermediate allocation which could be avoided if a new Rc<T> could be cloned directly from a &T

Solution sketch

Add a new function new_from_ref to Box<T>, Rc<T>, Arc<T> where T: ?Sized + CloneToUninit, and try_new_from_ref, new_from_ref_in, and try_new_from_ref_in additionally under feature(allocator_api).

impl<T: ?Sized + CloneToUninit> Box<T> {
  #[unstable(feature = "new_from_ref")]
  pub fn new_from_ref(src: &T) -> Box<T> { Box::new_from_ref_in(Global) }
  #[unstable(feature = "new_from_ref")]
  #[unstable(feature = "allocator_api")]
  pub fn try_new_from_ref(src: &T) -> Result<Box<T>, AllocError>  { Box::try_new_from_ref_in(Global) }
}
// same for Arc<T>, Rc<T>

// with `feature(allocator_api)`
impl<T: ?Sized + CloneToUninit, A: Allocator> Box<T, A> {
  #[unstable(feature = "new_from_ref")]
  #[unstable(feature = "allocator_api")]
  pub fn new_from_ref_in(src: &T, alloc: A) -> Box<T, A>;
  #[unstable(feature = "new_from_ref")]
  #[unstable(feature = "allocator_api")]
  pub fn try_new_from_ref_in(src: &T) -> Result<Box<T, A>, AllocError;
}
// same for Arc<T, A>, Rc<T, A>

(WIP branch (Box and partial Rc only): https://github.com/zachs18/rust/tree/new_from_ref)

Alternate bikeshed names: clone_from_ref, cloned_from_ref, new_cloned.

Alternatives

The existing From<&str> for Box<str> (etc) impls in the stdlib could be replaced by general impl<T: ?Sized + CloneToUninit> From<&T> for Box<T>/Rc<T>/Arc<T> impls. Unfortunately, this would be a breaking change, as downstream crates can currently implement From<&LocalType> for Box<LocalType>/Rc/Arc which a new stdlib impl<T: ?Sized + CloneToUninit> From<&T> for Box<T> would conflict with when LocalType: Clone. Also, it would conflict with the existing impl<'a, E: Error + 'a> From<E> for Box<dyn Error + 'a>, where E = &(dyn Error + 'a) (since alloc cannot know that core won't add an impl CloneToUninit for dyn Error + '_).

Links and related work

  • The CloneToUninit trait (tracking issue)
  • Rc/Arc::make_mut use CloneToUninit to clone an existing T already inside an Rc/Arc into a new, unique, heap allocation.
  • It is planned for impl<T: Clone> Clone for Box<T> to be generalized to T: ?Sized + CloneToUninit.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@zachs18 zachs18 added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Nov 15, 2024
@zachs18 zachs18 changed the title Box::new_from_ref for making a Box<T> from a &T where T: CloneToUninit + ?Sized Box::new_from_ref for making a Box<T> from a &T where T: CloneToUninit + ?Sized (and Rc and Arc) Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

1 participant