Stream: t-compiler/wg-rfc-2229

Topic: freevar refactoring #60205


nikomatsakis (Apr 23 2019 at 18:24, on Zulip):

OK, well, @csmoe I wrote up some mentoring instructions here for one case. Let me know if that makes any sense to you. =) (And if you still have time.)

lqd (Apr 23 2019 at 18:32, on Zulip):

the link is probably this instead

nikomatsakis (Apr 23 2019 at 18:44, on Zulip):

sigh

csmoe (Apr 24 2019 at 05:58, on Zulip):

@nikomatsakis WIP https://github.com/rust-lang/rust/pull/60227

csmoe (Apr 24 2019 at 06:52, on Zulip):

the closure's node-id -- we can get that by calling as_local_node_id on the closure def-id (which the calling function already has)

as_local_node_id returns Option<NodeId>, so how to deal with None?

ange (Apr 24 2019 at 07:00, on Zulip):

@csmoe I think it would be an internal compiler error if that failed?

csmoe (Apr 24 2019 at 07:01, on Zulip):

csmoe I think it would be an internal compiler error if that failed?

sounds reasonable :slight_smile:

nikomatsakis (Apr 25 2019 at 16:15, on Zulip):

unwrap :)

nikomatsakis (Apr 25 2019 at 16:15, on Zulip):

Well, it depends

nikomatsakis (Apr 25 2019 at 16:15, on Zulip):

But yeah in this case I think unwrap is fine -- basically, the None indicates a def-id from some other crate, but we are only running this code on closures in the current crate

csmoe (May 14 2019 at 03:15, on Zulip):

@nikomatsakis sorry for the delay, I was very busy in the past few days.
As the master branch removed with_freevars and seemed already refactored to what we wanna . https://github.com/rust-lang/rust/blob/a9ec99f4201ec33026a468ef1289f98a95b4d71a/src/librustc_mir/hair/cx/expr.rs#L519
so this issue should be marked done?

nikomatsakis (May 17 2019 at 20:01, on Zulip):

@csmoe lol .. ok :)

nikomatsakis (May 17 2019 at 20:01, on Zulip):

well,

nikomatsakis (May 17 2019 at 20:01, on Zulip):

there are other locations, have you looked into those?

nikomatsakis (May 17 2019 at 20:02, on Zulip):

actually, hmm

nikomatsakis (May 17 2019 at 20:02, on Zulip):

so tcx.upvars is .. not necessarily what we want

nikomatsakis (May 17 2019 at 20:02, on Zulip):

I think that's just a "nicer" version of with_freevars that @eddyb introduced -- but what we really want is to be getting our information from the results of type check

eddyb (May 20 2019 at 12:30, on Zulip):

@nikomatsakis to bring you up to speed: I have an almost-done refactor that, in the end, will remove Def::Upvar and compute tcx.upvars from HIR (so that HIR lowering can do funky things with closures and rustc_resolve doesn't need to know about it)

eddyb (May 20 2019 at 12:31, on Zulip):

it's plausible that we could remove tcx.upvars completely, and rely on TypeckTables, as MC and EUV run during or after typeck, and should always have access to those tables

eddyb (May 20 2019 at 12:34, on Zulip):

note that right now, the upvar_list (which IMO should just be named upvars) in TypeckTables is not very different from tcx.upvars and therefore I will likely not switch uses of one to the other

nikomatsakis (Jun 03 2019 at 20:20, on Zulip):

@eddyb is this in a PR that's available to view?

eddyb (Jun 03 2019 at 20:22, on Zulip):

it's merged, lol

eddyb (Jun 03 2019 at 20:23, on Zulip):

@nikomatsakis enjoy https://github.com/rust-lang/rust/pull/61276

nikomatsakis (Jun 03 2019 at 20:23, on Zulip):

thanks :)

eddyb (Jun 03 2019 at 20:24, on Zulip):

also, https://github.com/rust-lang/rust/pull/61413

eddyb (Jun 03 2019 at 20:24, on Zulip):

which is ready, huh

nikomatsakis (Jun 03 2019 at 21:02, on Zulip):

I should probably try to revise the instructions here in light of that

csmoe (Jun 06 2019 at 14:29, on Zulip):

I should probably try to revise the instructions here in light of that

@nikomatsakis would you mind updating the instruction after the compiler meeting? I'm lost with the refactoring.

nikomatsakis (Jun 07 2019 at 19:32, on Zulip):

yeah I will try to do so! might take a bit though

Last update: Nov 17 2019 at 07:00UTC