Skip to content
This repository has been archived by the owner on Apr 5, 2024. It is now read-only.

Ensure minimal places are captured #9

Closed
arora-aman opened this issue Jun 12, 2020 · 2 comments · Fixed by rust-lang/rust#78801
Closed

Ensure minimal places are captured #9

arora-aman opened this issue Jun 12, 2020 · 2 comments · Fixed by rust-lang/rust#78801

Comments

@arora-aman
Copy link
Member

If we need to capture a.b.c and a.b, then we only capture a.b

With/After #7

@arora-aman arora-aman changed the title Ensure minimal places are captured #11 Ensure minimal places are captured Jun 12, 2020
@arora-aman arora-aman assigned Azhng and null-sleep and unassigned Azhng and null-sleep Jun 13, 2020
@arora-aman
Copy link
Member Author

arora-aman commented Jul 19, 2020

During upvar analysis, we can keep a map from root variable to paths captured. As we see a new place that needs to be captured we can check if the root variable has a path that has been previously captured, if not just add it.

If the root variable has been captured we can iterate all captured paths and look for parent/child/sibling relations.
eg:

A B Relation What we keep
foo.x.y foo. x.z A is sibling of B, since paths diverged Both
foo.x.y foo.m A is sibling of B *, since paths diverged Both
foo.x foo. x.z A is parent of B A
foo.x.z foo. x A is child of B B
foo.x[20] foo. x[30] A == B** foo.x
foo.x[20].p foo. x[30].q A == B** foo.x
foo.x foo. x[30] A == B** foo.x

* foo.x.y and foo.m are not really siblings, but in our case it just indicates that we need to keep both
** We can't differentiate b/w two different array elements as being disjoint

Initially we can store the information as a mapping from root variable to list of pairs, where each pair is (Place, CaptureInfo).

To optimize answering these questions we can use Tries, but that should be after we stabalize this. It might just add overhead because I don't think we will be capturing crazy long paths, though might be useful if the closure captures a lot of different paths starting at the same root variable.

We also need to be careful about borrow kinds
eg:

let mut rect= Rectangle { top_left: Point { x: 10, y: 20 }, bottom_right: Point { x: 40, y: 50 } };
let c = || { 
      rect.top_left.x += 10;
      println!("{:?}", rect.top_left);
}

We will capture rect.top_left.x as mutable borrow and rect.top_left as immutable borrow. Overall we want to capture rect.top_left as mutable borrow. This would be similar as handling different kinds of use of the same place.

@nikomatsakis
Copy link
Contributor

Status update in #7 (comment)

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Nov 14, 2020
…akis

RFC-2229: Implement Precise Capture Analysis

### This PR introduces
- Feature gate for RFC-2229 (incomplete) `capture_disjoint_field`
- Rustc Attribute to print out the capture analysis `rustc_capture_analysis`
- Precise capture analysis

### Description of the analysis
1. If the feature gate is not set then all variables that are not local to the closure will be added to the list of captures. (This is for backcompat)
2. The rest of the analysis is based entirely on how the captured `Place`s are used within the closure. Precise information (i.e. projections) about the `Place` is maintained throughout.
3. To reduce the amount of information we need to keep track of, we do a minimization step. In this step, we determine a list such that no Place within this list represents an ancestor path to another entry in the list.  Check rust-lang/project-rfc-2229#9 for more detailed examples.
4. To keep the compiler functional as before we implement a Bridge between the results of this new analysis to existing data structures used for closure captures. Note the new capture analysis results are only part of MaybeTypeckTables that is the information is only available during typeck-ing.

### Known issues
- Statements like `let _ = x` will make the compiler ICE when used within a closure with the feature enabled. More generally speaking the issue is caused by `let` statements that create no bindings and are init'ed using a Place expression.

### Testing
We removed the code that would handle the case where the feature gate is not set, to enable the feature as default and did a bors try and perf run. More information here: rust-lang#78762

### Thanks
This has been slowly in the works for a while now.
I want to call out `@Azhng` `@ChrisPardy` `@null-sleep` `@jenniferwills` `@logmosier` `@roxelo` for working on this and the previous PRs that led up to this, `@nikomatsakis` for guiding us.

Closes rust-lang/project-rfc-2229#7
Closes rust-lang/project-rfc-2229#9
Closes rust-lang/project-rfc-2229#6
Closes rust-lang/project-rfc-2229#19

r? `@nikomatsakis`
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 17, 2020
RFC-2229: Implement Precise Capture Analysis

### This PR introduces
- Feature gate for RFC-2229 (incomplete) `capture_disjoint_field`
- Rustc Attribute to print out the capture analysis `rustc_capture_analysis`
- Precise capture analysis

### Description of the analysis
1. If the feature gate is not set then all variables that are not local to the closure will be added to the list of captures. (This is for backcompat)
2. The rest of the analysis is based entirely on how the captured `Place`s are used within the closure. Precise information (i.e. projections) about the `Place` is maintained throughout.
3. To reduce the amount of information we need to keep track of, we do a minimization step. In this step, we determine a list such that no Place within this list represents an ancestor path to another entry in the list.  Check rust-lang/project-rfc-2229#9 for more detailed examples.
4. To keep the compiler functional as before we implement a Bridge between the results of this new analysis to existing data structures used for closure captures. Note the new capture analysis results are only part of MaybeTypeckTables that is the information is only available during typeck-ing.

### Known issues
- Statements like `let _ = x` will make the compiler ICE when used within a closure with the feature enabled. More generally speaking the issue is caused by `let` statements that create no bindings and are init'ed using a Place expression.

### Testing
We removed the code that would handle the case where the feature gate is not set, to enable the feature as default and did a bors try and perf run. More information here: rust-lang#78762

### Thanks
This has been slowly in the works for a while now.
I want to call out `@Azhng` `@ChrisPardy` `@null-sleep` `@jenniferwills` `@logmosier` `@roxelo` for working on this and the previous PRs that led up to this, `@nikomatsakis` for guiding us.

Closes rust-lang/project-rfc-2229#7
Closes rust-lang/project-rfc-2229#9
Closes rust-lang/project-rfc-2229#6
Closes rust-lang/project-rfc-2229#19

r? `@nikomatsakis`
@nikomatsakis nikomatsakis added this to the Feature complete milestone Feb 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants