[Idea] Improving the ergonomics when using Rc

[dan_t]

This example shows quite nicely how an auto clone in this case would make the reasoning about this code harder. It might not be a lot harder, but this is the problem with such changes, that they themselves don’t seem like a big issue, but all this little changes together might be.

Auto clone might not seem to be that much different to other auto operations like Deref, and this is true in itself, but that doesn’t mean that more auto operations wouldn’t make the language harder to reason about. Just because two things seem similar doesn’t mean that the sum of both has still similar effects.

I’m also quite suspicious about a community consensus about how certain features should be used. My experience just tells me, if a feature can be used wrongly or dangerously, then it will be the case.

At the end, how often do you really need to call ‘clone’ on a ‘Rc’? In most cases you’ve very few places where the ‘Rc’ is shared and if you’re working with the value of the ‘Rc’, calling functions with it, you most likely will just use a ‘&T’.

[vorner]

At the end, how often do you really need to call ‘clone’ on a ‘Rc’? In most cases you’ve very few places where the ‘Rc’ is shared and if you’re working with the value of the ‘Rc’, calling functions with it, you most likely will just use a ‘&T’.

To be clear, I myself do find cloning reference counted pointers annoying (in my case it is Arc, not Rc, but I think that’s about the same thing).

The current one I’m facing is about hyper (or, actually, around a middleware that starts hyper servers based on configuration). Let’s say I have some global data I want to share to each request handler, I have code something like this (forgive any possible typos and such):

let data_srv = Arc::clone(&data);
Server::bind("127.0.0.1:8000")
  .serve(move || {
      let data = Arc::clone(&data_srv);
      server_fn(move |req| {
          req_handler(req, &data)
      })
   })

With the middleware thing, I actually get additional closure layer and additional Arc::clone(&data) layer.

This is not a big deal and when I have to write the code, I just write it (just adding clones until the compiler stops complaining).

However, I feel like this code is awkward. I would prefer to have the code more streamlined. I do acknowledge there’s a think that could be improved.

On the other hand, while the code looks hairy and is awkward to write, reading it needs minimal mind-power to reason about, the clumsiness is there in plain sight.

I prefer to err on the side of code that is ugly to look at, but doesn’t hide the ugliness. It does what it says on the box. If I want to err on the side of ergonomics and ease of use at the cost of clarity what happens and bad failure cases, I’m OK with picking a different language. There’s a big selection of those.

[cksac]

Throughout the discussion, I fully understand the concerns of introduce autoclone mechanism (through it is conservative one, only Rc and Arc) into the language. I realised that explicitness weight differently among Rust users. Therefore, I start thinking will a crate level autoclone feature flag do the best for both parties, which crate owner require to opt-in autoclone (similar handling like edition).

#![feature(allow_autoclone)] and introduce AutoClone trait.

AutoClone can either be

1. a marker trait like Copy, only Rc, Arc and their Weak form implement it. User type can only derive when all fields implement AutoClone
2. a normal trait like Deref, arbitrary user type can implement it.

Personally, I will opt for 2) as the feature is limited within crate.

[dan_t]

That’s IMHO even worse than having autoclone for Rc all the time. Now you haven’t only to reason about the types at hand but also about how features change the behavior of types.

[dhm]

The ::hyper closures example reminds me of the ‘sharing data with ::std::thread::spawn needs Arc and cloning’, vs ‘sharing data with ::crossbeam scoped threads works with plain shared references’.

I thus wonder if the problem in your example does not come from a “poor” (non-scoped threads) API (note: I don’t know ::hyper and its technical constraints).

As @josh pointed out:

Rc is not a beginner tool to bypass thinking about borrow checking, it’s a tool for use cases when you absolutely need it and can’t structure your code to avoid it

More generally, if using (A)Rc became too ergonomic, then (some) people would just circumvent Rust’s ownership-&-borrow-checking (its whole point and strength) by abusing them.

[skade]

More generally, if using (A)Rc became too ergonomic, then (some) people would just circumvent Rust’s ownership-&-borrow-checking (its whole point and strength) by abusing them.

I don’t find this argument very strong and potentially hazardous. Rc<T> will always be a visibly heavyweight type and have ergonomics implications (e.g. only immutable sharing). I also see patterns were people avoid Rc/Arc for the reason that refactoring to them is too hard and too much of an effort.

