Clone into closures #2407
Comments
|
A c++-lambda-ish macro could do all that boilerplate for us. |
|
Lambda captures are one of the rare points where I think C++ did it better than Rust: they are explicit and you can express how to capture on a per-capture basis (in Rust that would be by reference, by Clone, by move, by some arbitrary expression (C++ allows |
|
I like the idea of For reference, C++-style explicit lambda captures were considered pre-1.0. IIRC the reason for what we have now was that it requires fewer language features- you can get the same effects via the combination of a) default capture-by-reference and b) move closures that can capture temporary bindings. In the limit it's not even very different syntactically: [x = abc, y = def, &z]() { ... x ... y ... z }{ let x = abc; let y = def; let z = &z; move || { ... x ... y ... z } }I think what tips the balance is that C++'s equivalent of The hard part is that |
|
Scala has a proposal something similar to C++'s explicit capturing annotations in the form of spores. I think the corresponding Rust syntax would look a lot like what @rpjohnst suggested above use std::sync::Arc;
use std::sync::Mutex;
use std::thread;
fn main() {
let foo = Arc::new(Mutex::new(0));
thread::spawn({ let foo = foo.clone(); move || {
let _x = foo.lock();
}});
let _y = foo.lock();
} |
|
While that syntax seems fine if you only need to manipulate a few variables, it gets quite hairy in situations like this. That's why I proposed |
Centril
added
the
T-lang
|
What about a clone_all macro? Hygiene stops at passed-in identifiers. A macro can take that identifier and create a let binding with it. You can have Maybe this macro should be built-in: https://play.rust-lang.org/?gist=f3e8daccce06320e831aeb29dc26ea5c&version=stable&mode=debug |
|
So this: let args = Arc::clone(args);
let quiet_matched = quiet_matched.clone();
let paths_searched = paths_searched.clone();
let match_line_count = match_line_count.clone();
let paths_matched = paths_matched.clone();
let bufwtr = Arc::clone(&bufwtr);
let mut buf = bufwtr.buffer();
let mut worker = args.worker();Becomes: let args = Arc::clone(args);
clone_all!(quiet_matched, paths_searched, match_line_count, paths_matched);
let bufwtr = Arc::clone(&bufwtr);
let mut buf = bufwtr.buffer();
let mut worker = args.worker();It should also be possible to modify the macro to take optional clone_all!(a, mut b, c); |
|
Aaand I just make a crate for it. because y'know, I just had to. https://crates.io/crates/clone_all |
|
There are many scenarios where you should use either In any case, you could address the syntax directly without special casing clone with becomes We avoid noise around the Ideally, you could do this via a |
|
There's this macro_rules! enclose {
( ($( $x:ident ),*) $y:expr ) => {
{
$(let $x = $x.clone();)*
$y
}
};
}Example use: |
|
@SoniEx2 would you consider adding this to your crate? https://gist.github.com/rust-play/1cc71ed137157bb8115a9f063aabbea3 It allows using clone_all! on |
|
@lukaspiatkowski I encourage ppl to fork the things I've made. The whole point of free software is the ability to fork things, so please fork my things! (I'm not sure why ppl don't fork things...) |
|
@SoniEx2 Sure, I never got the hang of it, every time I try to contribute to open source libraries the authors are not very helpful, so its hard to start doing so. I guess it comes with practice. I have send a pull request: https://bitbucket.org/SoniEx2/clone_all/pull-requests/1 |
|
This is a bit out of this issue's scope, but related: what if I want to selectively move variables into the closure? One example in which this came up today: let v = vec!["a", "b", "c"];
rayon::scope(|s| {
for i in 0..9 { // `i` does not live long enough
s.spawn(|_| {
println!("{}", v[i % 3])
});
} // borrowed value only lives until here
}); // borrowed value needs to live until here
println!("{:?}", v);I really don't want to move This is a very simple example, but as shown in some comments above, it's not unusual to come across situations in which the workarounds are quite verbose. There's a clear need (in my view) of more granularity. I don't have any good suggestions at the moment, but something analogous to C++'s capture list would be handy. This macro (linked above as well) is one way to handle this, but I really think there should be a solution built into the language. |
|
@cauebs { let v = &v;
move |...| {
/* use v, it will be a reference */
}
}This is how it looks in your example. It costs two extra lines, which is not great but also not horrible. |
|
Or nikomatsakis' suggested form of the same, which places the borrow in the closest possible location, right when providing the closure: rayon::scope(|s| {
for i in 0..10 {
s.spawn({
let v = &v;
move |_| println!("{}", v[i % 3])
});
}
});This may be what @RalfJung meant, but the borrow can occur later than shown so I was a bit confused. Normally I'd have expected to see their code as: rayon::scope(|s| {
let v = &v;
for i in 0..10 {
s.spawn(move |_| println!("{}", v[i % 3]));
}
}); |
|
@RalfJung As I said, it was a simple example. But imagine there were dozens of variables that I wanted to be captured by reference, and only a single one that I wanted to be moved. That one is the exception, and that should be expressable in the code. I shouldn't have to change the default (moving everything) and treat all the others as exceptions. I come from Python and have, with time, learned that if something is simple to explain (and reasonably common), it should be just as simple to encode. Otherwise, the language might be lacking some expressiveness. |
|
@cauebs AFAIK you can also do The idea of capture lists has been raised many times, but I've never seen a concrete syntax proposal that's significantly more concise than adding a few |
|
I think |
clone_all from rust-lang/rfcs#2407 added some idempotency tests
|
I think @Diggsey |
|
I independently ran into this myself. Is there a reason we wouldn't want to add a bit of extra syntax specifically for cloning bindings at the point of closure? The suggestion I thought up was a let clonable = ...;
// compiler assumes _.clone() for all bindings used within the closure
let c = clone || { clonable.do_something(); };
clonable.do_something_else();Also, I considered you might want to mix // compiler clones only the specified binding and moves the rest
let c = move + clone clonable || {clonable.do_something;};or possibly even cloning multiples: // compiler clones the specified bindings, and moves the rest
let c = move + clone{clonable1, clonable2} || {...etc...};The macro solution is interesting, but it feels to me more like a workaround to something that should be in the language. |
|
@AberrantWolf Using macros isn't considered a workaround, it's considered a good idiom. The smaller that the Rust language can be, the better. So if something can be implemented as a library, it's considered better to implement it as a library. Rust doesn't even have (non-const) global variables, but you can use the Macros are considered an integral part of Rust, and they are widely used for all sorts of things. They aren't a hack. I personally would be in favor of built-in support for this, but the bar for new syntax and features is very high. So rather than new syntax, it's more likely that a |
|
I suspect our existing syntax provides the only intuitive choice: There are no intuitive capture list proposals because they all increase language strangeness with new grouping operations. We discussed I believe We also danced around some There are good reasons for We already need an anti-clone lint for types like |
|
There's no way we would use the syntax let clone = false;
let bool_var = true;
let res = clone || bool_var;and we won't make Forms including a keyword like |
|
Maybe it is enough to use proc-macro with explicitly listed arguments instead of some syntax extensions? That approach already works pretty fine I think. Though, it is still requires |
|
I think Instead, I'd suggest some external crate provide a "rebind" macro that avoids the repetition under more "self-like" situations, ala We tie |
Bafflingly, only when |
|
How about having the compiler do the cloning when needed, by checking if the binding is used after having been moved into a closure? The behaviour could be gated with a keyword like edit: example syntax let foo = SomeCloneable::new();
thread::spawn(move, cloneused || {
let bar = foo; // foo is cloned due to being used later in it's original scope
});
let baz = foo; |
|
It would be nice to just have movable bindings. let foo = SomeCloneable::new();
let move foo = foo.clone();
thread::spawn(move || {
let bar = foo;
});
let baz = foo;instead of let foo = SomeCloneable::new();
{
let foo = foo.clone();
thread::spawn(move || {
let bar = foo;
});
}
let baz = foo; |
|
The precedent is already set that we change how arguments are captured by putting something in front of the lambda, If explicit is generally better than implicit, isn't it clearer to allow people to explicitly specify which things they want moved? |
|
Variable-specific syntax was discussed in https://rust-lang.github.io/rfcs/0114-closures.html#alternatives
|
|
I find that this issue comes up very very frequently, especially in UI land, and is the number one thing holding back Rust GUI libraries, and Rust GUI in general. People come from other ecosystems typically looking to build a GUI in this new language they just learned. GUIs and games are the most visual way of seeing your code come to life. The Yew, Dioxus, Sycamore, GTK, Iced, Druid libraries all struggle with teaching beginners about why this is necessary in the first place. This issue is personally my #1 problem with Rust. My proposal: A new trait like Right now, if you want a value to be moved into a closure with no ceremony, it must implement the Likewise, a trait Capture {
fn capture(&self) -> Self;
}
impl<T> Capture for Rc<T> { // and Arc
fn capture(&self) -> Self {
self.clone()
}
}Then, to take advantage of Using let val = Rc::new(thing);
let handler = capture |_| println!("{val}");This is especially helpful for the many async bits of the ecosystem: fn app(cx: Scope) -> Element {
let val = use_ref(&cx, || None);
// What we have to do today
cx.spawn({
let val = val.clone();
async move {
val.set(Some("thing"));
}
});
// What the future could be
cx.spawn(async capture {
val.set(Some("thing"));
});
}I personally don't see |
But types that are not
Copy has a clear definition, whereas Capture seems pretty arbitrary. How cheap is "cheap"? Why even have Capture at all if we have Clone? What if I have two things implementing Capture, and I want one of them to be cloned into the closure, and the other one moved? |
If a type cannot be
The recommendation should be that types that are cheaply cloneable are capture. For std, only Arc/Rc satisfy this criteria. By default, all
Because IMO Rust is in a weird position where we don't have a garbage collector, so it's taught that you should use RC when you don't have a clear owner of data. But, there is close to zero language support for RC and is made especially apparent in GUI land. In many ways, std acts like RC doesn't even exist, even though a large amount of programs (and many/most larger libraries) use it extensively. |
I don't believe the borrow checker is currently allowed to affect behaviour, only to reject otherwise-working programs. For example, mrustc skips borrow checking entirely, on the assumption that the code has already been validated by rustc. Invisibly invoking (an equivalent of) By comparison, partial borrowing is more or less "just" a precedence change, changing the interpretation of |
For example, this array is "silently" copied into two handlers. In fact, since it's fn main() {
let vals = [0; 10];
let handler1 = move || println!("{vals:?}");
let handler2 = move || println!("{vals:?}");
handler2();
handler1();
}The fn main() {
let vals = std::rc::Rc::new([0; 10]);
let handler1 = capture || println!("{vals:?}");
let handler2 = capture || println!("{vals:?}");
handler2();
handler1();
}I don't think the conditions are any more hidden than the fact the Additionally, nowhere in the https://doc.rust-lang.org/std/marker/trait.Copy.html In fact, since EDIT: I've created a thread at https://internals.rust-lang.org/t/a-capture-trait-for-cheaply-cloning-into-closures/16141 |
|
A lot of people are subscribed here. I think it is a good place to link to (discussions about) specific proposals, but not a good place to discuss individual proposals. @jkelleyrtp I suggest creating a thread on https://internals.rust-lang.org/. |
|
Here's another idea I wanted to throw out there for consideration: Perhaps this problem could be alleviated by adding an let autoclone foo = Foo::new(...); // Must implement `Clone`, otherwise error
thread::spawn(move || { // Clones `foo`
foo.baz();
});
takes_ownership(foo); // Clones `foo`
let bar = foo; // End of `foo`'s NLL-scope (it's never used again), no clone required: moves `foo`The let autoclone mut foo = Foo::new(..);This does introduce more magic. Especially in longer function bodies, one might come across the use of a variable Some further considerations:
|
|
I'm new to Rust and I don't know if this is related, but I think it has something to do with the topic of the current discussion and may bring new ideas. Correct me if I'm wrong. I am trying to implement a function, which takes The only solution which I came up with is pub fn desktop_files<I>(dirs: I) -> impl Iterator<Item = DesktopFile>
where
I: IntoIterator,
I::Item: AsRef<Path>,
{
dirs.into_iter()
.map(|d| {
WalkDir::new(&d)
.min_depth(1)
.follow_links(true)
.into_iter()
.filter_map(|entry| entry.ok())
.filter(|entry| entry.metadata().ok().map_or(false, |md| md.is_file()))
.map(|entry| entry.into_path())
.filter(|p| p.extension().map_or(false, |ext| ext == "desktop"))
.filter_map({
let d = d.as_ref().to_owned();
move |p| {
p.strip_prefix(&d).ok().map({
let d = d.clone();
move |sp| DesktopFile::new(d, sp)
})
}
})
})
.flatten()
.unique_by(|df| df.id())
} |
|
What about borrowing c++ syntax a bit? let (foo, bar, baz) = (1,2,3);
let closure = [foo, &bar, =baz] || {}; // move, reference, clone respectively
let task = async [&, mut foo, =mut baz] { }; // fallback capture = referenceJust kidding :| Quickly created a crate from above syntax. Applied weird syntax on declaring mutable variables, which makes let ss = std::rc::Rc::new(());
let [foo, bar, baz, mut qux] = std::array::from_fn(|_| ss.clone());
let _ = capture!(
[foo, *bar, &baz, &mut qux, cloned = bar.clone(), *other = cloned.clone(), oar = 3],
|| {
// foo, mut bar -> cloned
// cloned, mut other, oar -> created
// &baz, &mut qux -> referenced
// other unspecified variables -> moved
}
); |
|
I also made a crate: https://crates.io/crates/clown let foo = Arc::new(...);
thread::spawn(#[clown] || {
honk!(foo).baz();
});
foo.baz();expands to let foo = Arc::new(...);
thread::spawn({
let __honk_0 = (foo).clone();
move || {
__honk_0.baz();
}
});
foo.baz();pros compared to function-like approaches e.g. https://crates.io/crates/enclose
cons:
|
|
here's my take on it: I'm using build.rs to generate bunch of extension traits for tuples, only caveat is that method names are suffixed with https://github.com/Niedzwiedzw/to-cloning-closure/blob/master/src/generated.rs usage: #[test]
fn test_it_works() {
use to_cloning_closure::prelude::*;
let append_index_to = ("whatever".to_owned(),)
.with_cloned_1(|(whatever,), idx| format!("{whatever} - {idx}"));
let _ = (0..2137).map(append_index_to).collect::<Vec<String>>();
} |
|
What if we go in a different direction? Like, we can specify, how to capture something inside of a closure: Or generally anything that has signature for Also we can allow syntax Which, however, keyword to pick? and what's the value? Edit: also may be quite cryptic aka Edit 2: What's about Unresolved: It feels strange when such a thing is used as expression (from UX perspective): is it a one time capture or change of capture mode for a binding in scope? |
|
@tema3210 That feels as unsound as a language structure that runs code in front of a function and assigns their results to variables (like |
|
Actually @jgarvin's suggestion above sounds like the right compromise. We could just extend the current || uses(a) // borrows `a` mutably or immutably depending on requirement
move || uses(a) // moves `a` into closure
move(a) || uses(a) // same as `move || a`
move(&a) || uses(a) // same as `|| uses(a)`
move(a=&a) || uses(a) // same as `move(&a) || uses(a)`; `&a` and `&mut a` are specifically defined to be `a=`
move(a = a.clone()) || uses(a) // same as `{let a = a.clone(); move || a}`
move(a.clone())` // same as the last line, since this is an expression that only involves one local variableBut in that case, the where the last |
It is structure that customizes how a capture is made, that's it. Capture is a known concept after all. The UX however gets very weird very quick:
|
|
I really like @SOF3's suggestion. It's backwards compatible and ties neatly with the current syntax. I would like to see a formal RFC for it. If no one else wants to write it, I'd be happy to write one and see where it goes. As it stands, I don't think we're getting anywhere with discussion in this issue |
|
I would like to try writing the RFC here |
I thought about very similar thing while was reading the thread. Just with additional "clone" keyword for cloning. But I'm suspecting that adding new keyword is a very breaking change. Especially so common like 'clone'... |
and others
When working with
moveclosures, and especially when spawning new threads, I often find myself having to clone a bunch of state (usuallyArcs, but not always). This is either because I want that state to be shared between many instances of a closure or with the closure and the code below the closure (e.g.,Arc/Rc), or for mundane reasons like I need to have aStringin each closure, and sincethread::spawnrequires'static, I need to clone. In futures-based code, this happens a lot withRc<Self>where I need to carry self along into futures chained withand_then. Usually, this results in code like this:This appears a lot in the wild. Some very quick sampling yields: [1], [2], [3], [4], [5], [6], [7], [8].
Even if you're just spawning a single thread, you often need to use weirdly tweaked variable names. For example, this code:
must be written as
or if you want to retain the
fooname (which you often do), you need to introduce an extra artificial scope:I don't have a concrete proposal for how this would be fixed. It'd be really neat to have something like
clone ||, but that's probably overly simplistic as often you want to move some variables and clone others. Some better ergonomics for this would be great!The text was updated successfully, but these errors were encountered: