Stream: t-compiler/wg-rfc-2229

Topic: sync 2019-04-09


nikomatsakis (Apr 09 2019 at 18:03, on Zulip):

Hey all :wave:

nikomatsakis (Apr 09 2019 at 18:04, on Zulip):

cc @WG-rfc-2229

ange (Apr 09 2019 at 18:04, on Zulip):

hello

nikomatsakis (Apr 09 2019 at 18:05, on Zulip):

I didn't do anything since last time except hang out at a cabin on Punta del Diablo

blitzerr (Apr 09 2019 at 18:05, on Zulip):

Hopefully some good food :D

nikomatsakis (Apr 09 2019 at 18:05, on Zulip):

but I was just starting to go through @csmoe's list of free variables

blitzerr (Apr 09 2019 at 18:05, on Zulip):

I won't be able to make it to the meeting today. Sorry guys

nikomatsakis (Apr 09 2019 at 18:05, on Zulip):

Hopefully some good food :D

well, it's not so veggie friendly there, but we ate well :)

blitzerr (Apr 09 2019 at 18:06, on Zulip):

I will read it async, if Niko can drive it today ?

ange (Apr 09 2019 at 18:17, on Zulip):

@nikomatsakis any input on https://paper.dropbox.com/doc/Upvar-Paths--Aa7C6aefXL6RNSXHuUu8jncVAg-PNRgk22ryKq1n3Wq3FXAO would be most appreciated, fwiw

nikomatsakis (Apr 09 2019 at 18:17, on Zulip):

OK. Sorry, I was supposed to drive, but there is a contractor here and they pulled me away from sething

nikomatsakis (Apr 09 2019 at 18:17, on Zulip):

back now

ange (Apr 09 2019 at 18:18, on Zulip):

np :-)

nikomatsakis (Apr 09 2019 at 18:18, on Zulip):

I'll take a look at this

nikomatsakis (Apr 09 2019 at 18:18, on Zulip):

the upvar paths proposal

nikomatsakis (Apr 09 2019 at 18:18, on Zulip):

let me just do that now

ange (Apr 09 2019 at 18:18, on Zulip):

cool

nikomatsakis (Apr 09 2019 at 18:19, on Zulip):

@ange I'll just leave some comments in the dropbox paper

nikomatsakis (Apr 09 2019 at 18:19, on Zulip):

mostly requests for clarification :)

ange (Apr 09 2019 at 18:19, on Zulip):

sure

ange (Apr 09 2019 at 18:21, on Zulip):

fwiw, I'm mostly concerned about the MIR borrow checker ramifications, as I guess I'll need to spend some time on it to get a handle on the bigger picture

ange (Apr 09 2019 at 18:22, on Zulip):

i.e. whether temporaries are (a) needed (b) a good idea

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

@ange one question I have

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

in the branch you have

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

is there some kind of "output" -- i.e., I'm trying to figure out what information we can glean before we go through all the work of changing codegen

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

I guess I'd be interested to have a list of closures and, for each one, the set of paths that wind up being computed

ange (Apr 09 2019 at 18:24, on Zulip):

umm, yah, see https://github.com/aoikonomopoulos/rust/commit/04ec3f7ced8d8f2bdeade78c4574515f917536a7

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

ok, I see

ange (Apr 09 2019 at 18:24, on Zulip):

though I haven't posted examples

nikomatsakis (Apr 09 2019 at 18:25, on Zulip):

yeah, so, probably a good thing to do (as a start)

nikomatsakis (Apr 09 2019 at 18:25, on Zulip):

would be to come up with a good list of tricky examples

nikomatsakis (Apr 09 2019 at 18:25, on Zulip):

basically try to write-up interesting test cases

nikomatsakis (Apr 09 2019 at 18:25, on Zulip):

this is useful no matter what we do

nikomatsakis (Apr 09 2019 at 18:25, on Zulip):

(and probably should be added to the roadmap as a work item)

ange (Apr 09 2019 at 18:26, on Zulip):

my current ideas are in the testing section

ange (Apr 09 2019 at 18:26, on Zulip):

I'm more worried about getting things so wrong that I'd have to start over, which is why I'm more concerned with getting an end-to-end example working first

ange (Apr 09 2019 at 18:27, on Zulip):

i.e. get a very simple example of capturing a field sorta working

nikomatsakis (Apr 09 2019 at 18:28, on Zulip):

ok, I added test cases to the roadmap, I think it's roughly the same as what you had

ange (Apr 09 2019 at 18:28, on Zulip):

I'd feel more comfortable building on something that is unlikely to be swept away by reality

ange (Apr 09 2019 at 18:28, on Zulip):

great

nikomatsakis (Apr 09 2019 at 18:28, on Zulip):

I just thikn it'd be good to be creating the .rs files

nikomatsakis (Apr 09 2019 at 18:28, on Zulip):

we don't have to make them work yet :)

nikomatsakis (Apr 09 2019 at 18:28, on Zulip):

anyway, I guess I will have to review your code in a bit more depth

nikomatsakis (Apr 09 2019 at 18:29, on Zulip):

I agree that convering the MIR will be some work

nikomatsakis (Apr 09 2019 at 18:29, on Zulip):

it may not be that bad

ange (Apr 09 2019 at 18:29, on Zulip):

at my level of understanding of the rustc internals, I would be relying on the debug output to verify I'm exercising the path I think I'm exercising in a testcase :-)

nikomatsakis (Apr 09 2019 at 18:29, on Zulip):

a lot of that, I think, is diagnostics ..

ange (Apr 09 2019 at 18:29, on Zulip):

would temporaries be at all applicable?

nikomatsakis (Apr 09 2019 at 18:29, on Zulip):