Appeals to “making things too easy for people” always strike me as odd. Bad usability for the sake of keeping people away from things puts no trust in peoples ability to choose their approaches. Community practice is the key here and it evolves.

There was a time in Rust (around 1.0) where everyone wanted to solve things using huge borrowing constructs, while nowadays, ownership tricks are beginning to get more popular. This is normal and not steered by the project team making things harder or easier, but by how the system is constructed.

Also, there’s a not-so-small group of people that does feel like Rc and Arc are generally underused in many systems.

[H2CO3]

I don’t find this argument very strong and potentially hazardous.

Appeals to “making things too easy for people” always strike me as odd. Bad usability for the sake of keeping people away from things puts no trust in peoples ability to choose their approaches.

The exact same argument can be made about the opposite point. For instance, I don’t find the “it’s so annoying to write rc.clone() that we should make it implicit and/or add it to the core language” argument strong at all, at least not strong enough to warrant a language change in any case.

Furthermore, the argument is not that Rc “should have bad usability”. Rather, it is that one of the things it does (cloning) shouldn’t be actively encouraged to be treated as insignificant and become invisible, especially not by means that are known to have caused countless problems and correctness bugs in the history of programming language design.

I don’t believe anyone who is against this proposal wants to punish fellow programmers for using Rc. I personally think that it’s one of the best and most important ideas in the art of memory management. I have written lots of code (heavy on graph manipulation) using Rc all the time, and I wouldn’t discourage anyone from using it should they deem it a good solution. I would just like to be able to easily read that code please. And typing a single method call is not a huge price to pay in exchange.

[josh]

Rather, it is that one of the things it does (cloning) shouldn’t be actively encouraged to be treated as insignificant and become invisible, especially not by means that are known to have caused countless problems and correctness bugs in the history of programming language design.

Very well said, thank you; this is exactly my concern throughout this thread.

[dhm]

Just a random thought that could (or not) conciliate both “parties” : now that the language is mature and far away from its overly sigilesque beginnings, could it be considered to add a cloning sigil / prefix-or-suffix operator ?

More precisely, I’d imagine an ops::CheapClone : Clone empty/marker trait that would allow using such a sugared operator on types like Rc or Arc

[ExpHP]

I am generally of the opinion that AutoClone is not the deadly threat to all the things we know and love about rust that some people are making it out to be; but surely others have other already stated the same reasons I would have, so I won’t bore you.

---

On the other hand, here’s one thing that I know isn’t mentioned in the thread: (I searched!)

impl<T: Clone> Rc<T> {
    /// Makes a mutable reference into the given `Rc`.
    ///
    /// If there are other `Rc` or [`Weak`][weak] pointers to the same value,
    /// then `make_mut` will invoke [`clone`][clone] on the inner value to
    /// ensure unique ownership. This is also referred to as clone-on-write.
    #[inline]
    #[stable(feature = "rc_unique", since = "1.4.0")]
    pub fn make_mut(this: &mut Self) -> &mut T {
        ...
    }
}

Thanks to make_mut, AutoClone on Rcs and Arcs could create drastic performance and memory issues in existing, working code. (like this issue) And it wouldn’t be easy to fix, either!

So… I don’t know if I’d implement it for these types!

[kornel]

I already assume deref() and as_ref() are always super cheap and nothing else than a pointer cast/offset. I would be unpleasantly surprised if someone actually took advantage of ability to run expensive arbitrary code in these contexts.

[H2CO3]

far away from its overly sigilesque beginnings

I would not like to go back there, so no, let’s not add more operators for something as trivial as one argument-less method call. In this regard .clone() is no better or worse candidate for an operator than e.g. .into() was for postfix ! a couple of months ago — there is really not much value in either.

[kornel]

In Swift I don’t know when refcounts are optimized out and when they aren’t. This feels similar to the problem I have with Rust: I don’t know when bounds checks are optimized out and when they aren’t.

If Rust had a practical solution for this “is it optimized out or not” problem, then automatic refcounts and other automatic things could be OK. Otherwise I prefer erring on the side of visibility and control.

What about automutex and autorefcell? These are often used together with the refcount types, so autorefcount solves only half of the boilerplate.

