# RFC 2229 Update

[Watch the recording](https://www.youtube.com/watch?v=mJbv4rM9Yt8)

The implementation for RFC 2229 has been progressing rapidly but we've encountered some interesting complications, especially around `move` closures (although the problems apply more generally).

[More detailed update from the group](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view)

## Implementation status

* [x] `![feature(capture_disjoint_fields)` works for simple cases
* [ ] "Naive" pass for migrations
* [ ] PR open for handling bugs around mutable references and restricting precison
* [ ] PRs open for improving diangostics (reporting precise) information back to the user
* [ ] Remove old data structures from the compiler

## Questions and conclusions from meeting

* Do we need a better syntax for migrations?
* Summary from meeting: no, assuming it's not that common
* How do we migrate around traits that are not satisfied by the closure anymore?
* some sketches below, we can introduce migrations for these if necessary (we should measure)
* for `Clone` there may be changes in behavior, not just failure to compile
* What should be the correct behavior in case of capture by move?
* no concerns raised about the behavior, nor extension to reborrow, for now at least
* what are some questions to try to answer
* What percentage of closures require `let x = x` and why?
* What percentage of closures change size and by how much?

## Rules as implemented today

* [Move closures don't capture derefs](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#1-Move-closure-don%E2%80%99t-capture-Derefs)
* [By value captures don't capture derefs](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#2-By-value-captures-don%E2%80%99t-capture-Derefs)
* [Don't capture anything more precise on top of raw pointers](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#3-Don%E2%80%99t-capture-anything-more-precise-on-top-of-Raw-ptrs)
* [Repr packed](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#4-Repr-packed)

## Why move closures don't capture derefs

* [Wrong behavior for references](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#Motivation), could fix with [reborrowing captures for move closures](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#5-Reborrowing-capture-possible-extension)
* [Performance concerns for box](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#Motivation)

## Migration plan

* Behavior changes in the following situation:
* a closure would have moved an entire variable (e.g., `x`) but now moves a more specific place (e.g., `x.y`)
* and other fields of `x` that are no longer moved have 'significant' destructors (e.g., `x.z`)
* In this scenario, the destructor for `x.z` runs when `x` goes out of scope, rather than when the closure is dropped
* Proposed fix:
* In such cases, we insert (via migraton lint) `let x = x;` into the closure
* If there are multiple such variables, we insert `let (a, b, c) = (a, b, c);`
* Improving accuracy:
* Permit annotation of destructors in the standard library that just free memory. Those destructors are not considered 'significant'.
* So e.g. if `x.z` is of type `String`, we would not insert `let x = x;` into the closure.

Observations and shortcomings:

* `let x = x` looks odd to be the recommended pattern, to be more specific
* non-obvious to figure out
* not obvious why it is there
* `let _ = x` looks similar but is very different :)
* could use `drop(x)` in some cases to make a more obvious desugaring
* we detect behavioral changes via these rules but it's also possible for closures to change size (sometimes dramatically), and that is not presently detected
* Auto traits: [Struct implements a Trait but subfield doesn’t](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#Struct-implements-a-Trait-but-subfield-doesn%E2%80%99t-Migration)
* MEETING NOTES:
* Could use a diagnostic
* Or could have special rules that look for cases where `w.0: !Send` but `w: Send` (also for `Sync` and maybe `Clone`) for the migration lint
* See notes about Clone below
* [Base of an index projection is always captured](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#Base-of-an-index-projection-is-always-captured) -- partly implementation, partly policy

Capturing subpaths can change the semantics of `closure.clone()`:

struct Foo { x: *mut u32 };

let foo: Foo;
let c = move || {
    // fix is to add `let foo = foo;`
    let y = foo.x;
};

c.clone(); // used to run `Foo::clone()` but now it runs `<*mut u32>::Clone`

We could detect cases where clone is implemented manually.

With specialization could be true for autotraits, but we don't have that yet.

## Other bugs

Notable bugs and limitations:

* [Place mentioned in the closure isn't entirely captured](https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view#Place-mentioned-in-the-closure-isn%E2%80%99t-entirely-captured) -- see also "captures and matches" below
* We may now capture many more paths than we used to, inflating the size of closures
* Recommendation: gather data
* Note that this is limited to non-move closures, unless we implement the reborrowing rule (which we probably will)

### Captures and matches

There are also some cases where the implementation is capturing more than you might think. These occur most notably around patterns, for example in cases like these:

let _ = x;
match x { _ => () }
let (_, _) = x;
match x { (_, _) => () }

We have tried to stick to the rule that "if the borrow checker considers this an access of type X, then the closure captures do too", for the most part. But right now when pattern matching against a place expression (e.g., `x` in the examples above), that place is always captured *at least* by read. This should be fixable with more refactoring, we've deferred the work.

Current behavor:

* `let _ = <place>` ICEs

Behavior we discussed last week:

* `let _ = <place>` will trigger a "fake capture" of `<place>`, as will `match`

Ultimate behavior:

* all of the above would not capture `x`