basically what happens there is that when you access (e.g.) the upvar x in MIR, it gets converted to this.foo

ange (Apr 09 2019 at 18:30, on Zulip):

i.e. for capturing x.y, do

let t1 = &x.y;
env.0 = t1;
nikomatsakis (Apr 09 2019 at 18:31, on Zulip):

well I meant more from the inside of the closure

nikomatsakis (Apr 09 2019 at 18:31, on Zulip):

but that's also true :)

nikomatsakis (Apr 09 2019 at 18:31, on Zulip):

so I guess we will need to think about how to generalize that

nikomatsakis (Apr 09 2019 at 18:31, on Zulip):

so that instead of intercepting at the upvar, we intercept at the full captured path

nikomatsakis (Apr 09 2019 at 18:31, on Zulip):

that's gonna be the tricky bit

ange (Apr 09 2019 at 18:31, on Zulip):

right. what currently concerns me for the inside of the closure is rewriting the access to trim the path

ange (Apr 09 2019 at 18:32, on Zulip):

well I meant more from the inside of the closure

well, if it means we might not have to touch 200+ upvar references...

ange (Apr 09 2019 at 18:33, on Zulip):

to be clear, that's the reason I'm asking about temporaries. would they be at all viable?

nikomatsakis (Apr 09 2019 at 20:10, on Zulip):

@ange sorry, how would temporaries help exactly?

ange (Apr 09 2019 at 20:14, on Zulip):

@nikomatsakis well, if you could have one upvar for each captured variable/field, perhaps consumers of upvars would not need to be updated (or at least, not as many consumers would)

nikomatsakis (Apr 09 2019 at 20:23, on Zulip):

The challenge I think is that right now an upvar always maps 1-to-1 with some user variable and we're going to have to change that assumption. In any case, I'm feeling a bit unsure about what the concrete steps ought to be to move forward with this refactoring. I think there may be value though in trying to land code that just figures out which paths should be captured, even if we haven't plumbed that all the way through -- particularly if you're going to have to stop working on this at some point (didn't you mention @ange that you might run out of time?)

nikomatsakis (Apr 09 2019 at 20:23, on Zulip):

but maybe it makes sense to wait and only land code when we have something end-to-end working

ange (Apr 09 2019 at 20:32, on Zulip):

I expect I can get a decent-looking PR ready that just dumps the capture paths when requested (what's missing is testcases I guess). personally, I wouldn't feel comfortable submitting it for inclusion ATM though, as I'm not even sure the cmt_-based code will be enough (and might need to be significantly rewritten)

ange (Apr 09 2019 at 20:33, on Zulip):

in any case, I can try to always have some mergeable branch on github

ange (Apr 09 2019 at 20:34, on Zulip):

ATM I'm more worried about work going to waste because the approach might prove to be a dead end

ange (Apr 09 2019 at 20:35, on Zulip):

which is why I'm mostly trying to look ahead to the end-to-end functionality

ange (Apr 09 2019 at 20:36, on Zulip):

I guess I'll try to investigate it myself and see where I get ¯\_(ツ)_/¯

ange (Apr 09 2019 at 20:37, on Zulip):

any and all input is more than welcome of course

ange (Apr 10 2019 at 17:27, on Zulip):

OK, so: the MIR borrow checker adjustments don't seem too bad after all. Things would be easier if UpvarId contained the path too, but non-invasive way to do it introduces lots of cloning (as UpvarId is Copy).

Alternatively, we could try making UpvarId an index and hold the non-Copy Upvar (which also contains the path) in a table

ange (Apr 10 2019 at 17:28, on Zulip):

I'm guessing all this cloning is an antipattern for rustc, even if the number of upvars is likely to be small in practice

ange (Apr 10 2019 at 17:29, on Zulip):

that said, I see the following difficulty: mem_categorization.rs wants to build an UpvarId to store in NoteClosureEnv/NoteUpvarRef. At that point, we don't have any path information available (indeed, we make use of cmt_ to deduce the path information). We can't straightforwardly backpatch the path either, as the cmt_ code starts from Def::Upvar. I guess we could add an optional path throughout, but this feels pretty icky.

Technically, we can start with an empty path (i.e. (x, []) means "here we're capturing x in its entirety") and then refine the path in upvar.rs. One problem here is that later calls to the cmt_ functions will still start from the Def::Upvar. I suppose we could use a adjust_upvar_captures as a side-table in cat_upvar in later calls to the cmt_ code? Nope. When looking at a Def::Upvar, we have no way of telling what the replacement UpvarId is.

ange (Apr 10 2019 at 17:32, on Zulip):

Wondering if there's a modifying visitor for the HIR...

ange (Apr 10 2019 at 17:34, on Zulip):

other than hair/cx that is :-)

ange (Apr 11 2019 at 09:15, on Zulip):

huh, I can't find existing code that modifies the HIR. AFAIU, modifying the HIR is needed for commuting the path of an upvar access within the closure body, i.e. convert:

let x = X { y, z };
// Capture x
let cl = || {
    bar(&env.0.y);
};

to

let x = X {y, z};
// Capture x.y
let cl = || {
    bar(&env.0);
}
ange (Apr 15 2019 at 19:10, on Zulip):

FWIW, updated the paper doc with notes on what I worked on / found out this week

nikomatsakis (Apr 15 2019 at 19:41, on Zulip):

@ange we never mutate the HIR, we just built up "Side tables"

ange (Apr 15 2019 at 19:54, on Zulip):

Yah, see the "Capture paths of an Upvar, not the variable itself" section in the paper doc (unfortunately only allows two levels of headings so can't link directly)

ange (Apr 15 2019 at 19:55, on Zulip):

Will try to clean up the doc before tomorrow's sync up

Last update: Nov 17 2019 at 07:00UTC