Rust: Tracking issue for RFC 2132: copy_closures/clone_closures

Created on 11 Sep 2017  路  21Comments  路  Source: rust-lang/rust

This is a tracking issue for the RFC "Copy/Clone Closures" (rust-lang/rfcs#2132).

Steps:

B-RFC-approved B-unstable C-tracking-issue E-mentor T-lang WG-epoch final-comment-period

Most helpful comment

@rfcbot fcp merge

I propose to stabilize the clone_closures and copy_closures features, first implemented in https://github.com/rust-lang/rust/pull/44551.

The following examples demonstrate that the feature is working as intended.

The RFC specifies that non-move closures which do not mutate captured variables are always Copy and Clone: Playground link

The RFC specifies that non-move closures which mutate captured variables are neither Copy nor Clone: Playground link

The RFC specifies that move closures are only Copy or Clone if the values they capture are Copy or Clone: Playground link

One mentioned drawback of the RFC is that cloned move closures which mutate variables will mutate independently of the other copies of the same closure. This is intuitive if you think about each copy of the closure as being its own state machine, but it can be surprising. In order to mitigate this, there is ongoing work on an MIR-based Copy lint to detect cases where copies might cause surprising changes here. I don't believe this issue will arise often enough in practice that we should block stabilization on it.

All 21 comments

I believe that @scalexm has implemented this RFC already. =)

@nmatsakis

That was for tuples, not for closures.

@arielb1 I was referring to #44551

(I think we can tick the first box)

What do we want to do around stabilization here? @cramertj was floating the idea of stabilizing only the clone impl for closures, not the copy one, at least until such time as we have a workable "unexpected copy" lint that would push you to invoke clone when copying closures (but that lint probably wants a bit of design).

Personally, I think I'd be ok stabilizing copy/clone closures in their entirety, but we probably want to see a bit more usage in the wild.

Can anyone report on using this feature?

I've been messing with it in some personal stuff, and I've really enjoyed having the feature. I'd be in favor of stabilizing both Copy and Clone.

Any plans to extend this to support cloning generators as well?

I haven't used this directly yet, but I can definitely report that people have complained to me that we don't have this feature on stable yet because it's unnecessarily forced them to change their design. It solves one of those irritating "when you want it, you really want it" problems, imo. :)

What's the status on this? Is it ready to stabilize once docs are ready? I'm planning a rewrite of some closure-related docs in the reference, so if this is going in, I'll include that.

@alercah yes basically. @cramertj -- would you be up for doing a quick stabilization report? (showing the tests in place and highlighting how any unresolved questions were resolved)

@rfcbot fcp merge

I propose to stabilize the clone_closures and copy_closures features, first implemented in https://github.com/rust-lang/rust/pull/44551.

The following examples demonstrate that the feature is working as intended.

The RFC specifies that non-move closures which do not mutate captured variables are always Copy and Clone: Playground link

The RFC specifies that non-move closures which mutate captured variables are neither Copy nor Clone: Playground link

The RFC specifies that move closures are only Copy or Clone if the values they capture are Copy or Clone: Playground link

One mentioned drawback of the RFC is that cloned move closures which mutate variables will mutate independently of the other copies of the same closure. This is intuitive if you think about each copy of the closure as being its own state machine, but it can be surprising. In order to mitigate this, there is ongoing work on an MIR-based Copy lint to detect cases where copies might cause surprising changes here. I don't believe this issue will arise often enough in practice that we should block stabilization on it.

Team member @cramertj has proposed to merge this. The next step is review by the rest of the tagged teams:

  • [x] @aturon
  • [x] @cramertj
  • [x] @eddyb
  • [x] @nikomatsakis
  • [x] @nrc
  • [x] @pnkfelix
  • [ ] @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

"The RFC specifies that non-move closures which do not mutate captured variables are always Copy and Clone"

"The RFC specifies that non-move closures which mutate captured variables are neither Copy nor Clone"

This omits the case where a capture of a Clone type is made by move into a non-move closure. In this case, the closure will be Clone but not Copy. See https://play.rust-lang.org/?gist=710b5c376ae8338867869c0e62f579d2&version=nightly.

I believe this is the correct behaviour, but wanted to draw attention to it anyway.

Yes, you're correct-- I should have said non-move closures which do not mutate or move captured variables are copy.

:bell: This is now entering its final comment period, as per the review above. :bell:

The final comment period is now complete.

I marked as E-mentor -- looks like we're ready for a stabilization PR here, right? There are instructions on the forge.

The reference is already updated. RBE probably doesn't need updating, the stdlib could maybe use mention on Copy and Clone traits, and I'm unsure about the book. @steveklabnik?

@alercah sounds right

Stabilization PR (with mention in docs for Clone and Copy): https://github.com/rust-lang/rust/pull/49299

49299 is merged, I think this is it. Let me know if I forgot something relevant to this tracking issue.

Was this page helpful?
0 / 5 - 0 ratings