Implement reborrow for closure captures by arora-aman · Pull Request #82007 · rust-lang/rust

arora-aman · 2021-02-11T21:25:19Z

The strategy for captures is detailed here with examples: https://hackmd.io/PzxYMPY4RF-B9iH9uj9GTA

Key points:

We only need to reborrow a capture in case of move closures.
- If we mutate something via a &mut we store it as a MutBorrow/UniqueMuBorrow of the path containing the &mut,
- Similarly, if it's read via & ref we just store it as a ImmBorrow of the path containing the & ref.
- If a path doesn't deref a &mut, &, then that path is captured by Move.
- If the use of a path results in a move when the closure is called, then that path is truncated before any deref and the truncated path is moved into the closure.
In the case of non-move closure if a use of a path results in a move, then the path is truncated before any deref and the truncated path is moved into the closure.

Note that the implementation differs a bit from the document to allow for truncated path to be used in the ClosureKind analysis that happens as part of the first capture analysis pass.

Closes: rust-lang/project-rfc-2229#31

r? @nikomatsakis

rust-highfive · 2021-02-11T21:25:21Z

Updates src/tools/cargo.

cc @ehuss

rust-highfive · 2021-02-11T21:25:23Z

⚠️ Warning ⚠️

These commits modify submodules.

arora-aman · 2021-02-11T21:26:37Z

Hmm fixing the cargo.lock and submodule issues 🙃

LingMan · 2021-02-12T09:00:18Z

compiler/rustc_typeck/src/check/upvar.rs

A bit shorter with Iterator::position:

Suggested change

for (i, proj) in place.projections.iter().enumerate() {

match proj.kind {

ProjectionKind::Deref => {

// We only drop Derefs in case of move closures

// There might be an index projection or raw ptr ahead, so we don't stop here.

place.projections.truncate(i);

return place;

}

_ => {}

}

}

place

if let Some(i) = place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref) {

// We only drop Derefs in case of move closures

// There might be an index projection or raw ptr ahead, so we don't stop here.

place.projections.truncate(i);

}

place

Edit: Removed unhelpful temporary.

Thank you, didn't know this was a thing. I'll update the PR at once, after the PR's been completely reviewed

nikomatsakis · 2021-02-12T20:39:32Z

This looks great. Go ahead and change to position() if you want.

@bors delegate+

bors · 2021-02-12T20:39:34Z

✌️ @arora-aman can now approve this pull request

nikomatsakis · 2021-02-12T20:39:46Z

After that you can tell bors r=nikomatsakis

arora-aman · 2021-02-13T17:56:23Z

@bors r=nikomatsakis

bors · 2021-02-13T17:56:24Z

📌 Commit 20788d666da8881ff3207e18967402c8e70bb7ed has been approved by nikomatsakis

bors · 2021-02-15T01:48:33Z

