Improve documentation on closure types. #249
Conversation
There was a problem hiding this comment.
First off, thank you oh so very much for working on this very complex corner of the language. I truly am grateful for this PR (and most PRs).
That said, there's some minor nits and small edits I've suggested.
And also, I am sorry it took sooo long to actually review this. Weather has been inclement where I live which lead to me getting more work hours to cover for those who couldn't get in. And then I was too tired or distracted to look at this.
src/types.md
Outdated
| closures*, can be coerced to function pointers (`fn`) with the matching | ||
| signature. To adopt the example from the section above: | ||
| The compiler prefers to capture a closed-over variable by immutable borrow, | ||
| followed by mutable borrow and finally by move (or copy, for [`Copy`] types). It |
There was a problem hiding this comment.
Might read better if the list was four long and by copy was in there before by move.
src/types.md
Outdated
| closures*, can be coerced to function pointers (`fn`) with the matching | ||
| signature. To adopt the example from the section above: | ||
| The compiler prefers to capture a closed-over variable by immutable borrow, | ||
| followed by mutable borrow and finally by move (or copy, for [`Copy`] types). It |
There was a problem hiding this comment.
Need a comma before the and.
src/types.md
Outdated
| closure to outlive the captured values, such as if the closure is being returned | ||
| or used to spawn a new thread. | ||
|
|
||
| Structs and tuples are always captured entirely, not by individual fields. It |
There was a problem hiding this comment.
"Composite types, such as structs or tuples, are"? Or are enums somehow different?
src/types.md
Outdated
|
|
||
| If, instead, the closure were to use `self.vec` directly, then it would attempt | ||
| to capture `self` by mutable reference. But since `self.set` is already | ||
| borrowed to iterate over, the closure would not compile. |
src/types.md
Outdated
| * A closure which does not mutate or move out of any captured variables | ||
| implements `[Fn]`, indicating that it can be called by shared reference. | ||
|
|
||
| > Note that `move` closures may still implement `[Fn]` or `[FnMut]`, even |
There was a problem hiding this comment.
Note needs to have a colon after it to remain consistent with the rest of the notes in the reference.
src/types.md
Outdated
| @@ -400,6 +471,33 @@ let bo: Binop = add; | |||
| x = bo(5,7); | |||
| ``` | |||
|
|
|||
| ### Other traits | |||
|
|
|||
| Closure types implement the following traits, if allowed to do so by the | |||
src/types.md
Outdated
| @@ -400,6 +471,33 @@ let bo: Binop = add; | |||
| x = bo(5,7); | |||
| ``` | |||
|
|
|||
| ### Other traits | |||
|
|
|||
| Closure types implement the following traits, if allowed to do so by the | |||
There was a problem hiding this comment.
The actual values captured don't matter, but the types of the variables do. I'd personally write it "Closures implement the following traits conditionally on the types of the captured variables."
There was a problem hiding this comment.
That's a fair point. But it also doesn't necessarily depend on the type of the variable captured, but the type of the capturing variable in the closure. For instance, capturing a non-Copy type by reference is fine for Copy, since the reference is itself Copy.
There was a problem hiding this comment.
Found a way to make this work, please take a look once I push.
src/types.md
Outdated
| Closure types implement the following traits, if allowed to do so by the | ||
| captured values: | ||
|
|
||
| * `[Sized]` |
There was a problem hiding this comment.
It feels weird that we're talking about conditionally implemented traits and then the first one on the list is always implemented.
src/types.md
Outdated
| * `[Sized]` | ||
| * `[Send]` | ||
| * `[Sync]` | ||
| * `[Clone]` |
There was a problem hiding this comment.
Given how Clone and Copy are lang items and Send and Sync aren't (I think? Maybe one of them is?), I'd rather see Clone and Copy listed first.
There was a problem hiding this comment.
Sync is, Send isn't. I think it would look better reordered though, so I will do that.
src/types.md
Outdated
| * `[Clone]` | ||
| * `[Copy]` | ||
|
|
||
| `[Sized]` is always implemented (local variables are all sized, so all captured |
There was a problem hiding this comment.
More of a meta-point, but I really don't like having parentheticals like this. They should generally just be another sentence.
There was a problem hiding this comment.
Removed that sentence anyway.
This PR more fully documents closure types, including the mechanics of captures and the traits implemented by them. I intend to work on closure expressions as well, either in this PR or a follow-up, but felt this was a good place to put them.
I didn't fully explain capturing here, as I feel like that's better for closure expressions. The bit about no field captures might make more sense there as well, but I think the mechanics of how captures are typed and the implications on the traits belong here.
This PR is written for a post-RFC 2132 world, where closures have
CloneandCopy, and intended as part of the stabilization docs for that RFC. The tracking issue is rust-lang/rust#44490. That bit could easily be split out though.Fixes #219, fixes #229.