Add a RFC for TryClone trait

[achanda]

Rendered

[Centril]

Nitpick: Prefer inline links instead of footnotes.

[Centril]

Should:

fn try_clone_from(&mut self, source: &Self) -> Result<(), Self::Error> { ... }

be added to mirror Clone?

[sfackler]

What code? Can you give an example of real-world code that would be improved here?

[ExpHP]

To me, it does not look like try_clone is the standard library's name for a clone function that can fail. I mean, the try_clone methods in the standard library are an odd bunch; notice that all of their descriptions are of the form with Creates a new independently owned handle to the underlying _, despite the general sounding name.

If anything, their naming says more to be about the ancestry of these types than anything else.

A number of crates have defined TryClone as a trait and implemented it on standard types. https://github.com/search?l=Rust&q=TryClone&type=Code&utf8=%E2%9C%93

What I see in that link are:

• A trait that entirely ignores the standard library's try_clone functions.
• Three cases (1 2 3) of what appear to me to be premature abstractions. They each wrap precisely one of the standard library's try_clone methods, and it's clear that none of them were ever intended to support any of the other types. AFAICT all of these traits merely exist to allow the structs in these crates to wrap types written by wrappers around the standard library type in client code (who can implement the trait for compatibility).
• ...and that's it.

[Evrey]

This trait should be added in std [...]

You meant core::clone there. I'm not that excited for a second HashMap, i.e. an item that could be in core (or rather alloc in this case) but isn't.

It is not clear under what conditions can cloning a type fail.

• On integer overflow
• On more integer overflow
• On yet even more integer overflow
• On conflicting borrows
• On members failing to clone
• On any other kind of invariant-breaking edge case, where keeping shared data like reference counts or dirty flags is a thing.

Note that those are also candidates for an impl TryClone and that there are probably many more, especially when fallible allocations are a thing.

All in all, I do like the proposal. I probably won't need it any time soon, but I like having it around, just in case.

One more thing, though:

Besides quite some typos, you forgot about Clone::clone_from, so you might want to expand TryClone like this:

pub trait TryClone: Sized {
    type Error;

    fn try_clone(&self) -> Result<Self, Self::Error>;

    #[inline]
    fn try_clone_from(&mut self, src: &Self) -> Result<(), Self::Error> {
        *self = src.try_clone()?
    }
}

[bjorn3]

```rust

For syntax highlighting

[bjorn3]

Oops, someone has already requestes this.

[achanda]

Thanks, everyone. Pushed a new version with all comments addresses.

[sfackler]

This error is only encountered if your process leaks at a minimum 2 billion copies of the Arc, after which you can never clone it again. How is that situation supposed to be "idiomatically handled"?

[Evrey]

Talking about 2 billion copies... An Arc takes 4 bytes on 32-bit architectures plus the data behind that 4 bytes pointer. If, in theory, you could use every single byte of your program to allocate a copy of an Arc, then you would have a maximum reference count of 0x40000000_u32, which is < u32::max_value(). And now reduce that number further, because you also have to fit the actual data, your whole program, and the OS kernel into your 4GiB address space.

As a result, it is absolutely impossible to overflow the reference counters of Arc and Rc, ever, on any CPU architecture. So, in a way, you could optimise alloc instead by not doing overflow checks.

[sfackler]

let foo = Arc::new(0);
loop {
    mem::forget(foo.clone());
}

[Evrey]

Now, that is just evil. ;)

[kornelski]

I see it as useful for wrapping C libraries. Most have functions for copying/duplicating their objects, but they're allowed to fail (NULL is always an option), so a non-panicking Clone sounds like a good fit for these.

[Centril]

Do we need a try_ version for std::borrow::ToOwned?

[burdges]

Just fyi, Cow::into_owned needs to take self by value, so if you push this enough you'd need to return the Cow along with your error, which breaks ?.

TryCow::try_into_owned(self) -> Result<<B as TryToOwned>::Owned,(Self,<B as TryToOwned>::Error)>

[dtolnay]

One important thing was not clear to me from the RFC and the discussion so far -- the text seems to frame TryClone as an improvement over ad hoc try_clone() methods on individual types, but why is this an improvement? What are some concrete uses cases that this enables that are inconvenient or not possible today?

[dtolnay]

Thanks for pursuing this design and for the insightful discussion! I am going to close because I think we will be all right continuing to use individual try_clone() methods on concrete types. There hasn't surfaced a use case that would benefit from having these abstracted into a TryClone trait.