-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
base: master
Are you sure you want to change the base?
Conversation
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 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 Which can presumably be solved either by making LLVM smarter about atomics for the specific case of 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. |
The problem with a Even without the
I think there's potential value there (and I've captured this in the RFC); for instance, we could omit a clone of a |
I'm afraid I'm pretty negative about this RFC.
|
I've added a new paragraph in the "Rationale and alternatives" section explaining why
|
@Diggsey wrote:
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.
That assumes that users want to call
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.
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.)
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.
I've added this as an alternative, but I don't think that would be nearly as usable. |
@Diggsey wrote:
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 |
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:
|
Since "Precise capturing" #3617 also abuses the |
We should really not overload the usage of 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?
In this, the
We use this multi-clones pattern outside async code too, so this non-async specific approach benefits everyone. |
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. |
How do you plan to do anything in a multithreaded context without |
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 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. |
If our main motivation is And yes, users will derive it for their own types when they can't be asked to write If however the argument is "users should have the choice to make |
but
Now, with tuples
is partially contradicted by the semantic effects that cloning an Looking at the general rules, seeing the “not statically known to be dead” case is restricted to by- I believe, for this, it’s only “really” problematic if compiler updates were to remove cases where this is detected, because then a users |
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 Don’t let perfect be the enemy of good. |
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 Alternatively, introduce 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. 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
}; |
While I'd prefer the 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 |
Note that
|
Currently, when I want to pass variables to a closure, I use the following construct:
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:
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:
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: "use" in this context could mean, clone or move if this is the last use of this variable |
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 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. |
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 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. |
This is the inverse of what we have today: you just have to know that 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 The learning curve here would be the same as learning why you don't need to |
You don't have to "know" that With
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.
How is that addressed by any of the auto-claim/clone proposals so far? At least with
It's not though: When you learn about In a world where the compiler no longer implicitly inserts copies of
|
[disclaimer: I'm not a a Rust expert] 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 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. |
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 |
For the cases where you do want to see every Rc clone, you can just |
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 |
This RFC uses as motivation the ergonomics of cloning instead of moving into closures and async blocks. It solves this by creating The RFC also proposes new syntax Some of these optimisations may be desired, and could be implemented for 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 |
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. |
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. And I'd honestly prefer a 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 Related, I don't like the addition of the |
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 pointMany 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
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 "Cheap is in the eye of the beholder"Several people pointed out that it is difficult to define what is "cheap". This led Dirbaio to propose "use-site" declarations like Another alternative (floated by Mark-Simulacrum and cynecx) was to provide some kind of "inline syntax" like "Optional" optimizations are creepyThere was some disagreement about whether the idea of optimizing and the Ugly (bikeshed-like concerns)The name
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
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 |
Provide a feature to simplify performing lightweight clones (such as of
Arc
/Rc
), particularly cloning them into closures or async blocks, whilestill 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 intoan 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:
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