Rust: Tracking issue for `From<&[T]> for Rc` & co

Created on 13 Mar 2017  Â·  16Comments  Â·  Source: rust-lang/rust

B-RFC-approved C-tracking-issue T-libs

Most helpful comment

@alexbool Sure, I just need to get started with and learn how to contribute to the compiler / stdlib. I'll ask for some help on #rust-internals.

All 16 comments

Anyone willing to implement this?
@Centril ? :)

@alexbool Sure, I just need to get started with and learn how to contribute to the compiler / stdlib. I'll ask for some help on #rust-internals.

I see it's been quiet here for a couple of months, so I thought I would try implementing this myself and I ran into an issue in the process:

Arc/Rc live in alloc and Vec lives in collections, which depends on alloc. Attempting to implement From<Vec<T>> for Arc<[T]>/Rc<[T]> as the RFC states requires introducing a circular dependency on collections for alloc.

I'm unsure how to proceed. A few options are immediately apparent to me:

  • Move Arc/Rc into a separate crate, which may then have collections as a dependency.
  • Instead implement Into<Arc<[T]>> (and Rc<[T]>) for Vec<T>.
  • Forego implementing From<Vec<T>> altogether. It would still be possible to perform the conversion indirectly, via Vec<T> -> Box<[T]> -> Arc/Rc<[T]>.

@murarth Apologies for my tardiness.
A fourth option is, and which I prefer:

  • Move Vec into alloc

This should be backwards compatible since collections can simply import Vec from alloc.

Move Vec into alloc

In that case what’s the point of having separating alloc and collections crates?

@SimonSapin It can be argued that Vec is more fundamental than the rest of collections, but there is RawVec that already does the "fundamental" part.

Regarding the other choices,

  • Moving Arc / Rc into a separate crate breaks backwards compatibility - but since alloc is experimental, I guess that's fine.
  • Into<Arc> ... is reasonable if no other choice is viable.
  • Forego... don't do this... Into<Arc> is a better choice.

Actually... I change my mind... Moving Arc / Rc into a crate named shared would be a great idea.

Or, move all of alloc and collections into a single crate (to be named like either of them or something else…)

@SimonSapin Hmm... Wouldn't some developers for the embedded domain want the remaining stuff in alloc but perhaps not collections? Are there no downsides to merging them into one crate?

@japaric ^

I'm leaning toward the first option (spinning Arc and Rc out into a separate crate) because it's the only way to fully implement the RFC. I don't think this is something that would typically be called a "breaking change" as the alloc crate is unstable.

@Centril I'm probably not the best person to ask about this because although I do embedded Rust development I try to avoid using dynamic memory allocation as much as possible.

I don't think this is something that would typically be called a "breaking change" as the alloc crate is unstable.

This is the other reason why I avoid any no_std crate that's not core.

As for the actual question: I wouldn't mind if collections was merged into alloc and see no downside in merging them other than increased compilation time if you only wanted the stuff that's currently in the alloc crate. (Personally I'm more of a fan of having more crates rather than less.)

One thing that could potentially be useful in the future is adding implementations to create Rcs of CStr, OsStr, and Path, like I did for Box (see #40380). I can in particular see Rc<Path> being useful, although in general it'd be nice to have feature parity between Box and Rc and Arc.

Can we just implement the &[T] -> Rc<[T]> conversion for now? It's pretty useful, and the functionality is just missing right now...

It’s already implemented. It needs to be exposed in a public API. https://github.com/rust-lang/rust/blob/1.18.0/src/liballoc/rc.rs#L402-L457

Although it’s from Box<[T]>. For &[T] we’d also need a T: Clone bound. Or should it be generic over Iterator<Item=T> + ExactSizeIterator?

Huzzah, the implementation PR #42565 has been merged!

The From impls are insta-stable, so should this be closed or wait until they reach the next stable release?

Indeed this can be stable now, thanks again for sticking with this @murarth!

Was this page helpful?
0 / 5 - 0 ratings