Skip to content
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

Update closures for edition 2021 disjoint closure capturing #1521

Merged
merged 11 commits into from
Dec 11, 2024

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jul 8, 2024

This updates the closure documentation to incorporate the changes for RFC 2229 disjoint closure captures in Edition 2021.

This is a repost of #1059 with some changes that I applied. I'm still not 100% confident in this, but I think it is getting close.

Closes #1066

@ehuss
Copy link
Contributor Author

ehuss commented Jul 8, 2024

@traviscross and/or @JoelMarcey, would you be willing to give this a review to see if it makes sense to you?

Various changes:

  • Lots of changes and additions to fix correctness, and to add some definitions and clarity.
  • Added a section about how Copy types are captured by ImmBorrow. I'm not certain this is worth saying, because in effect it is an optimization which can only minimally be observed. I think the only observable difference is the size of the closure value (though I'm not sure). Is that worth including?
  • Added a section on the rightmost shared reference truncation. I'm also not certain this is worth saying, because I believe this is also just an optimization that reduces the size of the closure (also not sure if that's the only observable difference).
  • Added unions.
  • Updated unaligned section for closure field capturing: don't depend on alignment of packed fields rust#115315.
  • Removed the algorithm section. I'm not comfortable including this, in part because it isn't very clear, and it also has inaccuracies that I'm not motivated to fix. Also, I believe the algorithm actually implemented in rustc is a bit different, and I think trying to keep those in sync and correct will be challenging. I realize that pseudo-code can be very helpful for illustrating how it works, and I would like to see this eventually restored, but it might be worth exploring a slightly different approach.
  • Removed the "key examples". These are now illustrated above, or were incorrect.
  • I did not elaborate on the drop order, but I think we should. I'm not convinced it is a good idea to leave it as an unspecified order, since I suspect users will depend on the order, and it doesn't seem like something that could be changed without consequence. My random notes:

Here are some resources that might help with review:

If you want to experiment with your own tests, you can annotate closures with the #[rustc_capture_analysis] attribute to have rustc spit out the capture modes used in a closure. Capture analysis is done in two passes, and it will tell you what happens in the first and second pass.

I can try to answer questions or guide you to where things are implemented.

@JoelMarcey
Copy link
Contributor

JoelMarcey commented Jul 8, 2024

@ehuss I am happy to have a look. Out of curiosity, how related/out-of-date is https://doc.rust-lang.org/book/ch13-01-closures.html with respect to what is being specified here?

(I know you are specifying new stuff, but I am more curious if there are multiple disjointed places on closures in general or not)

@ehuss
Copy link
Contributor Author

ehuss commented Jul 9, 2024

I skimmed through the book chapter, and I don't see anything that is particularly out of date. It doesn't cover disjoint captures, but I'm not surprised since I would consider that a relatively advanced topic and I believe the book tries to avoid getting into those kinds of topics.

@traviscross traviscross changed the title Update closures for edition 2021 precise capturing Update closures for edition 2021 disjoint closure capturing Jul 9, 2024
@compiler-errors
Copy link
Member

I think the only observable difference is the size of the closure value (though I'm not sure). Is that worth including?

Definitely also affects auto traits.

@ehuss ehuss added the A-edition-2021 Area: Edition 2021 label Aug 22, 2024
@ehuss ehuss added the S-waiting-on-review Status: The marked PR is awaiting review from a maintainer label Dec 5, 2024
@traviscross traviscross added this pull request to the merge queue Dec 11, 2024
Merged via the queue into rust-lang:master with commit d6d24b9 Dec 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2021 Area: Edition 2021 S-waiting-on-review Status: The marked PR is awaiting review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2021: Update for disjoint captures (RFC 2229)
6 participants