RFC for convenient, explicit closure capture using move($expr) expressions

[nikomatsakis]

View all comments

Add move($expr) syntax inside closures, async blocks, and generators. A move($expr) evaluates the expression at closure-creation time and captures the result by value. This gives precise control over what a closure captures and when, without needing temporary variables outside the closure.

A prototype implementation is available thanks to @TaKO8Ki

Desired feedback:

• Choice of syntax

Important

Since RFCs involve many conversations at once that can be difficult to follow, please use review comment threads on the text changes instead of direct comments on the RFC.

If you don't have a particular section of the RFC to comment on, you can click on the "Comment on this file" button on the top-right corner of the diff, to the right of the "Viewed" checkbox. This will create a separate thread even if others have commented on the file too.

Rendered

[nikomatsakis]

This has been the subject of much prior discussion. I'm nominating for brief lang-team discussion in our upcoming Wednesday meeting.

@rustbot label +I-lang-nominated

[kennytm]

Suggested change

	let output_tx = move(output_tx);
	let mut output_tx = move(output_tx);

This is used in line 96 as &mut output_tx so output_tx itself must be mut.

This raises a question, what happens in the case we don't list the captures separately?

|| process(&move(input.vec), &mut move(output_tx), move(&cx.data));

I suppose this would error unless output_tx in the parent scope is also declared as mut?

And similarly, if the content of move is an rvalue (i.e. not a place expression), taking &mut should be always allowed?

|| f(&mut move({ output_tx }))

View changes since the review

[nikomatsakis]

Thanks, fixed the original point.

[nikomatsakis]

I suppose this would error unless output_tx in the parent scope is also declared as mut?

Actually, I missed this observation, I would have expected it to compile, but it's worth stating explicitly either way (rvalues are generally mutable, and move() is an rvalue).

[nikomatsakis]

Desired feedback: choice of syntax

View changes since this review

[nikomatsakis]

Desired feedback: choice of syntax

I'm not convinced that move($expr) is the right syntax. Here are some alternatives keywords I've considered:

• capture($expr) (new keyword, though)
• super($expr) (heavily overloaded keyword, I generally prefer move as it is already associated with closure capture rules, but open to others' takes)

And then there is the question of () vs {} (thanks @juntyr for raising it earlier!)

• move($expr)
• move { $expr }

[nikomatsakis]

The primary goals I think are clarity of semantics -- I am not sure if move conveys that well or not. capture strikes me as the most clear, but the potential conflict with existing code may be a major issue.

I suppose that capture { $expr } could be done as a "contextual keyword" and is unlikely to conflict with existing structs. I generally dislike contextual keywords.

[steffahn]

IMO, move(x) is bad because it’s too similar to C++’s std::move(x) but it does something entirely different.

Also regarding move { $expr }, I think that

move {
    block_contents()
    …
}

might look too similar to

move || {
    block_contents()
    …
}

---

I personally would like a new keyword like capture. I’m not a big fan of recycling keywords for entirely different purposes, anyway. move for closures was to indicate that implicit closure captures should all be done by moving. This new thing is for explicit closure captures (and fully flexible w.r.t. whether things are captured by moving, borrowing, or other preprocessing steps).

I’m unsure about style of parentheses. Both capture($expr) and capture { $expr } seem reasonable. The latter is perhaps syntactically more noticeable without syntax highlighting. The only downside I could see is that “capture” is a bit long.

[kennytm]

I suppose that capture { $expr } could be done as a "contextual keyword" and is unlikely to conflict with existing structs.

Haven't we learnt from try { ... } 😅? As an expression capture cannot just be a contextual keyword.

And yes struct capture with a small c does exist.

[nikomatsakis]

The question is not so much whether it exists as whether it's widely used; it would require a new edition obviously but the transition is possible.

[teohhanhui]

As a user this feels more natural to me:

move || foo(super { expr.clone() })

super seems like the natural fit to refer to the outer scope.

move is too overloaded in the context of closures.

And { ... } feels more like "scope", whereas (...) really feels like function call syntax.

[Diddykonga]

Kinda orthogonal, but I remember this being mentioned with syntax similar to this before:

move(a, mut b) || {}

Was there a decision to not re-use/extend the implicit part? I thought it made it more clear as to the effect happening at creation-time, rather then during body-execution.

[kennytm]

Was there a decision to not re-use/extend the implicit part? I thought it made it more clear as to the effect happening at creation-time, rather then during body-execution.

This is answered in line 390 "Why this design over explicit capture clauses?"

[epage]

My concern with name(expr) is that there is no graphical hint that evaluation order is being affected. This feels like a major gotcha in reading Rust code. I assume there were similar arguments around expr.await. I suspect that await has a bit more innocuous of an effect on understanding control flow, particularly when it exists inside of an async block / fn. Here, something from inside the middle of a closure is actually running as part of the closure's construction. There are no hints to expect this when looking at the top, requires full inspection to identify the presence, and is likely less common / familiar for people approaching Rust such that they would likely guess wrong what is happening.

The closest syntactic analog feels like a closure itself which is why I lean towards name { expr } or even name || { expr } (e.g. capture || { data.clone() })

[epage]

The idea of closures being quasiquotation (#3968 (comment)) is also a good analogy for what could inspire the syntax (which I guess would be closest to name { expr }?)

[steffahn]

I think, if closure syntax is viewed as a form of quasiquotation, then these move(…) expressions basically just antiquotation, right? I’m not sure what the best concrete prior art to cite on this would be but I believe the comparison is quite useful.

View changes since the review

[nikomatsakis]

I didn't add anything for this, I wouldn't exactly call it prior art, but I guess we could add the comparison.

[nikomatsakis]

I've pushed a number of edits in response to comments made by folks on the RFC. I'm going to "resolve" the relevant conversations that are now captured in FAQs etc.

[scottmcm]

Might not need to be this PR, but if one is intending to be fully-explicit it would be nice to have a "capture nothing I didn't write a capture(foo) expression for" opt-in on the closure syntax so that if you're not actually fully-explicit you can get an error about it and find out, rather than accidentally capture something.

View changes since the review

[Skgland]

I find this a bit confusing.

move(move(v.clone()))
          ^^^^^^^^^ argument expression of inner move
     ^^^^^^^^^^^^^^^^ inner move and argument expression of outer move
^^^^^^^^^^^^^^^^^^^^^ outer move

I would expect the outer move to capture move(v.clone()) from inside the outer closure.
In the outer closure I would then expect the inner move to capture v.clone() from the surrounding context.

View changes since the review

[Skgland]

Maybe it would help to show the desugaring in two steps

|| {
    || {
        move(move(v.clone())).len()
    }
}

desugars as:

|| {
    let tmp_inner = move(v.clone());
    move_capture(tmp_inner) || {
        tmp_inner.len()
    }
}

desugars as:

{
    let tmp_outer = v.clone();
    move_capture(tmp_outer) || {
        let tmp_inner = tmp_outer;
        move_capture(tmp_inner) || {
            tmp_inner.len()
        }
    }
}

[kennytm]

IMO nested move() warrants at minimum a complexity-group clippy lint, it is too complicated for human consumption, and should better be split into multiple non-nested move() as

|| {
    let tmp_inner = move(v.clone());
    || {
        move(tmp_inner).len()
    }
}

[scottmcm]

(Not a question) Just wanted to say that I really like this section 👍

(Makes me ponder having the template include something like "Is there something you'd put in a Rust-by-Example page about your feature?" to prompt more bits like this in other RFCs.)

View changes since the review

[Mark-Simulacrum]

I'll selfishly flag this comment as well - #3680 (comment) :)

There's some possibly useful responses after that I think, though I believe they broadly align with where this RFC's concrete proposal has landed at.

View changes since the review

[scottmcm]

nit: While it's in some ways obvious, it might be nice to include a note here about nested closures, since of course nested ones aren't exactly evaluated in source order because it depends when the inner closures run.

View changes since the review

[scottmcm]

Another excellent call-out! Agreed that a "warning up-front to a linear reader" is important here.

I think this specifically pushes me towards braces in the syntax, because typically (perhaps unintentionally) things that affect the meaning of the whole scope are in braces, like how it's unsafe { ... } because it affects the scope not the resulting value.

And thus being || capture { foo.clone() }.clone() with braces here makes sense to me.

View changes since the review

[scottmcm]

Question: what's the closure capture behaviour of a capture expression in a nested thunk?

For example, if I have

let my_vec = ...;
foo(async {
    bar(|| move(my_vec.len()))
});

Is my_vec captured by reference in the outer thunk?

View changes since the review

[scottmcm]

Is it a place expression for things like ptr::addr_of!?

View changes since the review

[Nadrieril]

It should be yea, the explanatory desugaring suggests so. addr_of is &raw const anyway, and &raw const move(..) surely should work like &move(..).

EDIT: I hadn't read the section in question, expecting the answer to be "it's always a place expression", see my comment below

[Nadrieril]

I think move(..) must be a place expression. With what you suggest, &mut move($expr) would create a temporary to store the contents of move($expr):

let mut tmp = move($expr);
&mut tmp

which means that the mutation is not preserved across calls to the closure. In fact here if the type of tmp is not Copy, you can't call the closure twice.

Moreover, value-to-place coercion doesn't happen on the lhs of an assignment, yet we want this to work:

let mut x = 42;
let f = || move(x) += 1;

View changes since the review

[Nadrieril]

This is further justified by the proposed desugaring: it says the move(..) expression is replaced by a local variable, which is a place expression. (if I may shill my own work) Mine replaces it with a struct field, also a place expression.

[TaKO8Ki]

Now the feature gate is move_expr. We can fix it later if necessary.

Code: https://github.com/rust-lang/rust/blob/485ec3fbcc12fa14ef6596dabb125ad710499c9e/compiler/rustc_feature/src/unstable.rs#L642

The Unstable Book: https://doc.rust-lang.org/beta/unstable-book/language-features/move-expr.html

View changes since the review

[epage]

I suspect the existence of &move(x.clone()) would be surprising to people who might instead write move(x.clone()).clone(). If possible, might be worth a clippy lint.

View changes since the review