[illicitonion]

I would like to propose a somewhat alternative mechanism of achieving the same goal.

An attribute that be added to a crate/module/function which explicitly lists what types should be implicitly cloned in the context of that attribute. i.e. #![AutoClone(Arc, Rc)]

There’s no ImplicitClone trait, and nothing is declared on the type itself. This allows the caller to decide what they view as expensive or cheap, rather then the struct author deciding - because different consumers have different opinions here.

There’s an explicit list of types which are being opted in in the calling scope, which means you don’t need to worry that other types will implicitly be cloned which may be expensive. The crate author can decide explicitly, and their decisions don’t leak out into their dependees.

This doesn’t bifurcate the language; you have to be explicit about what you’re applying this to, and as there’s nothing special on the type-defining side, you don’t need to worry about half of the ecosystem not using a marker trait.

Also, if someone decides they want to undo this change in their crate, it should be possible to write some code which would re-write the source to remove the attribute and make the implicit clones explicit again.

My particular motivation here is that I use rust in higher-level contexts with a lot of Futures, and absent async/await with borrowing, I have to clone a lot of Arcs into Futures. I personally don’t care about the cost of these Arc clones (they’re much cheaper than the network calls / forks that run in my particular Futures), and I don’t care too much about having to write them, but the number of them I need to read actually makes it harder to read the intent of the code. In these projects, I would happily mark Arc as AutoClone, but I can see why other contexts would find this problematic. So let’s allow the author to decide, in a way that doesn’t force the decision on others?

[josh]

Please remember that usability and ergonomics depends not just on writing code and reading your own code, but on reading other people’s code. Opt-ins don’t solve that problem; they make it worse by making code inconsistent.

.clone() is self-documenting and makes it clear the points at which a clone occurs. That’s not a property I’d like to lose.

[illicitonion]

I can make the same argument about Copy - sometimes it’s expensive, and a reader may want to be altered to a copy happening. So let’s make Copy also explicit (need to call .copy()), and also add an AutoCopy attribute. We could do this in the next edition.

Perhaps we could add #[AutoCopy(u8, i8, ...)] to the prelude for convenience. Perhaps we shouldn’t because explicit is better than implicit, and each crate should opt in to their own idea of what’s cheap to copy.

Then we would have more consistency, and more explicitness, with a well known place to look for behaviour changes.

[josh]

There are already plans to flag and warn about expensive copies. Also, it’s not just a matter of “expensive”, it’s a matter of user-defined code (which can do anything) versus entirely compiler-generated code.

[cksac]

Please remember that usability and ergonomics depends not just on writing code and reading your own code, but on reading other people’s code. Opt-ins don’t solve that problem; they make it worse by making code inconsistent.

.clone() is self-documenting and makes it clear the points at which a clone occurs. That’s not a property I’d like to lose.

You are right that reading code is important too, but I think writing .clone is not the only way and may not the best way to help that. Those lightweight clone actually are noise make expense clone difficult to locate. With IDE and RLS’s help, we can highlight where autoclone occurs too. We also can have rustfix to covert autoclone into explicit clone if the crate author want to.

There are already plans to flag and warn about expensive copies. Also, it’s not just a matter of “expensive”, it’s a matter of user-defined code (which can do anything) versus entirely compiler-generated code.

We can do so to flag and warn autoclone also if it exists. And I don’t think “compiler genrated code” is strong reason to against introduce autoclone, as we already have operator overload and Deref which can run arbitrary user code. And most importantly, I believe compiler is a tool to execute user code after all.

I think explicitness is generally good and should encouraged but still some can be elided. The crate author should have a choice to opt-in, currently, is not possible.

[anp]

Apologies if I missed this up thread, but what about requiring that the clone function for AutoClone must be const? IIRC @cramertj proposed this recently. If you can’t do I/O, and you can only perform intermediate allocations that don’t escape the function, then many of the potential perf implications of pervasive AutoClone could be significantly mitigated. Going even further, you could imagine a compiler lint that ensures it doesn’t allocate temporarily even.

[rpjohnst]

That assumes that const fn will never be able to make escaping allocations. That may not be coming anytime soon but it would certainly be useful so I wouldn’t rule it out yet.