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

Use hir::Place in closure_captures #7

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

Use hir::Place in closure_captures #7

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

Comments

@arora-aman
Copy link
Member

arora-aman commented Jun 12, 2020

Ensure that field/member are marked borrowed/moved instead of variable hir_id.

This would look something like https://github.com/rust-lang/project-rfc-2229/blob/master/closure_captures.md

@arora-aman arora-aman added this to To-do in RFC-2229 via automation Jun 12, 2020
@arora-aman
Copy link
Member Author

arora-aman commented Jun 12, 2020

Depends on #12 and #2

@arora-aman
Copy link
Member Author

arora-aman commented Jun 12, 2020

closure_captures generated at: https://github.com/rust-lang/rust/blob/28946b3486d507418b8a4acb92d5e2baae193d65/src/librustc_typeck/check/upvar.rs#L115

Related to InferBorrowKind defined in rustc_typeck/check/upvar.rs

and might require adjust_upvar_captures to use a Place similar to closure_captures

This is then called from expr_use_visitor: https://github.com/rust-lang/rust/blob/28946b3486d507418b8a4acb92d5e2baae193d65/src/librustc_typeck/expr_use_visitor.rs#L554

@arora-aman arora-aman changed the title Update typechk expr_use_visitor Typechk: Store capture info based on Place Jun 12, 2020
@arora-aman arora-aman changed the title Typechk: Store capture info based on Place Use hir::Place in closure_captures Jun 12, 2020
@arora-aman arora-aman moved this from To-do to In progress in RFC-2229 Jun 20, 2020
@arora-aman arora-aman self-assigned this Jun 20, 2020
arora-aman added a commit to sexxi-goose/rust that referenced this issue Jul 6, 2020
Needed to support rust-lang/project-rfc-2229#7

This resembles the MIR equivalent located in `lbrustc_middle/mir`.

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Jennifer Wills <wills.jenniferg@gmail.com>
Co-authored-by: Logan Mosier <logmosier@gmail.com>
arora-aman added a commit to sexxi-goose/rust that referenced this issue Jul 8, 2020
Needed to support rust-lang/project-rfc-2229#7

Currently rustc_typeck depends on rustc_middle for definition TypeckTables, etc.
For supporting project-rfc-2229#7, rustc_middle would've to depend on
rustc_typeck for Place -- introducing a circular dependcy.

This resembles the MIR equivalent of `Place` located in `lbrustc_middle/mir`.

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Jennifer Wills <wills.jenniferg@gmail.com>
Co-authored-by: Logan Mosier <logmosier@gmail.com>
@arora-aman arora-aman assigned Azhng and unassigned logmosier and jenniferwills Jul 14, 2020
arora-aman added a commit to sexxi-goose/rust that referenced this issue Jul 16, 2020
Needed to support rust-lang/project-rfc-2229#7

Currently rustc_typeck depends on rustc_middle for definition TypeckTables, etc.
For supporting project-rfc-2229#7, rustc_middle would've to depend on
rustc_typeck for Place -- introducing a circular dependcy.

This resembles the MIR equivalent of `Place` located in `lbrustc_middle/mir`.

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Jennifer Wills <wills.jenniferg@gmail.com>
Co-authored-by: Logan Mosier <logmosier@gmail.com>
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 17, 2020
…atsakis

Move hir::Place to librustc_middle/hir

Needed to support rust-lang/project-rfc-2229#7

Currently rustc_typeck depends on rustc_middle for definition TypeckTables, etc.
For supporting project-rfc-2229#7, rustc_middle would've to depend on
rustc_typeck for Place -- introducing a circular dependency.

This resembles the MIR equivalent of `Place` located in `lbrustc_middle/mir`.

Separate PR for this move will make the actual PR for using Places to represent captures cleaner/more focused.

r? @nikomatsakis  @matthewjasper
arora-aman added a commit that referenced this issue Jul 31, 2020
Breif details about implementation:
- Instead of pre-initializing `closure_captured` and `upvars_capture_map` using `upvars_mentioned` we will be rely on use within the closure. This will be handled as part of #7 
- For coercing a closure to FnPtr, instead of relying on `upvars_mentioned` in typechk we will accept such code in typechk while setting a no-capture obligation that will be validated in trait selection post capture analysis.
arora-aman added a commit that referenced this issue Jul 31, 2020
Breif details about implementation:
- Instead of pre-initializing `closure_captured` and `upvars_capture_map` using `upvars_mentioned` we will be rely on use within the closure. This will be handled as part of #7 
- For coercing a closure to FnPtr, instead of relying on `upvars_mentioned` in typechk we will accept such code in typechk while setting a no-capture obligation that will be validated in trait selection post capture analysis.
@arora-aman arora-aman moved this from In progress to In review in RFC-2229 Sep 24, 2020
@arora-aman arora-aman moved this from In review to In progress in RFC-2229 Sep 24, 2020
@nikomatsakis
Copy link
Contributor

Current status:

  • Outlined the data structures to store minimum structures
  • Implemented analysis to compute minimum structures
  • Bridge code that creates the minimum captures (preserves current behavior for let _ = x when feature gate is not present)
  • How is it being tested? Gone through individual tests by hand and checked that they're fine thus far.

Action items:

  • To select new algorithm, should add feature gate capture_disjoint_fields and use that to decide instead of environment variable.
  • Testing system based on the rustc_variance-like attributes, where you would have a rustc_minimum_captures attribute that dumps the minimum captures for each closure in the form of special warnings or errors.

@arora-aman arora-aman moved this from In progress to In review in RFC-2229 Nov 6, 2020
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`
RFC-2229 automation moved this from In review to Done Nov 17, 2020
@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
RFC-2229
  
Done
Development

Successfully merging a pull request may close this issue.

6 participants