Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify lightweight clones, including into closures and async blocks #3680

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Aug 20, 2024

Provide a feature to simplify performing lightweight clones (such as of
Arc/Rc), particularly cloning them into closures or async blocks, while
still keeping such cloning visible and explicit.

A very common source of friction in asynchronous or multithreaded Rust
programming is having to clone various Arc<T> reference-counted objects into
an async block or task. This is particularly common when spawning a closure as
a thread, or spawning an async block as a task. Common patterns for doing so
include:

// Use new names throughout the block
let new_x = x.clone();
let new_y = y.clone();
spawn(async move {
    func1(new_x).await;
    func2(new_y).await;
});

// Introduce a scope to perform the clones in
{
    let x = x.clone();
    let y = y.clone();
    spawn(async move {
        func1(x).await;
        func2(y).await;
    });
}

// Introduce a scope to perform the clones in, inside the call
spawn({
    let x = x.clone();
    let y = y.clone();
    async move {
        func1(x).await;
        func2(y).await;
    }
});

All of these patterns introduce noise every time the program wants to spawn a
thread or task, or otherwise clone an object into a closure or async block.
Feedback on Rust regularly brings up this friction, seeking a simpler solution.

This RFC proposes solutions to minimize the syntactic weight of
lightweight-cloning objects, particularly cloning objects into a closure or
async block, while still keeping an indication of this operation.


This RFC is part of the "Ergonomic ref-counting" project goal, owned by
@jkelleyrtp. Thanks to @jkelleyrtp and @nikomatsakis for reviewing. Thanks to
@nikomatsakis for key insights in this RFC, including the idea to use use.

Rendered

@joshtriplett joshtriplett added T-lang Relevant to the language team, which will review and decide on the RFC. I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. labels Aug 20, 2024
text/0000-use.md Outdated Show resolved Hide resolved
@5225225
Copy link

5225225 commented Aug 20, 2024

Personally, I don't feel that the non-closure/block use cases of this are really strong enough to warrant adding this, and the closure/block use case can be fixed with clone blocks.

The example

let obj: Arc<LargeComplexObject> = new_large_complex_object();
some_function(obj.use); // Pass a separate use of the object to `some_function`
obj.method(); // The object is still owned afterwards

could just be written as some_function(obj.clone()) with the only downsides being "that will still compile even if obj is expensive to clone" (Which is likely more easily solvable through a lint rather than a language feature), and not being able to remove redundant clones.

Which can presumably be solved either by making LLVM smarter about atomics for the specific case of Arc, or having an attribute on a clone impl that gives it the semantics use is being given here (Which would benefit all code that uses that type, not just new code that has been written to use .use)

The ergonomics of needing to clone in a block are annoying though, I agree, but that's a smaller feature by being able to do:

spawn(async clone {
    func1(x).await;
    func2(y).await;
});

and similarly for closures.

@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 20, 2024

the closure/block use case can be fixed with clone blocks

The problem with a clone block/closure is that it would perform both cheap and expensive clones. A use block/closure will only perform cheap clones (e.g. Arc::clone), never expensive ones (e.g. Vec::clone).

Even without the .use syntax, async use blocks and use || closures provide motivation for this.

or having an attribute on a clone impl that gives it the semantics use is being given here (Which would benefit all code that uses that type, not just new code that has been written to use .use)

I think there's potential value there (and I've captured this in the RFC); for instance, we could omit a clone of a String if the original is statically known to be dead. I'd be concerned about changing existing semantics, though, particularly if we don't add a way for users to bypass that elision (which would add more complexity).

@Diggsey
Copy link
Contributor

Diggsey commented Aug 20, 2024

I'm afraid I'm pretty negative about this RFC.

Use trait

I don't like the Use concept as applied here: I don't think it makes sense to tie the concept of a "lightweight clone" to the syntax sugar for cloning values into a closure. Why can't I clone heavy-weight objects into a closure? It seems like an arbitrary distinction imposed by the compiler, when the compiler cannot possibly know what the performance requirements of my code are.

I could imagine there might be scenarios where knowing if a clone is light-weight is useful, but I don't think this is one of them.

.use keyword

I think the suffix .use form is unnecessary when you can already chain .clone(), and it's confusing for new users that .clone() requires brackets, whilst .use does not. Consistency is important. .use does not do anything that couldn't be done via a method, so it should be method - in general the least powerful language construct should be chose, in the same way that you wouldn't use a macro where a function would suffice.

However, x.use does not always invoke Clone::clone(x); in some cases the compiler can optimize away a use.

