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

Use min capture results everywhere #18

Closed
arora-aman opened this issue Oct 15, 2020 · 3 comments · Fixed by rust-lang/rust#82951
Closed

Use min capture results everywhere #18

arora-aman opened this issue Oct 15, 2020 · 3 comments · Fixed by rust-lang/rust#82951
Assignees
@logmosier @jenniferwills
Labels
captures-refactor
Projects
RFC-2229
Milestone
Feature complete

Comments

@arora-aman
Copy link
Member

arora-aman commented Oct 15, 2020 •
edited

Currently, min capture analysis results are written to MaybeTypeckResults, which is a transient data structure within rustc_typeck.

For the results to be available post-typeck they need to be written out to TypeckResults.

The writeback stage in rustc_typeck/src/check/writeback.rs needs to be updated to add the analysis results to the final results table.
@arora-aman arora-aman added this to To-do in RFC-2229 via automation Oct 15, 2020
@arora-aman arora-aman changed the title Write min capture results to the final TypechkResults Write min capture results to TypechkResults Oct 15, 2020
@arora-aman
Copy link
Member Author

arora-aman commented Oct 20, 2020 •
edited

This will also close rust-lang/rust#76005, since the span will available using tcx.hir().span(capture_info.expr_id)

@arora-aman arora-aman mentioned this issue Nov 6, 2020
Merged
@arora-aman
Copy link
Member Author

Note once this is implemented we can remove Span from ty::UpvarCapture::ByValue(span) since the capture information contains the expr id corresponding to that span

@arora-aman
Copy link
Member Author

For ease, we can TypeckResults to

  1. Add captured_places(closure_def_id: LocalDefId) -> impl Iterator<CapturedPlace<'tcx>> + 'tcx, this returns a the min_captures data structure flattened. (Look flat_map).
  2. Remove upvar_capture(), As far as I can see -- this shouldn't be needed since CapturedPlace contains both the CapturInfo and the Place.

@arora-aman arora-aman moved this from To-do to In progress in RFC-2229 Nov 13, 2020
arora-aman added a commit to sexxi-goose/rust that referenced this issue Nov 15, 2020
@arora-aman
Fix case when ExprUseVisitor is called after typeck writeback
bb8c5e5 
Clippy uses `ExprUseVisitor` and atleast in some cases it runs
after writeback.

We currently don't writeback the min_capture results of closure
capture analysis since no place within the compiler itself uses it.

In the short term to fix clippy we add a fallback when walking captures
of a closure to check if closure_capture analysis has any entries in it.

Writeback for closure_min_captures will be implemented in
rust-lang/project-rfc-2229#18
@arora-aman arora-aman mentioned this issue Dec 2, 2020
Merged
7 tasks
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 12, 2020
@bors
Auto merge of rust-lang#79553 - sexxi-goose:mir_min_cap_writeback, r=…
5bd9b60 
…nikomatsakis

Capture precise paths in THIR and MIR

This PR allows THIR and MIR to use the result of the new capture analysis to actually capture precise paths

To achieve we:
- Writeback min capture results to TypeckResults
- Move handling upvars to PlaceBuilder in mir_build
- Lower precise paths in THIR build by reading min_captures
- Search for ancestors in min_capture when trying to build a MIR place which starts off of an upvar

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

Partly implements: rust-lang/project-rfc-2229#18

Work that remains (not in this PR):
- [ ] [Known bugs when feature gate is enabled](https://github.com/rust-lang/project-rfc-2229/projects/1?card_filter_query=label%3Abug)
- [ ] Use min_capure_map for
  - [ ] Liveness analysis
  - [ ] rustc_mir/interpret/validity.rs
  - [ ] regionck
- [ ] rust-lang/project-rfc-2229#8
- [ ] remove closure_captures and upvar_capture_map

r? `@ghost`
@arora-aman arora-aman changed the title Write min capture results to TypechkResults Use min capture results everywhere Jan 6, 2021
@jenniferwills jenniferwills mentioned this issue Mar 9, 2021
Merged
@arora-aman arora-aman moved this from In progress to In review in RFC-2229 Mar 11, 2021
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 19, 2021
@bors
Auto merge of rust-lang#82951 - sexxi-goose:wr-mir-replace-methods2, …
cebc8fe 
…r=nikomatsakis

Replace closures_captures and upvar_capture with closure_min_captures

Removed all uses of closures_captures and upvar_capture and refactored code to work with closure_min_captures. This also involved removing functions that were no longer needed like the bridge.

Closes rust-lang/project-rfc-2229#18
r? `@nikomatsakis`
RFC-2229 automation moved this from In review to Done Mar 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@logmosier logmosier

@jenniferwills jenniferwills

Labels
captures-refactor
Projects
RFC-2229
  
Done
Milestone
Feature complete
Development

Successfully merging a pull request may close this issue.

Replace closures_captures and upvar_capture with closure_min_captures sexxi-goose/rust
4 participants
@nikomatsakis @arora-aman @logmosier @jenniferwills