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

Improve logging of capture analysis #19

Closed
arora-aman opened this issue Oct 20, 2020 · 0 comments · Fixed by rust-lang/rust#78801
Closed

Improve logging of capture analysis #19

arora-aman opened this issue Oct 20, 2020 · 0 comments · Fixed by rust-lang/rust#78801
Assignees
Projects

Comments

@arora-aman
Copy link
Member

arora-aman commented Oct 20, 2020

Discussion happened here:

Capture analysis happens in two passes:

  1. Collecting information about each place, i.e. the CaptureKind (ByValue/ByRef) and the expression that led to such capture.
  2. Minimum capture analysis, where we find the set of the smallest set of Places (and associated CatureKind) that need to be captured to meet the same requirements as collected by 1.

If we take an example

let s: String;  // hir_id_s
let mut p: Point; // his_id_p  
let c = || {
       println!("{}, s");  // L1
       p.x += 10;  // L2
       println!("{}" , p.y) // L3
       println!("{}, p) // L4
       drop(s);   // L5
};

After 1. the structure we will have is:

{
      Place(base: hir_id_s, projections: [], ....) -> (L5, ByValue),
      Place(base: hir_id_p, projections: [Field(0, 0)], ...) -> (L2, ByRef(MutBorrow))
      Place(base: hir_id_p, projections: [Field(1, 0)], ...) -> (L3, ByRef(ImmutBorrow))
      Place(base: hir_id_p, projections: [], ...) -> (L4, ByRef(ImmutBorrow))

After 2

{
      hir_id_s -> [
           Place(base: hir_id_s, projections: [], ....) -> (L4, ByValue)
      ],
      hir_id_p -> [
           Place(base: hir_id_p, projections: [], ...) -> (L2, ByRef(MutBorrow)),
       ]

The structures in reality look a bit more complex. We want to print them out to stderr in a nicer fashion.

What should the output look like

The Place can be printed as <variable_name>[<projecitons>], where projections is a comma-separated list of pretty printed projection kinds.

Deref -> Deref
Field(u32, VariantIndex) -> (x.y); where x corresponds to u32, y corresponds to VariantIndex
Index -> Index
Subsclice -> Subsclice

For each capture, we want to throw an error at the expression that resulted in the capture along with the Place and the CaptureKind.

let s: String;
let mut p: Point;
let c = || {
       println!("{}, s");
       p.x += 10;
       // ^ ERROR: Capturing p[(0.0)] -> MutBorrow
       println!("{}" , p.y);
       // ^ ERROR: Capturing p[(1.0)] -> ImmBorrow
       println!("{}", p);
       // ^ ERROR: Capturing p[] -> ImmBorrow
       drop(s);
       // ^ ERROR: Capturing s[] -> ByValue
};

When this gets extended to min capture

let c = || {
       println!("{}, s");
       p.x += 10;
       // ^ ERROR: Capturing p[(0.0)] -> MutBorrow
       // ^ ERROR: MinCapture p[] -> MutBorrow
       println!("{}" , p.y);
       // ^ ERROR: Capturing p[(1.0)] -> ImmBorrow
       println!("{}", p);
       // ^ ERROR: Capturing p[] -> ImmBorrow
       drop(s);
       // ^ ERROR: Capturing s[] -> ByValue
       // ^ ERROR: MinCapture s[] -> ByValue
};
@arora-aman arora-aman added this to To-do in RFC-2229 via automation Oct 20, 2020
@arora-aman arora-aman moved this from To-do to In progress in RFC-2229 Oct 20, 2020
@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.