I don't like the "can" in this statement. Optimizations should fall into one of two camps:

  1. Automatic. These optimizations are based on the "as if" principle - ie. the program executes just as if the optimization was not applied, it just runs faster.
  2. Opt-in. This covers things like guaranteed tail calls, where the programmer says "I want this to be a tail-call" and the compiler returns an error if it can't do it.

Giving the compiler implementation a choice which has program-visible side-effects, and then specifying a complicated set of rules for when it should apply the optimization is just asking for trouble (eg. see C++'s automatic copy elision...) and I don't want to work in a language where different compilers might make the exact same code execute in significantly different ways.

use closure

If any object referenced within the closure or block does not implement Use (including generic types whose bounds do not require Use), the closure or block will attempt to borrow that object instead

I think this fallback is dangerous, as it means that implementing Use for existing types can have far-reaching implications for downstream code, making it a backwards compatibility hazard.

Motivation

Getting back to the original motivation: making reference counting more seamless, I think simply adding a syntax or standard library macro for cloning values into a closure or async block would go a long way to solving the issue... Potentially even all the way.

If a more significant change is needed, then I think this should be a type of variable binding (eg. let auto mut x = ...) where such variables are automatically cloned as necessary, but I hope such a significant change is not needed.

@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 20, 2024

I've added a new paragraph in the "Rationale and alternatives" section explaining why async clone/clone || would not suffice:

Rather than specifically supporting lightweight clones, we could add a syntax
for closures and async blocks to perform any clones (e.g. async clone /
clone ||). This would additionally allow expensive clones (such as
String/Vec). However, we've had many requests to distinguish between
expensive and lightweight clones, as well as ecosystem conventions attempting
to make such distinctions (e.g. past guidance to write Arc::clone/Rc::clone
explicitly). Having a syntax that only permits lightweight clones would allow
users to confidently use that syntax without worrying about an unexpectedly
expensive operation. We can then provide ways to perform the expensive clones
explicitly, such as the use(x = x.clone()) syntax suggested in
[future possibilities][future-possibilities].

@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 20, 2024

@Diggsey wrote:

I don't think it makes sense to tie the concept of a "lightweight clone" to the syntax sugar for cloning values into a closure. Why can't I clone heavy-weight objects into a closure?

You can; I'm not suggesting that we couldn't provide a syntax for that, too. However, people have asked for the ability to distinguish between expensive and lightweight clones. And a lightweight clone is less of a big deal, making it safer to have a lighter-weight syntax and let users mostly not worry about it. We could additionally provide syntax for performing expensive clones; I've mentioned one such syntax in the future work section, but we could consider others as well if that's a common use case.

I think the suffix .use form is unnecessary when you can already chain .clone()

That assumes that users want to call .clone(), rather than calling something that is always lightweight. If we separate out that consideration, then the question of whether this should be .use or a separate (new) trait method is covered in the alternatives section. I think it'd be more unusual to have the elision semantics and attach them to what otherwise looks like an ordinary trait method, but we could do that.

.use does not do anything that couldn't be done via a method, so it should be method

This is only true if we omitted the proposed elision behavior, or if we decide that it's acceptable for methods to have elision semantics attached to them. I agree that in either of those cases there's no particular reason to use a special syntax rather than a method.

I don't like the "can" in this statement. [...] Giving the compiler implementation a choice which has program-visible side-effects, and then specifying a complicated set of rules for when it should apply the optimization is just asking for trouble

This is a reasonable point. I personally don't think this would cause problems, but at a minimum I'll capture this in the alternatives section, and we could consider changing the elision behavior to make it required. The annoying thing about making it required is that we then have to implement it before shipping the feature and we can never make it better after shipping the feature. I don't think that's a good tradeoff.

Ultimately, though, I think the elisions aren't the most important part of this feature, and this feature is well worth shipping without the elisions, so if the elisions fail to reach consensus we can potentially ship the feature without the elisions. (Omitting the elisions entirely is already called out as an alternative.)

Getting back to the original motivation: making reference counting more seamless, I think simply adding a syntax or standard library macro for cloning values into a closure or async block would go a long way to solving the issue... Potentially even all the way.

See the previous points about people wanting to distinguish lightweight clones specifically. This is a load-bearing point: I can absolutely understand that if you disagree with the motivation of distinguishing lightweight clones, the remainder of the RFC then does not follow. The RFC is based on the premise that people do in fact want to distinguish lightweight clones specifically.

If a more significant change is needed, then I think this should be a type of variable binding (eg. let auto mut x = ...) where such variables are automatically cloned as necessary

I've added this as an alternative, but I don't think that would be nearly as usable.

@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 20, 2024

@Diggsey wrote:

use closure

If any object referenced within the closure or block does not implement Use (including generic types whose bounds do not require Use), the closure or block will attempt to borrow that object instead

I think this fallback is dangerous, as it means that implementing Use for existing types can have far-reaching implications for downstream code, making it a backwards compatibility hazard.

While I don't think this is dangerous, I do think it's not the ideal solution, and I'd love to find a better way to specify this. The goal is to use the things that need to be used, and borrow the things for which a borrow suffices. For the moment, I've removed this fallback, and added an unresolved question.

@davidhewitt
Copy link
Contributor

Thank you for working on this RFC! PyO3 necessarily makes heavy use of Python reference counting so users working on Rust + Python projects may benefit significantly from making this more ergonomic. The possibility to elide operations where unnecessary is also very interesting; while it's a new idea to me, performance optimizations are always great!

I have some questions:

  • The name Use for the trait was quite surprising to me. Reading the general description of the trait and the comments in this thread, it seems like "lightweight cloning" or "shallow cloning" is generally the property we're aiming for. Why not call the trait LightweightClone or ShallowClone? (Maybe can note this in rejected alternatives?)

  • The RFC text doesn't make it clear to me why use & move on blocks / closures need to be mutually exclusive. In particular what if I want to use an Arc<T> and move a Vec<Arc<T>> at the same time; if I'm not allowed the move keyword then I guess I have to fall back to something like let arc2 = arc.use; and then moving both values? Seems like this is potentially confusing / adds complexity.

  • I would like to see further justification why the rejection of having Use provide automatic cloning for these types. I could only find one short justification in the text: "Rust has long attempted to keep user-provided code visible, such as by not providing copy constructors."

    • We already have user-provided code running in Deref operations for most (all?) of the types for which Use would be beneficial. Is it really so bad to make these types a bit more special, if it's extremely ergonomic and makes room for optimizations of eliding .clone() where the compiler can see it?
    • Further, I think Clone is already special: for types which implement Copy, a subtrait of Clone, we already ascribe special semantics. Why could we not just add ShallowClone as another subtrait of Clone which allows similar language semantics (but doesn't go as far as just a bit copy, which would be incorrect for these types)?

@kennytm
Copy link
Member

kennytm commented Aug 21, 2024

Since "Precise capturing" #3617 also abuses the use keyword this may be confusing to teach about the 3 or 4 unrelated usage of the keyword (use item; / impl Trait + use<'captured> / use || closure & async use { block } / rc.use).

@burdges
Copy link

burdges commented Aug 21, 2024

We should really not overload the usage of the keyword use so much, but ignoring the keyword..

Isn't it easier to understand if we've some macro for the multiple clones that run before the code that consumes them, but still inside some distinguished scope?

{
    same_clones!(x,y,z);
    spawn(async move { ... });
}

In this, the same_clones! macro expands to

let x = x.clone();
let y = y.clone();
let z = z.clone();

We use this multi-clones pattern outside async code too, so this non-async specific approach benefits everyone.

@ssokolow
Copy link

ssokolow commented Nov 27, 2024

I'm sorry but I have to disagree.

I've already said what I could more eloquently... but what you proposed just makes this idea feel even more C++-esque to me than it already did... in a language where "Becomes too complex" was already the biggest worry voiced for the future of Rust in the 2023 Annual Rust Survey and had gone up 5.5 percentage points since the 2022 survey.

@dev-ardi
Copy link

Shared ownership is inherently a code smell and nothing will change that...

How do you plan to do anything in a multithreaded context without Arc?

@ssokolow
Copy link

ssokolow commented Nov 28, 2024

Shared ownership is inherently a code smell and nothing will change that...

How do you plan to do anything in a multithreaded context without Arc?

The quote you truncated continues with "we just have to recognize that, sometimes, the least bad solution to a problem is still a smelly one, rather than trying to abolish the concept of a sense of smell."

In other words, I'm saying that this is a "the solution to unsafe being ugly isn't to make unsafety opt-out instead of opt-in" situation.

Sometimes, there are no "good solutions"... but that doesn't mean hanging a tablecloth on the un-good-ness to hide it if that'll encourage people getting sloppy elsewhere.

@Nadrieril
Copy link
Member

If our main motivation is Arc/Rc, we could phrase this feature explicitly as being about smart pointers and similar things. Much like Deref says "this is meant for smart pointers, please avoid hacky uses", AutoClone could have a documentation that explains the typical usage and lets users decide what's appropriate. I've not seen abuse over Deref in the ecosystem, I'm pretty confident we can reach a similar state with AutoClone.

And yes, users will derive it for their own types when they can't be asked to write .clone() everywhere, much like users write convenient Deref or AsRef impls today. That seems fine, we handle it today by not exposing those to public APIs.

If however the argument is "users should have the choice to make clone optional/configurable in their crate (even for e.g. Vec and String), then that's a substantially different feature that might look more like Niko's profiles.

@steffahn
Copy link
Member

steffahn commented Dec 8, 2024

This trait should be implemented for anything in the standard library that meets these criteria. Some notable types in the standard library that implement Use:> * …

  • Tuples of types whose components all implement Use

but

/// Cloning an object implementing this trait should in general:
/// - …
/// - not require copying more than roughly 64 bytes (a typical cache line size),

Now, with tuples 64 bytes (e.g. 8 pointers or 4 fat pointers) is reached rather quickly. Maybe tuples should be excluded? Similarly derive(Use) with multiple fields could be bad– especially for types with generics, because that then also easily allows users to then build up arbitrarily large Use-implementing types out of them.


/// Cloning an object implementing this trait should in general:
/// - …
/// - not have any semantic side effects (e.g. allocating a file descriptor), and

is partially contradicted by the semantic effects that cloning an Arc/Rc can have: When there’s only one reference, access such as Arc::get_mut is possible. Especially w.r.t. the mechanism of eliding .use calls, this seems relevant.

Looking at the general rules, seeing the “not statically known to be dead” case is restricted to by-&_ usage which prevents any usage of get_mut-style API to begin with, this need only concern the optimization based on the x is owned and statically known to be dead case.

I believe, for this, it’s only “really” problematic if compiler updates were to remove cases where this is detected, because then a users .get_mut call may start to fail. Still, as far as I know a value being “statically known to be dead” isn’t necessarily always the easiest analysis, so one should maybe start only with obvious cases, and add complexity of the analysis only as long as it seems tractable to ensure that future improvements/iterations can never miss any previously-detected cases of a value being “statically known to be dead”.

@orf
Copy link

orf commented Dec 16, 2024

Well, this is a… fun read. It’s definitely gone quite far off the rails though.

The main contention is that we don’t want users to do complex stuff in a shallow copy implementation and have the compiler then silently invoke that implementation. So we either restrict it, or make it “explicit but ergonomic” (this RFC).

There is no consensus on which direction is best, but there is a wide consensus that the “clone” vs “copy” dichotomy is missing something in-between.

Perhaps we should focus on that, as an internal, private trait that cannot be implemented except for built-in types? This would allow an incremental and useful step forward, without being blocked by discussions around syntax changes.

Simply making Arc ShallowCopy-able would simplify a significant amount of code, in a way that can be easily adapted in the future to any future improvements in this area.

Don’t let perfect be the enemy of good.

@thynson
Copy link

thynson commented Dec 17, 2024

I believe one of Rust's directions is to ergonomically make something explicit while still preserving a reasonable implicit default; the feature of precise capture reflects this spirit.

I think the debates around whether cloning an Arc is cheap don't make sense, since such cost is unavoidable if the program is implemented in this way.

And I do care about the ergonomical explicitness, that says, I accept syntax like use || {} or claim || {} instead of || {}. However, I would prefer clone || {} instead, which does not require introducing a new trait.

Alternatively, introduce Claim or Use but limit their usage to types that act as a smart pointer. And lint rules should be introduced that warn for types to implement the trait that are larger than a fat pointer, and/or are not smart pointers.

On the other hand, I feel it's regret that Rust closure didn't adopt a syntax to precisely control how variables are captured( like C++ lambda capture list), which sometimes brings unnecessary cost while sometimes requiring a workaround to make a variable be captured, e.g.

let guard = some_guard(); // release locks when dropped
do_something(|ctx| async move {
  // ...
  let _ = &guard; // We need to mention `guard` to move it into this closure
});

So my (informal) proposal is about ergonomic-explicit-capture, which would be a bit off-topic but related.
I believe a closure should be able to specify how each variable is captured.

let foo = "foo".to_string();
let bar = "bar".to_string();
let buz = "buz".to_string();
let closure = move(bar) clone(buz) || {
  log::debug!("{}", foo); // foo is still borrowed
  // do something with bar and buz
};
// bar no longer available

for the previous example, it can be improved to

let guard = some_guard();
let param = some_param();

do_something(|ctx| async move(guard,..) {
  // move(guard,..) denotes anything other than `guard` is also captured implicitly by move
  let a = ctx.get_something(param);
});

and when not capturing variables explicitly

let closure_1 = clone || { // equialent to clone(..) 
   // anything mentioned is cloned
};
let closure_2 = move || { // equialent to move(..) 
   // anything mentioned is moved, which is exactly what it is in the current Rust
};
let closure_3 = clone(..), move(..) {
   // should be a compile error, conflicting capture expression
};

@ssokolow
Copy link

ssokolow commented Dec 17, 2024

and when not capturing variables explicitly

While I'd prefer the let closure_1 = clone || { // equialent to clone(..) case not exist (i.e. I believe it's too far removed from "make costs explicit"), I'd still prefer this idea as-is over Claim.

To me, automatic cloning at the call site is less a problem if it must be opted into at the call site, similar to how ? is better than throwing exceptions for similar reasons.

@kennytm
Copy link
Member

kennytm commented Dec 17, 2024

Note that clone || ptr isn't possible (neither is clone(..) || ptr) without making clone a keyword (meaning a new edition and everyone needs to write r#clone elsewhere).

use(clone) || ptr (as in #3680 (comment)) should be the minimal unambiguous syntax.

@szagi3891
Copy link

szagi3891 commented Dec 21, 2024

Currently, when I want to pass variables to a closure, I use the following construct:

let z = {
    let a = a.clone();
    let b = b.clone();

    move || {
       //... some operations on these variables
    }
};

At the beginning of my learning process, I found it quite inconvenient that I couldn't define more precisely which variables should be captured by the closure and in what mode.

It might be worth considering an alternative syntax that doesn't require adding a new trait into language:

let z = use a, use b || {
    //... some operations on these variables
};

This way, we could specify very precisely how individual variables are passed to the closure. It would also allow mixing variables that are moved into the closure with those shared by reference (though I'm not sure if such a mixed use case makes sense).

For example, mixed usage could look like this:

let z = use a, use b, &c, &d, move e || {
    //... some operations on these variables
};

By explicitly defining which variables and how they are passed to the closure, the compiler could pinpoint errors very precisely (in cases where a bug exists in the program).

For instance:
The compiler could indicate that a variable is missing in the "capture definition."
The compiler could indicate that we passed a variable to the closure that is not used within it.

"use" in this context could mean, clone or move if this is the last use of this variable

@orf
Copy link

orf commented Dec 21, 2024

It might be worth considering an alternative syntax that doesn't require adding a new trait into language:

let z = use a, use b || {
    //... some operations on these variables
};

IMO this will still be confusing to new users, and adds non-trivial syntax.

This is the main problem with these proposals. The ultimate, easy-to-learn outcome would be for this to work:

let a = Arc::new(vec![]);
let z  = move || {
    a.clone()
};
drop(a);

but for this to fail:

let a = vec![];
let z  = move || {
    a.clone()
};
drop(a);

Adding “use” prefixes brings little benefit over just cloning in an outer closure: it’s an extra thing to learn, refactor, update etc.

Is there no way we can think of to make a distinction between a captured variable that is only cloned, and a capture that is used in any way other than cloning?

This would involve a separate trait to clone, one suitable for Arc<T> and other reference counted data that isn’t quite copy but isn’t quite clone either.

it just needs to de-sugar to something akin to this:

let a = Arc::new(vec![]);
let a_ref = a.clone();
let z  = move || {
    a_ref
};
drop(a);

but only for types that are marked as being capable of this.

@bitwalker
Copy link

The ultimate, easy-to-learn outcome would be for this to work:

let a = Arc::new(vec![]);
let z  = move || {
    a.clone()
};
drop(a);

but for this to fail:

let a = vec![];
let z  = move || {
    a.clone()
};
drop(a);

How can this be characterized as "easy to learn"? Someone new to Rust isn't going to understand why the former works, but the latter doesn't. It is completely non-obvious from the source code, you just have to know that Arc is special here (if I even understand your implied semantics correctly). I had to re-read it a couple times to make sure I wasn't missing something, and that's despite knowing the code was presented in the context of this discussion.

The approach one has to use today is verbose, but explicit, which makes it easy to understand (albeit annoying, to some). Readability, an almost useless metric considering how subjective it is, is arguably negatively impacted. IMO one can only make that claim if knowing what is captured (and how) is irrelevant to the rest of the code - but that is entirely context-sensitive, and not something that can be inferred by the compiler. The same person, reading the same code, might in one instance care about those details, and in other instance, consider it irrelevant detail. Ideally, any solution here ensures that both readings do not suffer from lack of clarity.

Explicit capture syntax has the advantage of removing the worst of the boilerplate and verbosity of the current approach people use today, while retaining (and in fact improving) clarity of capture semantics for those who value that more. That appears to me to be a strict improvement over the status quo, for everyone, regardless of which metric you measure it by.

@orf
Copy link

orf commented Dec 21, 2024

How can this be characterized as "easy to learn"? Someone new to Rust isn't going to understand why the former works, but the latter doesn't. It is completely non-obvious from the source code, you just have to know that Arc is special here (if I even understand your implied semantics correctly). I had to re-read it a couple times to make sure I wasn't missing something, and that's despite knowing the code was presented in the context of this discussion.

This is the inverse of what we have today: you just have to know that Arc (and types like it) are special in that Arc::clone is cheap like a Copy - except it's not Copy for reasons ™️, and because of these reasons ™️ you need to verbosely use clone() (a call with "heavy" connotations), create extra references and use extra scopes in order to do something that you understand to be unnecessary.

And, on top of all that, you need to just know that by refactoring completely different code in a completely different place your super cheap clone() calls can abruptly become super expensive without any notice or warnings.

The learning curve here would be the same as learning why you don't need to clone() a usize when passing it into a move closure. I see it as less to learn.

@bitwalker
Copy link

This is the inverse of what we have today: you just have to know that Arc (and types like it) are special in that Arc::clone is cheap like a Copy - except it's not Copy for reasons ™️, and because of these reasons ™️ you need to verbosely use clone()

You don't have to "know" that Arc is cheap at all, nor is it special in that regard, you see a .clone(), and if you are unsure/concerned about what happens when that type is cloned, you pop open its Clone impl and find out as-needed. Whether that impl is acceptable to clone at that particular point in the program is up to you. Over time, you build an intuition for what happens when various types are cloned, but you start off understanding that Clone in general can be expensive, have side effects, and that paying attention to when things are cloned is important for performance-sensitive code.

With Copy, it is implied that the type is cheap to copy, that it will be done as a bitwise copy and can have no side effects, and that the compiler enforces the invariant that a Copy type cannot consist of non-Copy elements. This is restrictive, but something one can immediately build an intuition for. The fact that the compiler does not currently raise a lint about large Copy types, is an oversight that could be trivially addressed (I haven't checked, but perhaps clippy already does this?).

(a call with "heavy" connotations), create extra references and use extra scopes in order to do something that you understand to be unnecessary.

The idea that "you understand [it] to be unnecessary" here is completely subjective and context-sensitive. That may very well be the case for many situations, but not for all situations. There have been various addendums/suggestions to the proposal to allow opting-out of auto-claim/clone behavior, specifically because of that. The problem with some kind of crate/module/function-wide opt-out, is that the language ends up with two more dialects, one where this is all implicit, and one where it is all explicit - that seems to me to be an even worse state of affairs for those who will be learning Rust.

And, on top of all that, you need to just know that by refactoring completely different code in a completely different place your super cheap clone() calls can abruptly become super expensive without any notice or warnings.

How is that addressed by any of the auto-claim/clone proposals so far? At least with Copy, a compiler lint could tell you straight away "hey, this is going to make this type expensive to move around, you might want to reconsider this, or remove the Copy impl". Claim/Use cannot have their invariants compiler-checked, and thus require much more vigilance than Copy.

The learning curve here would be the same as learning why you don't need to clone() a usize when passing it into a move closure. I see it as less to learn.

It's not though: When you learn about Copy, you not only learn that you never need to call .clone() on such types, but that Copy values are, by definition, moved by copying. So it would come as no surprise to anyone that you not only don't need a call to .clone() in that situation, but that moving a Copy value doesn't consume the original.

In a world where the compiler no longer implicitly inserts copies of Copy types, and instead Claim/Use is used for that purpose, things are immediately and drastically less intuitive:

  • The compiler can no longer guarantee that these implicit operations are free of side-effects. The contract of Claim is vague enough that it is non-obvious that these implicit operations are acceptable in any given context. The compiler will just assume that it is.
  • Reasoning about Copy is intuitive (it's "just a bitwise copy"), but Claim has no such grounding idea (it's "cheaply cloneable", where "cheap" is whatever the implementor defined it to mean). This makes it difficult to reason about effects on program performance without looking into a specific implementation, and even then it may not be readily apparent what the costs are (e.g. do you know how much modifying a reference counter costs there? what about if it is contended?). With a bitwise copy of a chunk of memory, you can immediately reason in precise terms what it means to copy 8, 16, 64, or 256 bytes at a given point in a program.
  • The rationale for a Copy trait in this world becomes less clear, since it would be reasonable for someone to assume that the compiler would have all necessary information from Claim and Clone alone to determine that a given type could have its Clone impl lowered to a bitwise copy. The compiler would probably need to do more work to make that happen, but the idea of implementing Copy will make less sense to someone coming into Rust when it isn't inherently tied to the implicit copies inserted by the compiler.

@xkr47
Copy link

xkr47 commented Jan 16, 2025

[disclaimer: I'm not a a Rust expert]
While there has been discussions about various syntaxes, the above two PRs seem to suggest a syntax is being considered where closures do not explicitly indicate which objects are getting cloned into the closure. I worry because this would effectively hide increases in reference counts of *Rc types. I.e. it would make it harder to track down why memory usage increases because some Rc got surprise-cloned into some closure when it is no longer in your face; you can no longer look for calls to clone() or some other syntactical element to find all places where a Rc gets cloned. I therefore vote for a syntax where every clone operation is explicitly visible — be it through .use or some other syntax. So instead of

let foo = Arc::new(123);
bar(use || { doSomething(foo) });

I would suggest for example (reusing syntax suggested in dtolnay/syn#1802):

let foo = Arc::new(123);
bar(|| { doSomething(foo.use) });

or if the use before || is still considered good practice then:

let foo = Arc::new(123);
bar(use || { doSomething(foo.use) });

This would make it already much easier to track down all such ergonomic clones imo.

@ssokolow
Copy link

ssokolow commented Jan 16, 2025

The point @xkr47 is making is one of my biggest concerns with this idea, added complexity to be taught aside.

One of the reasons I use Rust is that, from the perspective of managing memory consumption, it has fewer footguns than a garbage collected language. (And I've run into stuff on more than one occasion where a "memory leak" turned out to be some piece of JavaScript forgetting to unhook an HTML DOM event handler, leading to stale stuff being spuriously held alive by some closure.)

If cloning Rc<T> and Arc<T> becomes completely automatic, then I'll have to start to consider forbidding them (ideally with Clippy lints rather than some kind of slapdash grep-based CI script) and replacing direct use of them with wrappers which block the auto-clone behaviour. (Actually, now that I think about it, this is starting to sound like the dynamic between memory-unsafe APIs, unsafe, and safety-enforcing abstractions.)

@Nadrieril
Copy link
Member

For the cases where you do want to see every Rc clone, you can just deny(implicit_clone) or however we call that.

@ssokolow
Copy link

ssokolow commented Jan 17, 2025

Better than nothing... though my much-earlier-mentioned concerns about knock-on effects still apply.

(That the ecosystem as a whole will become less memory efficient due to a more "laissez-faire by default" approach toward having the compiler surface these concerns. As C and C++ demonstrated very clearly, it's not as if it's practical for you to take responsibility for bringing your entire transitive dependency tree up to whatever standard you want to hold to. Rust has let mut instead of let const and unsafe instead of safe for a reason, after all.)

@nebneb0703
Copy link

This RFC uses as motivation the ergonomics of cloning instead of moving into closures and async blocks. It solves this by creating use || closures and async use blocks, where types that implement a Use trait may be cloned instead of moved.

The RFC also proposes new syntax .use which is not motivated and does not contribute to the ergonomics of cloning into closures or async blocks, as that is already solved without this operator. Aside from the future possibilities of the syntax, the main rationale for this syntax is to enable optimising redundant clones in the same scope (which is already solved by existing lints), or by allowing owned reference types to be "moved back" into the caller scope after calling a function that moves the reference. This behaviour strongly depends on the semantics of the type and its Drop implementation. A type becoming Use will be a SemVer breaking change as some functions/scopes can no longer depend on the fact that an owned type will have its Drop called, and this can be UB if not implemented on ref-like types. In particular, a function may check the strong count of an Arc to verify that it is the sole owner of the Arc. The description of the Use trait only specifies that an example implementation is a reference counted type, but does not specify that this optimisation may be invalid for a lot of other types. This trait is safe to implement, but may cause UB when combined with certain Drop implementations and functions.

Some of these optimisations may be desired, and could be implemented for .clone()s of Use types. Many negative responses to this RFC have been reassured that the new syntax and behaviour is optional, but I would like ref-like optimisations to also apply to code which opts out of these features. If a compiler implemented such features, what would be the difficulty in adding special cases to the .clone() calls, compared to a new .use operator?


What is the current status of this RFC? There remain unresolved review comments, and no FCP has started, but PRs have popped up in nightly and syn to begin implementation

@piegamesde
Copy link

As I already said in more words a while back (#3680 (comment)), this RFC mixes up two distinct issues which should be handled separately. I'd appreciate if it could be closed and superseded by two separate RFCs to each deal with one issue more fully.

@ColonelThirtyTwo
Copy link

ColonelThirtyTwo commented Jan 24, 2025

Coming across this for the first time, I do agree that cloning things into closures is unergonomic, but this RFC is the wrong solution.

The entire "cheaply cloneable" concept is a solution to a footgun that is only introduced by the solution of automatically cloning closure variables. If, for example, you consider a closure syntax where you explicitly mention the variables to clone, the footgun goes away.
It's an XY problem.

And I'd honestly prefer a clone(foo, bar, baz) || { ... } style syntax over implicit cloning - not only is it more explicit, but you don't have to artificially limit yourself to "cheaply cloneable" types for no reason.

The name "Use" is awful. The word "Use" can mean so many different things, and aliasing it with "clone" is extremely unclear. If a non-Rust programmer comes across foo.clone(), it is immediately clear that the value is being cloned, but what the heck does foo.use mean? Considering how much of these clones are going to be of Arc<RwLock<T>> or other locks, I would not put it past a newbie to think foo.use may mean "acquire a lock".

Related, I don't like the addition of the foo.use syntax at all. It's just foo.clone() but with strings attached. The only upside is that the RFC is written in a way that may omit a final clone for the last value, but since this is explicitly for "cheaply cloneable" types anyway, it's not going to matter.

@traviscross traviscross added I-lang-radar Items that are on lang's radar and will need eventual work or consideration. and removed I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. labels Jan 26, 2025
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jan 30, 2025

Hi folks,

I just took an hour to read every comment on this thread. I wanted to summarize the various bits of feedback to try and capture the main points that were made. I would break things into three buckets. I'd love to hear if you think there is any major point that I missed here.

The Good (points of general agreement)

capturing ref counted values in closures and async blocks is a pain point

Many folks (though not all) agreed that the state of capture in today's closures is indeed a pain point. David Hewitt from PyO3 also highlighted that "PyO3 necessarily makes heavy use of Python reference counting so users working on Rust + Python projects may benefit significantly from making this more ergonomic".

The Bad (points of disagreement with the RFC)

"Can't serve two masters"

The gist of this concern is that, although the RFC declares a motivation of making things easier for new users, adding a new syntax like x.use and hence a category of types is, in fact, going to work against that goal. Diggsey I think put it well:

Having to navigate a codebase filled with this strange .use keyword, which is hard to explain (it copies the value.. except when it doesn't, and it can be used on Use types, but not Clone types, but Use is not Copy, it's different… When is something Use? What makes something lightweight? Uh… read this essay. Why do I need to make this closure use but this one doesn't need to be? Ahahaha, yeah this is going to take a while to explain…) is more of a blocker than clone-into-closure ever was.

Similar comments were made by boats and matthieu-m. In David Hewitt's comment, he also requested further justification for why one needs to write x.use and not just x, as there is already precedent for invisibly invoking user-provided methods via auto-deref.

"Cheap is in the eye of the beholder"

Several people pointed out that it is difficult to define what is "cheap". Rc and Arc have different execution costs, for example, and for many applications, even deep-cloning a map may not be prohibitive.

This led Dirbaio to propose "use-site" declarations like #![autoclone(Rc, Arc)], where crates or modules would declare the types that they believe should be autocloned.

#3680 (comment)

Another alternative (floated by Mark-Simulacrum and cynecx) was to provide some kind of "inline syntax" like use { x.clone() } or x.clone().super to indicate an "inline" expression in the closure that would actually execute at closure creation time.

"Optional" optimizations are creepy

There was some disagreement about whether the idea of optimizing x.use to potentially skip the clone was a good idea. The RFC did not say precisely when such calls would be elided, which means that users would be able to observe distinct behavior depending on the compiler they were using. On the other hand, as Josh pointed out, specifying a precise set of optimizations would mean that we are locking into those specific semantics forever.

and the Ugly (bikeshed-like concerns)

The name use is heavily overloaded

Several people pointed out that the use keyword is fairly generic and already has two meanings in Rust, use foo::bar and the recently introduce "precise capture" rules for impl Trait.

We could have a syntax for cloning into a closure

Given that closures are the main pain point, several people proposed having some form of syntax for cloning into a closure. There are a number of variations on this, e.g., clone || x or clone(x) || fooormove(x.clone()) foo` (as I once proposed in a draft RFC long ago) and so forth.

@ChayimFriedman2

This comment was marked as resolved.

@nikomatsakis

This comment was marked as resolved.

@ssokolow
Copy link

ssokolow commented Feb 1, 2025

Re: "We could have a syntax for cloning into a closure", I'm not sure it's fair to call it a "bikeshed-like concern" when talking about whether Rust should have a new hole in its preference for making behaviours explicit at the call site.

It's bad enough that we have panic! without a maintained cargo-geiger analogue and, as I mentioned, if anything, the examples with heavy-to-Copy arrays speak to #[derive(Copy)] being an under-cooked idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-lang-radar Items that are on lang's radar and will need eventual work or consideration. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.