Tracking Issue for arc_unwrap_or_clone

[richard-uk1]

Feature gate: #![feature(arc_unwrap_or_clone)]

This is a tracking issue for arc_unwrap_or_clone. This feature adds methods to Rc/Arc to gain access to the contained value, by unwrapping if there is exactly one reference, or cloning otherwise. It is equivalent to cloning the inner value, but saves the clone operation if the caller has the only reference to the rc'd value.

Public API

impl<T: Clone> Arc<T> {
    pub fn unwrap_or_clone(this: Self) -> T;
}

Steps / History

• Implementation: impl Arc::unwrap_or_clone #91589
• Final comment period (FCP)
• Stabilization PR

Unresolved Questions

• More bikeshedding on the name is possible. Existing discussion on irlo. There seems to be consensus that the current name is good.

[patrick96]

Apologies if this is the wrong place for this comment, feel free to delete.

Is there a technical reason why unwrap_or_clone is an associated function and not a method?

If I have x: Rc<T>, I cannot call x.unwrap_or_clone(), but have to resort to Rc::unwrap_or_clone(x) which feels unwieldy.

[richard-uk1]

@patrick96 it's because Arc/Rc are 'wrapper types' that implement Deref. A lot of the methods are implemented as associated functions so they don't clash with method names on the Deref::Target. This makes it easier to change between different wrapper types like Arc/Box etc without suddenly getting a name clash.

Apologies if I'm using the wrong terminology but that's the gist of it.

[patrick96]

That makes sense. Thanks for the explanation :)

[tbu-]

unwrap_or_clone sounds like a bad name to me: unwrap has a connotation of potentially panicking code in Rust. The other suggested names in #91589 all sound better to me.

(Couldn't comment on the thread in IRLO as it has been closed.)

[richard-uk1]

There is precedence for this pattern: `Option::unwrap_or` for example.

…

On Tue, 13 Sept 2022, 01:03 Tobias Bucher, ***@***.***> wrote: unwrap_or_clone sounds like a bad name to me: unwrap has a connotation of potentially panicking code in Rust. The other suggested names in #91589 <#91589> all sound better to me. (Couldn't comment on the thread in IRLO as it has been closed.) — Reply to this email directly, view it on GitHub <#93610 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKT4XWYWI7GWMGR4SYTBZDV57AFJANCNFSM5NOMB77Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[tbu-]

True, I overlooked it.

[PureWhiteWu]

I think this name is good, and it's easy for me to understand what it means.

I'm really looking forward for this to be fcped and stabilized.

[poliorcetics]

At $work we've been using the exact same implementation as the one used in nightly (https://doc.rust-lang.org/src/alloc/sync.rs.html#1535), it works perfectly well

[davidhalter]

While I like the proposal very much (for it's convenience), I feel like I want to point out something that might be interesting as well in the future and is related:

It's not possible to use Rc::unwrap_or_clone(x) where let x = Rc::from("foo");. In this case we have an Rc<str>, which is ?Sized. However this is a conversion that is also very useful. It would be intersting to go from Rc<str> to Box<str>. The other way is already possible, but this way is currently not possible, however there are sometimes cases where the count is known and converting back to a Box would be very useful.

Consider wanting to remove an element from an Rc<Vec<...>> for example. The current way to do this is to clone the vec, remove an element and make a new Rc, even when an Rc is empty.

I understand that there would need to be something like a try_unwrap_into_box first, but I just wanted to bring it up, so it's clear that there's potentially more things that could "unwrap" from an Rc.

[safinaskar]

Consider wanting to remove an element from an Rc<Vec<...>> for example. The current way to do this is to clone the vec, remove an element and make a new Rc, even when an Rc is empty.

Try this:

let a: Rc<Vec<_>> = ...;
Rc::make_mut(&mut a).pop(); // make_mut is stable

The code above will copy vector if needed. (I didn't test the code)

[davidhalter]

Yes, good point, thanks for bringing it up. However a Vec is unfortunately another layer of indirection. Typically I have something like Rc<[Foo]>.

[ximon18]

Is there something that still needs to be done before this can go to FCP? I just came across the need for this and noticed that the comments seem positive with confirmation that it works at $work for at least one commenter above, so am wondering what if anything is preventing it moving forward?

[ranile]

Pinging @rust-lang/t-libs here for input. There seems to be consensus on stabilizing this. Can anyone start an FCP? I'd be happy to work on a stabilization PR (with some guidance)

[dtolnay]

@rust-lang/libs-api:
@rfcbot fcp merge

• https://doc.rust-lang.org/nightly/std/sync/struct.Arc.html#method.unwrap_or_clone
• https://doc.rust-lang.org/nightly/std/rc/struct.Rc.html#method.unwrap_or_clone

Both of these are already expressible on stable using the following incantation, which is exactly the standard library's implementation too: Rc::try_unwrap(this).unwrap_or_else(|rc| (*rc).clone()).

The rationale for a dedicated method is that "I need a T" is a common enough use case that it's worthwhile to have one easy -> T function prominently available in the type's API doing the thing you most likely want to do in that scenario.

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[safinaskar]

@davidhalter and others: #116113 is related

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[gorentbarak]

I believe that can check off the stabilization?

[gorentbarak]

And update the documentation as this seems to have been stabilized.

[gorentbarak]

I am getting this:

warning: the feature `arc_unwrap_or_clone` has been stable since 1.77.0-nightly and no longer requires an attribute to enable

[Bwallker]

While I like the proposal very much (for it's convenience), I feel like I want to point out something that might be interesting as well in the future and is related:

It's not possible to use Rc::unwrap_or_clone(x) where let x = Rc::from("foo");. In this case we have an Rc<str>, which is ?Sized. However this is a conversion that is also very useful. It would be intersting to go from Rc<str> to Box<str>. The other way is already possible, but this way is currently not possible, however there are sometimes cases where the count is known and converting back to a Box would be very useful.

Consider wanting to remove an element from an Rc<Vec<...>> for example. The current way to do this is to clone the vec, remove an element and make a new Rc, even when an Rc is empty.

I understand that there would need to be something like a try_unwrap_into_box first, but I just wanted to bring it up, so it's clear that there's potentially more things that could "unwrap" from an Rc.

Arc<str> stores the string inline on the heap together with the strong_count and weak_count of the Arc. This means it has a different in-memory representation than Box<str> and thus can't be converted into a Box<str> without copying the contents. A Box<str> to Arc<\str> conversion also copies the string contents into the Arc allocation.

[davidhalter]

@Bwallker I understand that. But moving elements from an Rc to a Vec is different from cloning. The opposite direction is already possible.

This means that you can write let foo: Rc<[usize]> = Rc::from(vec![1,2]);, but not Vec::from(foo).

I understand that moving is different from unwrap_or_clone. This would probably need to be named something like Vec::from_moved_rc_or_clone`.

[Bwallker]

GitHub deleted all my angle brackets. I was specifically talking about converting between string types because there's no difference between moving and cloning characters.