☔ The latest upstream changes (presumably #82103) made this pull request unmergeable. Please resolve the merge conflicts.

arora-aman · 2021-02-16T05:05:11Z

@bors r=nikomatsakis

bors · 2021-02-16T05:05:12Z

📌 Commit f99e152 has been approved by nikomatsakis

Implement reborrow for closure captures The strategy for captures is detailed here with examples: https://hackmd.io/PzxYMPY4RF-B9iH9uj9GTA Key points: - We only need to reborrow a capture in case of move closures. - If we mutate something via a `&mut` we store it as a `MutBorrow`/`UniqueMuBorrow` of the path containing the `&mut`, - Similarly, if it's read via `&` ref we just store it as a `ImmBorrow` of the path containing the `&` ref. - If a path doesn't deref a `&mut`, `&`, then that path is captured by Move. - If the use of a path results in a move when the closure is called, then that path is truncated before any deref and the truncated path is moved into the closure. - In the case of non-move closure if a use of a path results in a move, then the path is truncated before any deref and the truncated path is moved into the closure. Note that the implementation differs a bit from the document to allow for truncated path to be used in the ClosureKind analysis that happens as part of the first capture analysis pass. Closes: rust-lang/project-rfc-2229#31 r? `@nikomatsakis`

Implement reborrow for closure captures The strategy for captures is detailed here with examples: https://hackmd.io/PzxYMPY4RF-B9iH9uj9GTA Key points: - We only need to reborrow a capture in case of move closures. - If we mutate something via a `&mut` we store it as a `MutBorrow`/`UniqueMuBorrow` of the path containing the `&mut`, - Similarly, if it's read via `&` ref we just store it as a `ImmBorrow` of the path containing the `&` ref. - If a path doesn't deref a `&mut`, `&`, then that path is captured by Move. - If the use of a path results in a move when the closure is called, then that path is truncated before any deref and the truncated path is moved into the closure. - In the case of non-move closure if a use of a path results in a move, then the path is truncated before any deref and the truncated path is moved into the closure. Note that the implementation differs a bit from the document to allow for truncated path to be used in the ClosureKind analysis that happens as part of the first capture analysis pass. Closes: rust-lang/project-rfc-2229#31 r? ``@nikomatsakis``

Implement reborrow for closure captures The strategy for captures is detailed here with examples: https://hackmd.io/PzxYMPY4RF-B9iH9uj9GTA Key points: - We only need to reborrow a capture in case of move closures. - If we mutate something via a `&mut` we store it as a `MutBorrow`/`UniqueMuBorrow` of the path containing the `&mut`, - Similarly, if it's read via `&` ref we just store it as a `ImmBorrow` of the path containing the `&` ref. - If a path doesn't deref a `&mut`, `&`, then that path is captured by Move. - If the use of a path results in a move when the closure is called, then that path is truncated before any deref and the truncated path is moved into the closure. - In the case of non-move closure if a use of a path results in a move, then the path is truncated before any deref and the truncated path is moved into the closure. Note that the implementation differs a bit from the document to allow for truncated path to be used in the ClosureKind analysis that happens as part of the first capture analysis pass. Closes: rust-lang/project-rfc-2229#31 r? ```@nikomatsakis```

Rollup of 8 pull requests Successful merges: - rust-lang#77728 (Expose force_quotes on Windows.) - rust-lang#80572 (Add a `Result::into_ok_or_err` method to extract a `T` from `Result<T, T>`) - rust-lang#81860 (Fix SourceMap::start_point) - rust-lang#81869 (Simplify pattern grammar, improve or-pattern diagnostics) - rust-lang#81898 (Fix debug information for function arguments of type &str or slice.) - rust-lang#81972 (Placeholder lifetime error cleanup) - rust-lang#82007 (Implement reborrow for closure captures) - rust-lang#82021 (Spell out nested Self type in lint message) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

pthariensflame · 2021-04-06T08:04:18Z

Is this visible in the stable language? If so, does it need relnotes?

arora-aman · 2021-04-06T11:00:58Z

This is behind the capture_disjoint_feilds feature gate, so not part of stable yet. That said I'm unsure what all goes in relnotes, so here is some context:

If I recall correcly this increases the precision of the capture (for non-unsafe code) which was restricted while trying to support move of a path that uses accesses reference in #80092. That PR was 1.51 based on the associated milestone.

nikomatsakis · 2021-04-09T16:34:23Z

relnotes usually sticks to stable stuff

rust-highfive assigned nikomatsakis Feb 11, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 11, 2021

This comment has been minimized.

Sign in to view

arora-aman force-pushed the reborrow branch from 222b1c3 to 1a5b1e8 Compare February 11, 2021 22:12

LingMan reviewed Feb 12, 2021

View reviewed changes

nikomatsakis approved these changes Feb 12, 2021

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2021

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 15, 2021

arora-aman added 4 commits February 15, 2021 22:00

Implement reborrow for closure captures

b86c5db

Handle restricting closure origin

e39c3c0

Treat read of COpy types via refs as not move in move-closure

1b86ad8

Use iter::position in truncate_capture_for_move

f99e152

arora-aman force-pushed the reborrow branch from 20788d6 to f99e152 Compare February 16, 2021 03:01

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 16, 2021

Dylan-DPC-zz mentioned this pull request Feb 16, 2021

Rollup of 13 pull requests #82186

Closed

Dylan-DPC-zz mentioned this pull request Feb 16, 2021

Rollup of 11 pull requests #82200

Closed

Dylan-DPC-zz mentioned this pull request Feb 17, 2021

Rollup of 11 pull requests #82201

Closed

Dylan-DPC-zz mentioned this pull request Feb 17, 2021

Rollup of 8 pull requests #82241

Merged

bors merged commit 0b2f2b9 into rust-lang:master Feb 18, 2021

rustbot added this to the 1.52.0 milestone Feb 18, 2021

m-ou-se mentioned this pull request Sep 3, 2021

🎉 Rust 2021 celebration and thanks #88623

Closed

-    for (i, proj) in place.projections.iter().enumerate() {
-        match proj.kind {
-            ProjectionKind::Deref => {
-                // We only drop Derefs in case of move closures
-                // There might be an index projection or raw ptr ahead, so we don't stop here.
-                place.projections.truncate(i);
-                return place;
-            }
-            _ => {}
-        }
-    }
-    place
+    if let Some(i) = place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref) {
+        // We only drop Derefs in case of move closures
+        // There might be an index projection or raw ptr ahead, so we don't stop here.
+        place.projections.truncate(i);
+    }
+    place

Uh oh!

Conversation

arora-aman commented Feb 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Feb 11, 2021

Uh oh!

rust-highfive commented Feb 11, 2021

Uh oh!

arora-aman commented Feb 11, 2021

Uh oh!

This comment has been minimized.

LingMan Feb 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arora-aman Feb 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikomatsakis commented Feb 12, 2021

Uh oh!

bors commented Feb 12, 2021

Uh oh!

nikomatsakis commented Feb 12, 2021

Uh oh!

arora-aman commented Feb 13, 2021

Uh oh!

bors commented Feb 13, 2021

Uh oh!

bors commented Feb 15, 2021

Uh oh!

arora-aman commented Feb 16, 2021

Uh oh!

bors commented Feb 16, 2021

Uh oh!

pthariensflame commented Apr 6, 2021

Uh oh!

arora-aman commented Apr 6, 2021

Uh oh!

nikomatsakis commented Apr 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

arora-aman commented Feb 11, 2021 •

edited

Loading

LingMan Feb 12, 2021 •

edited

Loading

arora-aman Feb 12, 2021 •

edited

Loading