-
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 vaguely remember reading some ideas about a postfix super syntax, which could maybe make this pattern more compact: {
let x = x.clone();
let y = y.clone();
spawn(async move {
func1(x).await;
func2(y).await;
});
}
// and in sugared form with a postfix super syntax
spawn(async move {
func1(x.super.clone()).await; // or maybe x.clone().super?
func2(y.super.clone()).await;
}); But I admit I might be misremembering the exact proposed semantics of that. |
Would |
@N4tus Good point. Quite honestly, not sure. But perhaps |
Yup, using This has the advantage that you don't need to name the value you want to capture and can use it directly. You also sidestep the issue on what types auto-clone works. When people say they want only auto-clone for "cheap" values, then what I hear is that they want auto-clones sometimes and not other times, because there are cases when a clone should not be hidden. By making it less painful to make every clone explicit I am hoping that is a sufficient compromise, between auto-cloning for a "smoother" developer-experience on higher-level projects and the explicit cloning that we currently have now. |
Yep, Setup: rustc: 1.80, Ubuntu 24.04 running on Windows 11 with WSL2 (2.1.5.0), 11th Gen Intel(R) Core(TM) i7–1165G7 @ 2.80 GHz
|
I agree with the motivation but can't shake that "use" is a poor name and this mechanism seems a bit arcane. Following on from #3680 (comment) comment proposing Would it be enough to have:
This seems fairly simple and ergonomic. The downside is only for those that don't want auto cheap clones having to remember to disable it, but this could be clearly documented with the edition. |
I think this RFC tackles two different issues at once and intermingles them:
The RFC text currently focuses on the shallow cloning, while not going far enough onto the cloning into closures problem. Some thoughts I'd see addressed:
|
As someone who is not a novice, but struggles with sleep issues that can often help me relate to people who struggle more to grasp concepts, I have to say that Conceptually, I see ...especially when this is scoping in the context of ownership and borrowing, ownership and borrowing is already something that takes effort to learn, and, unless I missed a big announcement, NLL Problem Case #3 is still a work in progress, providing even more learning friction. |
|
||
If every invocation of `.use` or `use ||` here resulted in a call to | ||
`Clone::clone`, this program would call `Clone::clone` 10 times (and have 11 | ||
`Arc`s to drop); however, the compiler can elide all the uses in `main` and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f
takes the Arc
by-value... how can the compiler elide a use
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea is that since the body of f
is known and would also "work" with &Arc
, the compiler could "rewrite" it to instead take &Arc
using the "If x
is not statically known to be dead, but all of the following conditions are met, the compiler may elide an x.use
and use &x
instead: ..." optimization listed above. (Saying nothing of how feasible implementing such an optimization would be)
[unresolved-questions]: #unresolved-questions | ||
|
||
Should we allow *any* `x.clone()` call to be elided if a type is `Use` (or `Use | ||
+ Copy`)? If we did this, that would not give us a way to explicitly avoid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The +
at the beginning of the line here is interpreted as a list by markdown
blocks). Effectively, these conditions mean that the user could theoretically | ||
have refactored the code to use `&x` rather than `x.use`. | ||
|
||
An example of these elisions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to also add an example of x.use
-> &x
as `async use { ... }`, analogous to the use of `move`. (Note that `use` and | ||
`move` are mutually exclusive.) | ||
|
||
For any object referenced within the closure or block that a `move` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this a little bit confusing. You can probably write this better than me but I'd be more explicit about that the closure you're referring to is the "use" closure and I also think you wanted to say async block?. So maybe something like ...
For any object referenced within the closure or block that a `move` | |
For any object referenced within the use closure or use async block that a `move` |
Still my phrasing reads weird but I guess you'd get what I meant to say :).
In order to combat the ergonomic pain of cloning clones of /// Clone the target twice and return both as a tuple. This is a convenience
/// method for operations where two clones are needed, saving a let binding.
/// A common example is cloning an [`std::sync::Arc`] and sharing a cloned
/// reference between threads.
///
/// ```rust
/// use std::sync::Arc;
/// use std::thread::spawn;
///
/// let foo: Arc<i32> = Arc::default();
/// let (clone1, clone2) = foo.duplicate();
///
/// spawn(move || {
/// println!("clone1: {clone1}");
/// });
///
/// println!("clone2: {clone2}");
/// ```
pub trait Duplicate<T> {
fn duplicate(self) -> (T, T);
fn duplicate_from<V>(value: V) -> (Self, Self)
where
Self: From<V> + Clone,
{
let item = Self::from(value);
(item.clone(), item.clone())
}
fn duplicate_from_default() -> (Self, Self)
where
Self: Default + Clone,
{
Self::duplicate_from(Self::default())
}
}
impl<T> Duplicate<T> for T
where
T: Clone,
{
fn duplicate(self) -> (T, T) {
(self.clone(), self.clone())
}
} |
@marziply the point of this RFC is removing all let binding boilerplates for a closure to capture-by-(shallow)-clone, I'm not sure how let (clone1, clone2) = foo.duplicate();
// ^^^ BTW for generating multiple copies of an Arc at once, the (unstable) function #![feature(array_repeat)]
use std::{array, sync::Arc};
fn main() {
let foo = Arc::new(1_u32);
let [a, b, c, d, e] = array::repeat(foo); // <--
assert_eq!(Arc::strong_count(&a), 5);
} In stable Rust we could use let [a, b, c, d, e] = array::from_fn(move |_| foo.clone()); |
(NOT A CONTRIBUTION) Unlike most commenters on this thread, I do think giving "normal type semantics" (meaning they can be used any number of times) to some types that are fairly cheap to copy but not by memcpy is a very good idea. However, like most other commenters, I think this RFC is not the solution to that problem. My impression of it is that it is an attempt to split the baby: on the one hand, there are people (like me) who think implicitly incrementing refcounts is fine and want to eliminate this clutter; on the other hand, there are people (like most people who engage on RFC threads) who disagree for whatever reason. This RFC satisfies neither group. Fundamentally, I think the concept of this RFC is misguided: the design dispute is not at all about the syntax for "using a type without moving it." I think everyone is perfectly happy with how you use a value without moving it for types that support using without moving them: you just use them, like literally every other language, no special operator needed. The design dispute is about which types can be used without moving them: one party wants it to be strictly aligned with types that can be used by executing memcpy, another party wants some types which requires some other code execution. This RFC is a solution to a problem Rust doesn't have. Accepting this RFC, in addition to making neither party happy, would introduce a whole additional load of complexity on users who now need to understand the difference between using a |
My comment was a means to justify this feature - I'm not fond of having to clone in this way, the trait in the example is a means to an end so if it were possible to clone syntactically rather than semantically then that would be my preference. The example was an illustration of how common clones are with channels and threads, not necessarily as a way to subvert current syntax. Your alternatives to my implementation are probably better than mine, no doubt, but my point was that in my opinion, this should be a problem solved with syntax, not semantics. That's why I'm particularly fond of |
@marziply Sorry. I thought "that specific syntax" meant " In any case, as other mentioned this RFC is conflating two features (capturing by clone i.e. Using #3680 (comment) syntax, let foo: Arc<i32> = Arc::default();
spawn(use(clone foo) || {
println!("clone1: {foo}"); // *not* foo.use
});
println!("clone2: {foo}"); |
I do agree that this RFC represents more than one problem as others have mentioned. I just wanted to chime into the usefulness I personally would find with a feature like this! |
I really do like the approach of I'd be in favour of having the All types implementing This This snippet should work, the compiler should insert all of the calls to let x = Arc::new(3);
somethig_async(async move { x; }).await;
let y = x;
consume_arc(x); I believe that this solution would satisfy the two groups of users: The ones like me that want implicit cloning of refcounted types and the ones that consider cloning I'm not in favour of the |
could you expand on this? the way I was imagining it So if you use |
This is off topic for this discussion (ergonomic ref counting) as that is a much different feature with different implications and additional design: fn f() {
mod m {
#![autoclone(Foo)]
pub struct Foo(Vec<u8>);
}
let x: m::Foo;
}
#![autoclone(Rc<_>)]
struct Foo(Rc<()>);
struct Bar((Rc<()>, u8));
Even if that was a good solution, I think that it is a very significant feature creep. |
With my proposal there's no such thing as "a type being autoclone". Autoclone is NOT a property of types, it's a property of code, ie functions. You're telling the compiler "within this piece of code, instead of raising // No autoclone enabled for the `blah` function.
fn blah() {
let x = Arc::new(());
let y = x;
drop(x); // error: use of moved value: `x`
drop(y);
}
#[autoclone(*)] // enable autoclone for the `meh` function
fn meh() {
let x = Arc::new(());
let y = x; // implicitly clones here.
drop(x); // no error!
drop(y);
}
#[autoclone(*)] // enable autoclone for all functions within this module
mod foo { ... } Similarly, within If you do So there's no need to expose autoclone in rustdoc at all. Autoclone is not a property of types, it's a setting that changes how code is compiled. if I enable autoclone in a lib crate, it has no effect in other crates. It's not public API. |
Honestly, that still feels like it poses a risk of creeping toward "I don't want to contribute to projects written in this dialect of C++" but for Rust. (Yes, rust-analyzer can claw back some ease of understanding of what is cloning when, but Rust has historically been designed on the principle that an IDE or similar should not be required to paper over weaknesses in the language's design.) In short, it feels like it's breaking from the "code is read much more often than it's written" facet of Rust's design philosophy. |
Yes, that's the tradeoff. I'm not 100% convinced we should do autoclone or some other form of implicit cloning. The upside is much better ergonomics, the downside is now you have two Rust dialects. The main point I wanted to make in this thread is that IMO any solution that uses a trait So if we do some form of light/implicit syntax for cloning, it should be controlled by an attribute (puts the choice in the crate writing the code), not a trait (puts the choice in the crate defining the type). |
I can certainly agree with that. If it's trait-based, as a downstream consumer, I want something I can NOTE: I say this as an application developer, not a low-level developer. I came to Rust for the compile-time correctness... but I really do like being able to stave off the creeping resource bloat that comes from so much on my desktop migrating to garbage-collected languages and webtech frameworks, and part of that is remaining aware of the dynamics of my code... such as when reference counts are getting incremented. |
based on the numbers people are pulling out for |
Personally, I'd be very much in favour of, at minimum, a To me, this whole EDIT: To clarify, I've been on the "Programmers just focus on their goals and I pay the 'de facto memory leak' price." side of things like garbage collection and automatic reference counting far too often. I like that Rust puts road bumps in the way of things that make it easy to fall off the fast/efficient path. It helps me to not burn out when I'm refusing to be a hypocrite on my own projects. |
aside from anything else: too much keyword overloading!! we already added the refcounting is frequently used by beginners that either can't figure out the borrow checker, or can't figure out how to restructure their code into something the borrow checker will understand. adding a third meaning of |
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