rust-lang / libs-team

The home of the library team
Apache License 2.0
127 stars 19 forks source link

ACP: Add `From<Rc<str>> for String` and analogously with `Arc` and other unsized types #217

Closed Kixunil closed 1 year ago

Kixunil commented 1 year ago

Proposal

Problem statement

From<String> for Rc<str> already exists but the converse doesn't despite it making sense: it's just the change of layout of a type fundamentally representing the same thing - a string. It looks more like this was forgotten rather than there being a different reason for it.

Motivation, use-cases

A typical example where this is useful is a generic function that takes a string that conditionally needs to be owned:

fn foo(s: impl AsRef<str> + Into<String>) {
    if some_condition() {
        consume_owned_string(s.into());
    } else {
        use_borrowed_str(s.as_ref());
    }

This bound is the most optimal because if the caller has String (or Cow<'_, str>) and doesn't need to use it afterwards passing it can eliminate allocation. However if the caller doesn't have String it can still avoid allocation in the other branch. Neither impl AsRef<str> alone, nor &str nor String have this property.

This works fine for &str, String, Cow<'_, str> but is a little bit annoying for Rc<str> and Arc<str> (and their references) because the invocation needs to be foo(&*bar) instead of simple foo(bar).

Admittedly this is very small difference but still it does look strange (inconsistent) that these impls are not present.

Solution sketches

Implement the traits by simply allocating a new string and copying the contents into it. The reverse conversion works similarly. Possibly also implement the traits for references.

Links and related work

N/A

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

ChayimFriedman2 commented 1 year ago

From<String> for Rc<str> could not be implemented manually without unsafe code, but the opposite is as simple as (*rc).to_owned().

Amanieu commented 1 year ago

We discussed this in the libs meeting today and we are happy to add these so that Arc and Rc achieve parity with Box which already has this impl.