Skip to content
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

[DONT MERGE] Test min capture #78762

Closed
wants to merge 11 commits into from

Conversation

arora-aman
Copy link
Member

@arora-aman arora-aman commented Nov 5, 2020

New capture analysis for RFC 2229.

  • This PR is to just test the entire bors try suite passes if the feature was to be enabled by default.
  • Know issue: let _ = <Place> statements aren't handled properly in closures yet.
  • Modified some ui tests to bypass the issue mentioned above.
  • Regions for captures maybe created in a different order than before, so updated ui tests to reflect that as well.

r? @ghost

@arora-aman arora-aman changed the title Test min capture [DONT MERGE] Test min capture Nov 5, 2020
@arora-aman arora-aman force-pushed the test_min_capture branch 2 times, most recently from d1e8c93 to bdedfb4 Compare November 5, 2020 06:55
arora-aman and others added 8 commits November 5, 2020 01:58
Co-authored-by: Archer Zhang <archer.xn@gmail.com>
Signed-off-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Dhruv Jauhar <dhruvjhr@gmail.com>
Co-authored-by: Dhruv Jauhar <dhruvjhr@gmail.com>
Co-authored-by: Chris Pardy <chrispardy36@gmail.com>
Co-authored-by: Logan Mosier <logmosier@gmail.com>
Co-authored-by: Chris Pardy <chrispardy36@gmail.com>
Co-authored-by: Logan Mosier <logmosier@gmail.com>
Co-authored-by: Jenny Wills <wills.jenniferg@gmail.com>
Co-authored-by: Aman Arora <me@aman-arora.com>
@lcnr
Copy link
Contributor

lcnr commented Nov 5, 2020

@bors try

@bors
Copy link
Contributor

bors commented Nov 5, 2020

⌛ Trying commit af945c1d7baf997e4d7100f647d3472b6dea3761 with merge 5aa51ad7810a1bb29155998d7121b19511cc80c4...

@bors
Copy link
Contributor

bors commented Nov 5, 2020

☀️ Try build successful - checks-actions
Build commit: 5aa51ad7810a1bb29155998d7121b19511cc80c4 (5aa51ad7810a1bb29155998d7121b19511cc80c4)

@m-ou-se m-ou-se added the S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. label Nov 5, 2020
@lqd
Copy link
Member

lqd commented Nov 5, 2020

@bors try

@bors
Copy link
Contributor

bors commented Nov 5, 2020

⌛ Trying commit c053d96 with merge 4ef9e8c67e8251f2b7d6a6657c2b8cdf2c13b4d2...

@lqd
Copy link
Member

lqd commented Nov 5, 2020

@rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Nov 5, 2020

☀️ Try build successful - checks-actions
Build commit: 4ef9e8c67e8251f2b7d6a6657c2b8cdf2c13b4d2 (4ef9e8c67e8251f2b7d6a6657c2b8cdf2c13b4d2)

@rust-timer
Copy link
Collaborator

Queued 4ef9e8c67e8251f2b7d6a6657c2b8cdf2c13b4d2 with parent 9d78d1d, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (4ef9e8c67e8251f2b7d6a6657c2b8cdf2c13b4d2): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 6, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request 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 pull request 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`
@arora-aman
Copy link
Member Author

Closing this since the #78801 got merged

@arora-aman arora-aman closed this Nov 17, 2020
@arora-aman arora-aman deleted the test_min_capture branch November 17, 2020 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants