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

refactor uses of freevars to use the upvar list #60227

Closed
wants to merge 1 commit into from

Conversation

csmoe
Copy link
Member

@csmoe csmoe commented Apr 24, 2019

Address #60205

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 24, 2019
@csmoe csmoe force-pushed the upvar branch 3 times, most recently from 52173d5 to 4d33df4 Compare April 24, 2019 13:06
@cramertj
Copy link
Member

r? @nikomatsakis

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good so far!

.upvar_list[&def_id]
.iter()
.zip(substs.upvar_tys(def_id, cx.tcx))
.enumerate()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks about right =)

let upvar_capture = cx.tables().upvar_capture(upvar_id);
let temp_lifetime = cx.region_scope_tree.temporary_scope(closure_expr.hir_id.local_id);
let var_ty = cx.tables().node_type(var_hir_id);
let var_ty = cx.tables().node_type(upvar_id.var_path.hir_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it'd be better to pass the var_ty in as a parameter -- but this seems ok too

@bors
Copy link
Contributor

bors commented May 4, 2019

☔ The latest upstream changes (presumably #60462) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jun 2, 2019

☔ The latest upstream changes (presumably #61276) made this pull request unmergeable. Please resolve the merge conflicts.

@Mark-Simulacrum Mark-Simulacrum added S-inactive and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2019
@nikomatsakis
Copy link
Contributor

I'm going to close this PR -- @csmoe let's discuss later restarting this effort :)

@crlf0710
Copy link
Member

@rustbot modify labels to -S-inactive +S-inactive-closed

@rustbot rustbot added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-inactive labels Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants