reference icon indicating copy to clipboard operation
reference copied to clipboard

Update closures for edition 2021 disjoint closure capturing

Open ehuss opened this issue 1 year ago • 4 comments

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

This is a repost of https://github.com/rust-lang/reference/pull/1059 with some changes that I applied. I'm still not 100% confident in this, but I think it is getting close.

Closes https://github.com/rust-lang/reference/issues/1066

ehuss avatar Jul 08 '24 19:07 ehuss

@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 https://github.com/rust-lang/rust/pull/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:
    • The destructors chapter explicitly says "closure captures by move are dropped in an unspecified order.", but otherwise is not clear about drop order.
    • https://github.com/rust-lang/project-rfc-2229/issues/42 notes some subtleties about the drop order of precise captures. See this comment and the surrounding code for more information on how it is sorted.

Here are some resources that might help with review:

  • If you are interested in seeing the implementation, almost all of it is in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc_hir_typeck/src/upvar.rs and https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc_hir_typeck/src/expr_use_visitor.rs. Just don't read the comments there, since some of them are no longer correct (if I can, I'm going to try to update it).
  • Many of the tests for this feature are in https://github.com/rust-lang/rust/tree/master/tests/ui/closures/2229_closure_analysis.
  • There are various notes in https://github.com/rust-lang/project-rfc-2229, but I would not put too much weight in how up-to-date those are. However, they provide a lot of context about what people were thinking.
  • The original RFC is https://github.com/rust-lang/rfcs/blob/master/text/2229-capture-disjoint-fields.md, though similarly I would not put too much weight into its accuracy.

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.

ehuss avatar Jul 08 '24 19:07 ehuss

@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)

JoelMarcey avatar Jul 08 '24 22:07 JoelMarcey

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.

ehuss avatar Jul 09 '24 17:07 ehuss

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.

compiler-errors avatar Jul 10 '24 20:07 compiler-errors