-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Closures Capture Disjoint Fields #2229
Merged
Merged
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
7475f06
initial capture disjoint fields RFC
samsartor c154b90
capture disjoint drop case and rephrasing
samsartor 322a2e4
capture disjoint: mir question and error examples
samsartor b7bb50d
begin rewriting
samsartor 52c6eb4
major rewrite of reference section complete
samsartor f366f92
fixed a few nits and borrowck usage
samsartor 15f2eee
reference examples: add box, nll fixes, and typos
samsartor 4c8f1ae
attempt to clarify behavior of move
samsartor d30f678
final phrasing revisions
samsartor 395ced4
RFC 2229
Centril File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fixed a few nits and borrowck usage
- Loading branch information
commit f366f92abaca37cb7df9cb312b4341416073bf75
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,19 +101,19 @@ impl FirstDuplicated { | |
# Reference-level explanation | ||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
This RFC does not propose any changes to borrowck. Instead, the MIR generation | ||
for closures should be altered to produce the minimal capture. Additionally, a | ||
hidden `repr` for closures might be added, which could reduce closure size | ||
through awareness of the new capture rules *(see unresolved)*. | ||
|
||
In a sense, when a closure is lowered to MIR, a list of "capture expressions" | ||
is created, which we will call the "capture set". Each expression is some part | ||
of the closure body which, in order to capture parts of the enclosing scope, | ||
must be pre-evaluated when the closure is created. The output of the | ||
expressions, which we will call "capture data", is stored in the anonymous | ||
struct which implements the `Fn*` traits. If a binding is used within a | ||
closure, at least one capture expression which borrows or moves that binding's | ||
value must exist in the capture set. | ||
This RFC does not propose any changes to the borrow checker. Instead, the MIR | ||
generation for closures should be altered to produce the minimal capture. | ||
Additionally, a hidden `repr` for closures might be added, which could reduce | ||
closure size through awareness of the new capture rules *(see unresolved)*. | ||
|
||
In a sense, when a closure is lowered to MIR, a list of "capture expressions" is | ||
created, which we will call the "capture set". Each expression is some part of | ||
the closure body which, in order to capture parts of the enclosing scope, must | ||
be pre-evaluated when the closure is created. The output of the expressions, | ||
which we will call "capture data", is stored in the anonymous struct which | ||
implements the `Fn*` traits. If a binding is used within a closure, at least one | ||
capture expression which borrows or moves that binding's value must exist in the | ||
capture set. | ||
|
||
Currently, lowering creates exactly one capture expression for each used | ||
binding, which borrows or moves the value in its entirety. This RFC proposes | ||
|
@@ -140,10 +140,10 @@ Additionally capture expressions are not allowed to... | |
Note that *all* functions are considered impure (including to overloaded deref | ||
impls). And, for the sake of capturing, all indexing is considered impure *(see | ||
unresolved)*. It is possible that overloaded `Deref::deref` implementations | ||
could be marked as pure by using a new, unsafe marker trait (such as | ||
`DerefPure`) or attribute (such as `#[deref_transparent]`). In the meantime, | ||
`<Box as Deref>::deref` could be a special case of a pure function *(see | ||
unresolved)*. | ||
could be marked as pure by using a new, marker trait (such as `DerefPure`) or | ||
attribute (such as `#[deref_transparent]`). However, such a solution should be | ||
proposed in a separate RFC. In the meantime, `<Box as Deref>::deref` could be a | ||
special case of a pure function *(see unresolved)*. | ||
|
||
Note that, because capture expressions are all subsets of the closure body, | ||
this RFC does not change *what* is executed. It does change the order/number of | ||
|
@@ -176,7 +176,7 @@ let _a = &mut foo.a; | |
- `&mut foo.b` (used in entirety) | ||
- `&mut foo.a` (used in entirety) | ||
|
||
Borrowck passes because `foo.a`, `foo.b`, and `foo.c` are disjoint. | ||
The borrow checker passes because `foo.a`, `foo.b`, and `foo.c` are disjoint. | ||
|
||
```rust | ||
let _a = &mut foo.a; | ||
|
@@ -185,7 +185,7 @@ move || foo.b; | |
|
||
- `foo.b` (used in entirety) | ||
|
||
Borrowck passes because `foo.a` and `foo.b` are disjoint. | ||
The borrow checker passes because `foo.a` and `foo.b` are disjoint. | ||
|
||
```rust | ||
let _hello = &foo.hello; | ||
|
@@ -194,7 +194,7 @@ move || foo.drop_world.a; | |
|
||
- `foo.drop_world` (owned & implements drop) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. heh I'd recommend avoiding the use of |
||
|
||
Borrowck passes because `foo.hello` and `foo.drop_world` are disjoint. | ||
The borrow checker passes because `foo.hello` and `foo.drop_world` are disjoint. | ||
|
||
```rust | ||
|| println!("{}", foo.wrapper_thing.a); | ||
|
@@ -222,7 +222,7 @@ let _foo_again = &mut foo; | |
|
||
- `&mut foo.a` (used in entirety) | ||
|
||
Borrowck fails because `_foo_again` and `foo.a` intersect. | ||
The borrow checker fails because `_foo_again` and `foo.a` intersect. | ||
|
||
```rust | ||
let _a = foo.a; | ||
|
@@ -231,7 +231,7 @@ let _a = foo.a; | |
|
||
- `foo.a` (used in entirety) | ||
|
||
Borrowck fails because `foo.a` has already been moved. | ||
The borrow checker fails because `foo.a` has already been moved. | ||
|
||
```rust | ||
let _a = drop_foo.a; | ||
|
@@ -240,15 +240,15 @@ move || drop_foo.b; | |
|
||
- `drop_foo` (owned & implements drop) | ||
|
||
Borrowck fails because `drop_foo` can not be destructured + use of partially | ||
moved value. | ||
The borrow checker fails because `drop_foo` can not be destructured + use of | ||
partially moved value. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
This RFC does ruin the intuition that *all* variables named within a closure | ||
are captured. I argue that that intuition is not common or necessary enough to | ||
justify the extra glue code. | ||
This RFC does ruin the intuition that all variables named within a closure are | ||
*completely* captured. I argue that that intuition is not common or necessary | ||
enough to justify the extra glue code. | ||
|
||
# Rationale and alternatives | ||
[alternatives]: #alternatives | ||
|
@@ -277,8 +277,4 @@ places where the language could benefit? | |
- Should `Box` be special? | ||
|
||
- Drop order can change as a result of this RFC, is this a real stability | ||
problem? It is hard to imagine this breaking anything in the rust ecosystem. | ||
|
||
- Would lifetime changes be more breaking after NLL is stabilized? | ||
|
||
- How to avoid breaking changes if needed. | ||
problem? How should this be resolved? |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this second element of the capture set be
&mut foo.c
?That is, I assume this is a typo.
Or am I misunderstanding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a typo. Nice catch!