Stream: t-compiler/wg-rfc-2229

Topic: sync 2019-04-16


ange (Apr 16 2019 at 17:45, on Zulip):

Hey all. Perhaps overwhelming description of open issues, implementation work, thoughts on my end here

nikomatsakis (Apr 16 2019 at 17:53, on Zulip):

:wave: hey all

nikomatsakis (Apr 16 2019 at 17:53, on Zulip):

cc @WG-rfc-2229 -- brief sync

nikomatsakis (Apr 16 2019 at 17:53, on Zulip):

So @blitzerr mentioned to me that they'll be busy for the next little bit owing to work stuff

nikomatsakis (Apr 16 2019 at 17:54, on Zulip):

I'll talk a look at what you wrote @ange, and I'm hoping today to leave some notes on the freevars stuff

ange (Apr 16 2019 at 17:54, on Zulip):

cool

ange (Apr 16 2019 at 17:54, on Zulip):

I'll be around to for clarifications/questions the next half hour or so

ange (Apr 16 2019 at 17:55, on Zulip):

async works too

nikomatsakis (Apr 16 2019 at 18:01, on Zulip):

Hey all. Perhaps overwhelming description of open issues, implementation work, thoughts on my end here

I see the notes from today -- did you add material to the other sections, @ange?

ange (Apr 16 2019 at 18:02, on Zulip):

yah

ange (Apr 16 2019 at 18:03, on Zulip):

I honestly don't know how to best do updates in such a doc; I think currently the easiest way might be for you to browse the history, the edits should be clustered together

ange (Apr 16 2019 at 18:03, on Zulip):

I tried to add open issues at some places, but really almost every section has something appended to it

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

I'm thinking that the most effective way to progress here is probably to schedule some time for us to do a deep dive chat

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

I feel like I have to focus on this problem for 1-2h :)

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

What time zone are you in, @ange ?

ange (Apr 16 2019 at 18:05, on Zulip):

I feel like I have to focus on this problem for 1-2h :)

Makes sense. Whatever time you can spend on it...

ange (Apr 16 2019 at 18:05, on Zulip):

currently EEST

nikomatsakis (Apr 16 2019 at 18:06, on Zulip):

I don't know what EEST is :)

nikomatsakis (Apr 16 2019 at 18:06, on Zulip):

Ah, ok, Greek time ;)

ange (Apr 16 2019 at 18:06, on Zulip):

sorry, eastern europe summer time

ange (Apr 16 2019 at 18:06, on Zulip):

yah

nikomatsakis (Apr 16 2019 at 18:10, on Zulip):

OK, so, I could probably dedicate some time Friday morning (my time) -- say at 09:00 UTC-04:00

ange (Apr 16 2019 at 18:11, on Zulip):

I'm afraid I can't do Friday this week, traveling to attend a wedding

ange (Apr 16 2019 at 18:12, on Zulip):

(so western hemisphere Thursday morning probably won't work for me either)

ange (Apr 16 2019 at 18:15, on Zulip):

fwiw, I can do up to 1am EEST if that would be helpful

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

(currently on vacation :P)

nikomatsakis (Apr 16 2019 at 18:22, on Zulip):

what about next Tuesday?

ange (Apr 16 2019 at 18:23, on Zulip):

sure

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

ok let me add something to the compiler team calendar -- 10:30 UTC-04:00 (17:30 EEST, if I'm not mistaken) work for you?

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

yah, that's fine. one point though

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

yep

ange (Apr 16 2019 at 18:25, on Zulip):

I'll only have like 3-4 free days after that to wrap up - going back to work full time in May, so dunno how much time I'll have to spend

ange (Apr 16 2019 at 18:25, on Zulip):

but I think this is a useful discussion to have regardless

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

agreed

ange (Apr 16 2019 at 18:25, on Zulip):

for this WG

ange (Apr 16 2019 at 18:25, on Zulip):

zulip?

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

Yeah

nikomatsakis (Apr 16 2019 at 18:26, on Zulip):

I'll only have like 3-4 free days after that to wrap up - going back to work full time in May, so dunno how much time I'll have to spend

I don't really have much time before that, I fear :(

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

that's quite alright :-)

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

glad I don't need to set up zoom :-)

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

I hope I can figure out some issues on my own till then, or at least tie up some loose ends

nikomatsakis (Apr 16 2019 at 18:34, on Zulip):

I'll see if I can carve out some time for "pre-thinking" and to review your notes in more detail

ange (Apr 23 2019 at 14:34, on Zulip):

@nikomatsakis just a suggestion, it might make sense to skim the dropbox paper first if you haven't already. No need to worry about duplication of questions though I'll figure that out after the fact

nikomatsakis (Apr 23 2019 at 14:35, on Zulip):

Hi @ange -- sounds good, I've been writing some notes already

ange (Apr 23 2019 at 14:35, on Zulip):

cool

ange (Apr 23 2019 at 14:41, on Zulip):

umm yah, AFAICT the example you're putting forward is not addressed in the RFC

nikomatsakis (Apr 23 2019 at 14:41, on Zulip):

@ange ok so it's probably good for us to elaborate our goals a bit

nikomatsakis (Apr 23 2019 at 14:41, on Zulip):

one part of it I think should be to raise up interesting lang design questions :)

nikomatsakis (Apr 23 2019 at 14:41, on Zulip):

but we should also talk about what the impl in rustc will need

ange (Apr 23 2019 at 14:41, on Zulip):

sure

nikomatsakis (Apr 23 2019 at 14:41, on Zulip):

I was wondering -- why did you extend Rvalue, was there a particular motivating example

ange (Apr 23 2019 at 14:42, on Zulip):

@nikomatsakis there was, but it's been so long I don't really remember -- I'll have to get back to you on that

ange (Apr 23 2019 at 14:43, on Zulip):

it could also be that I was confused re: what the RFC required (e.g. my commit message mentions indexing), so maybe it's a red herring, I would suggest tabling this until I can give a definitive example (or not)

ange (Apr 23 2019 at 14:45, on Zulip):

re: the pointer overwrite example above, this feels like a definitional thing (i.e. could go either way if one were starting from scratch), but it seems we might need to pick between backwards compat regarding that and special treatment of box?

nikomatsakis (Apr 23 2019 at 14:45, on Zulip):

Well, strict backwards compatibility may not be a concern

nikomatsakis (Apr 23 2019 at 14:45, on Zulip):

e.g., my example that I gave would not compile today

nikomatsakis (Apr 23 2019 at 14:47, on Zulip):

I was skimming your notes about drop and so forth

ange (Apr 23 2019 at 14:47, on Zulip):

what would be the considerations for lax backwards compatibility? user astonishment?

nikomatsakis (Apr 23 2019 at 14:47, on Zulip):

My concern is user astonishment, yes

nikomatsakis (Apr 23 2019 at 14:48, on Zulip):

I general I think that -- unless you have a move closure -- the variables in the surrounding stack frame and those in the closure should not "go out of sync"

nikomatsakis (Apr 23 2019 at 14:48, on Zulip):

i.e., if the code compiles, then the same path in both places should yield same value

nikomatsakis (Apr 23 2019 at 14:48, on Zulip):

But this is a bit of a "design decision", worth surfacing

nikomatsakis (Apr 23 2019 at 14:48, on Zulip):

(Still, that is -- I think -- a useful invariant that Rust maintains today, and it is by design that it does so)

nikomatsakis (Apr 23 2019 at 14:49, on Zulip):

(in the past, we considered "auto-inferring" move annotations on closures, for example, but opted against it for this reason -- and because we can't always do so)

ange (Apr 23 2019 at 14:49, on Zulip):

well, if closures are defined to be minimal, this feels like a teachability thing, i.e. a "but beware of references" paragraph somewhere. but I guess that's something to get wider feedback on

ange (Apr 23 2019 at 14:49, on Zulip):

ah

nikomatsakis (Apr 23 2019 at 14:51, on Zulip):

so I suspect that you added the rvalue feedback because

nikomatsakis (Apr 23 2019 at 14:51, on Zulip):

of overloaded index and deref operators

nikomatsakis (Apr 23 2019 at 14:52, on Zulip):

i.e., if we have some_vec[3].foo, we will probably give back a cmt that roughly corresponds to this desugared version:

tmp = Index::index(&some_vec, 3);
&(*tmp).foo

i.e., this "temporary" is the rvalue

nikomatsakis (Apr 23 2019 at 14:53, on Zulip):

but then again that seems .. sort of ok .. in that there is also a borrow of some_vec, and indeed we probably need to capture the whole vec for this reason

nikomatsakis (Apr 23 2019 at 14:53, on Zulip):

but I guess that if we had a notion of "pure" overloaded ops, we would have trouble

ange (Apr 23 2019 at 14:53, on Zulip):

yah. it's just that the RFC explicitly specifies that indexing is impure, so it could be that I was just confused

nikomatsakis (Apr 23 2019 at 14:53, on Zulip):

and so perhaps you were motivated by (e.g.) Box

ange (Apr 23 2019 at 14:54, on Zulip):

that does ring a bell, but I do know I didn't have any Box testcases back then -- perhaps I was just trying to code ahead

ange (Apr 23 2019 at 14:55, on Zulip):

I keep learning about both rustc and rust semantics, so I wouldn't read too much into my implementation choices from 3 weeks back

nikomatsakis (Apr 23 2019 at 14:56, on Zulip):

it appears you asked me about it here

nikomatsakis (Apr 23 2019 at 14:56, on Zulip):

it looks like you were concerned about foo.bar[x].baz

nikomatsakis (Apr 23 2019 at 14:57, on Zulip):

in such a case, we probably want to borrow &foo.bar

nikomatsakis (Apr 23 2019 at 14:57, on Zulip):

however, I would expect that the current code will give you two callbacks

nikomatsakis (Apr 23 2019 at 14:57, on Zulip):

one for foo.bar and one for (rvalue).baz, something like that

ange (Apr 23 2019 at 14:57, on Zulip):

right, I remember now, I wanted to look past the indexing from right to left

ange (Apr 23 2019 at 14:58, on Zulip):

hmm. I'm not sure that's the case then, but it could be that I hadn't instrumented all the delegate callbacks back then, so I misunderstood and thought that was necessary

ange (Apr 23 2019 at 14:58, on Zulip):

I can try and verify whether that's the case or not

nikomatsakis (Apr 23 2019 at 14:58, on Zulip):

it's possible the code doesn't, but it seems like it'd be a gaping hole in the old borrow check

nikomatsakis (Apr 23 2019 at 14:58, on Zulip):

which..of course..is possible, there were numerous bugs :)

nikomatsakis (Apr 23 2019 at 14:59, on Zulip):

In any case, I do think it's worth us looking a bit to the future

ange (Apr 23 2019 at 14:59, on Zulip):

I would say it's extraordinarily more likely that I didn't really under which circumstances each callback was called (I'm still not 100% sure I get all call paths)

nikomatsakis (Apr 23 2019 at 15:00, on Zulip):

The existing upvar code is based on this ExprUseVisitor, which is shared by typeck and the old, AST-based borrowck

ange (Apr 23 2019 at 15:00, on Zulip):

right

nikomatsakis (Apr 23 2019 at 15:00, on Zulip):

(as well as two other pieces of code, both of which I think are outdated in a fashion similar to borrow check)

nikomatsakis (Apr 23 2019 at 15:01, on Zulip):

my general feeling is that it makes sense to rewrite expr-use-visitor to be cleaner and more tailored to the needs of upvarck

nikomatsakis (Apr 23 2019 at 15:01, on Zulip):

this would probably include creating a new data structure that would replace cmt for this purpose

nikomatsakis (Apr 23 2019 at 15:01, on Zulip):

probably this is the CapturePath that you had talked about

nikomatsakis (Apr 23 2019 at 15:02, on Zulip):

my general feeling is that it makes sense to rewrite expr-use-visitor to be cleaner and more tailored to the needs of upvarck

the integration with upvar.rs has always been a bit crufty

ange (Apr 23 2019 at 15:02, on Zulip):

what parts of ExprUseVisitor strike you as redundant for upvar.rs?

nikomatsakis (Apr 23 2019 at 15:03, on Zulip):

it's not so much that they are redundant as superfluous

nikomatsakis (Apr 23 2019 at 15:03, on Zulip):

i.e., unnecessary

nikomatsakis (Apr 23 2019 at 15:03, on Zulip):

but here are some examples

nikomatsakis (Apr 23 2019 at 15:03, on Zulip):

for one thing, the ExprUseVisitor tries to "desugar" references to upvars

nikomatsakis (Apr 23 2019 at 15:03, on Zulip):

this is kind of awkward for us

nikomatsakis (Apr 23 2019 at 15:03, on Zulip):

we have to "resugar" those references by intercepting the "notes" that are attached to cmts

nikomatsakis (Apr 23 2019 at 15:04, on Zulip):

I guess in general the ExprUseVisitor is doing more desugaring than I think we truly want

nikomatsakis (Apr 23 2019 at 15:04, on Zulip):

the case of Rvalue is another such example

ange (Apr 23 2019 at 15:04, on Zulip):

right, AFAIU this is needed for borrowck, though I'm totally unclear on the details

nikomatsakis (Apr 23 2019 at 15:04, on Zulip):

that said, we do want it to do some amount of -- at least normalization

nikomatsakis (Apr 23 2019 at 15:04, on Zulip):

yes, it is, but if we made a split copy of the code that was tailored to the needs of upvar.rs, it could be simpler

nikomatsakis (Apr 23 2019 at 15:04, on Zulip):

that's my general point

ange (Apr 23 2019 at 15:04, on Zulip):

hmm

nikomatsakis (Apr 23 2019 at 15:05, on Zulip):

also relevant here is looking -- as you were starting to do -- at the code which comes after

ange (Apr 23 2019 at 15:05, on Zulip):

it's a fair point - do you think it would make sense to reuse cmt_ and aim to trim it when upvar becomes the sole user?

nikomatsakis (Apr 23 2019 at 15:05, on Zulip):

yes, it is, but if we made a split copy of the code that was tailored to the needs of upvar.rs, it could be simpler

( in general, I think re-using and sharing code is good, but since the old borrowck is going to be deleted, it won't be shared anymore )

nikomatsakis (Apr 23 2019 at 15:05, on Zulip):

it's a fair point - do you think it would make sense to reuse cmt_ and aim to trim it when upvar becomes the sole user?

I do not, I would rewrite that code from scratch

nikomatsakis (Apr 23 2019 at 15:05, on Zulip):

it's quite old

nikomatsakis (Apr 23 2019 at 15:05, on Zulip):

you can tell by the naming conventions ;)

nikomatsakis (Apr 23 2019 at 15:06, on Zulip):

I feel like what we want is something much closer to the HIR

ange (Apr 23 2019 at 15:07, on Zulip):

well, "old" is not necessarily bad in my book :-) but as there are specific changes to be made, sure

nikomatsakis (Apr 23 2019 at 15:07, on Zulip):

that is, we want simpler paths that are much closer to the what the user wrote. But it's interesting to think about just what bits of extra info we do need

nikomatsakis (Apr 23 2019 at 15:08, on Zulip):

well, "old" is not necessarily bad in my book :-) but as there are specific changes to be made, sure

no, I agree, but

ange (Apr 23 2019 at 15:08, on Zulip):

I think I get where you're coming from here - this might be an opportunity to reduce some of the technical debt in the compiler and, if so, we should take it

nikomatsakis (Apr 23 2019 at 15:08, on Zulip):

yes, basically that

nikomatsakis (Apr 23 2019 at 15:08, on Zulip):

so, let's imagine that we created a CapturePath like type

nikomatsakis (Apr 23 2019 at 15:08, on Zulip):

I think it would be fairly simple, probably reasonably similar to the MIR Place

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

personally, I'd be much more comfortable doing that if I had extensive tests that fully exercise the code though (so, in my mind, that would be a mostly complete end to end implementation of the RFC)

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

(though the new MIR place that we're trying to move to, not the current one)

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

I haven't looked at MIR Place 2.0 yet

nikomatsakis (Apr 23 2019 at 15:10, on Zulip):

It's not that complex, it's basically a re-oriented data structure that is better optimized for going forward

ange (Apr 23 2019 at 15:10, on Zulip):

that's what we need for this RFC, yah

nikomatsakis (Apr 23 2019 at 15:11, on Zulip):

the main change is that you have (sort of) (CapturePathBase, Vec<CapturePathExtension>)

nikomatsakis (Apr 23 2019 at 15:11, on Zulip):

where CapturePathExtension is like deref, field, index

ange (Apr 23 2019 at 15:12, on Zulip):

why index?

nikomatsakis (Apr 23 2019 at 15:12, on Zulip):

personally, I'd be much more comfortable doing that if I had extensive tests that fully exercise the code though (so, in my mind, that would be a mostly complete end to end implementation of the RFC)

I do think we should think about tests here -- but of course we don't have any tests for this new feature, since it doesn't exist. What we do have (for the old code) is borrow check tests, that show it is "detecting" the mutations, borrows, etc at least a lot of the time. So this is probably what we'd have to reproduce.

nikomatsakis (Apr 23 2019 at 15:12, on Zulip):

But that's good anyway

nikomatsakis (Apr 23 2019 at 15:12, on Zulip):

because those tests exist for simple fns, but not necessarily for closures

nikomatsakis (Apr 23 2019 at 15:13, on Zulip):

(and indeed creating a thorough test suite is I think a key part of the work to be done)

nikomatsakis (Apr 23 2019 at 15:13, on Zulip):

why index?

you're correct that for capture paths it could be excluded

nikomatsakis (Apr 23 2019 at 15:13, on Zulip):

it is part of the "generality"

nikomatsakis (Apr 23 2019 at 15:13, on Zulip):

that is, it is part of MIR

ange (Apr 23 2019 at 15:13, on Zulip):

I only meant, this would be one argument for going with cmt_, then rewriting after we're comfortable it mostly works -- but that's probably me being unsafe about my knowledge of rustc

ange (Apr 23 2019 at 15:14, on Zulip):

that is, it is part of MIR

Oh you were describing Place 2.0? OK

nikomatsakis (Apr 23 2019 at 15:14, on Zulip):

I see. Well, it seems to have a "base case" problem -- how are we confident that it mostly works :)

nikomatsakis (Apr 23 2019 at 15:14, on Zulip):

In any case, the next question

nikomatsakis (Apr 23 2019 at 15:14, on Zulip):

presume we had this notion of capture-path

ange (Apr 23 2019 at 15:14, on Zulip):

Well, currently I'm trying to extend my testcases to exercise all paths in upvar.rs :-)

nikomatsakis (Apr 23 2019 at 15:15, on Zulip):

so, to talk about the tests for a moemnt

ange (Apr 23 2019 at 15:15, on Zulip):

Well, currently I'm trying to extend my testcases to exercise all paths in upvar.rs :-)

(I know path coverage isn't enough)

nikomatsakis (Apr 23 2019 at 15:15, on Zulip):

esp .give as your time is limited :)

nikomatsakis (Apr 23 2019 at 15:15, on Zulip):

I think a really great think we could focus on right now

nikomatsakis (Apr 23 2019 at 15:15, on Zulip):

is trying to map out the scenarios that make sure we have tests covering them and expected outcomes

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

I'm trying to think what the "matrix" looks like

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

(it seems like you've been doing a lot of that work)

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

basically I think some of the key "unresolved questions" of the RFC were around the specifics of the "capture" algorithm

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

(though the hardest one to answer are the potential changes to drop order)

ange (Apr 23 2019 at 15:17, on Zulip):

(it seems like you've been doing a lot of that work)

most of what I have is in https://github.com/aoikonomopoulos/rust/blob/b44513e3b7fb25c370fb91e5271f30701bbb09fe/src/test/ui/closures/closure-partial-captures.rs fwiw

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

great

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

I think a really great think we could focus on right now

Agreed, though I would add that having a less fuzzy implementation plan for the next steps would make this much easier for people to work on piecemeal

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

yes, it's worth trying to talk out that plan a bit more

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

one nice thing about this refactoring that I'm sort of pitching, of course, is that we can refactor upvar.rs as it is now (without supporting (yet) RFC 2229) and use our test suite and crater to check for problems

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

that should shake out most of the bugs

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

(and help us grow our test suite, since any bugs we find with crater that weren't detected by our tests are good candidates for unit tests)

nikomatsakis (Apr 23 2019 at 15:21, on Zulip):

anyway, the next question I think is what canges downstream

nikomatsakis (Apr 23 2019 at 15:21, on Zulip):

as you highlight in your doc, there are lots of places where we talk about "upvars" =)

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

The most immediate thing would be that we have to alter HAIR construction -- right now, the upvars are always individual variables. We would presumably load use the capture path to construct a more precise result.

ange (Apr 23 2019 at 15:26, on Zulip):

The most immediate thing would be that we have to alter HAIR construction -- right now, the upvars are always individual variables. We would presumably load use the capture path to construct a more precise result.

So, I have this relevant note in the doc:
Alternatively, we could do the rewriting of both the capturing and the accessing during translation to HAIR or MIR. This would mean changing the number of upvar captures after type/borrow checking though. The implementation might end up a bit more involved and there’s less of a safety net.

nikomatsakis (Apr 23 2019 at 15:27, on Zulip):

I don't really think we should change the basic shape of how things work today

nikomatsakis (Apr 23 2019 at 15:28, on Zulip):

They are this way for a reason :)

nikomatsakis (Apr 23 2019 at 15:28, on Zulip):

That is, I think that HAIR should make explicit the paths that it accesses

nikomatsakis (Apr 23 2019 at 15:28, on Zulip):

MIR construction really wouldn't have to change at all

nikomatsakis (Apr 23 2019 at 15:28, on Zulip):

Well, that's not quite true

nikomatsakis (Apr 23 2019 at 15:28, on Zulip):

The complex bit is now so much constructing the closure

nikomatsakis (Apr 23 2019 at 15:29, on Zulip):

The complex bit is constructing the closure body

nikomatsakis (Apr 23 2019 at 15:29, on Zulip):

in particular, right now, we can intercept just the root local variable access

nikomatsakis (Apr 23 2019 at 15:29, on Zulip):

and translate it to an upvar access

ange (Apr 23 2019 at 15:29, on Zulip):

ok, so the fundamental issue I'm worried about here is that we can change what's captured in upvar.rs, but we can't change what's used until HAIR translation, so how will type checking work?

nikomatsakis (Apr 23 2019 at 15:29, on Zulip):

but we now have to intercept arbitrary points

ange (Apr 23 2019 at 15:29, on Zulip):

right

nikomatsakis (Apr 23 2019 at 15:29, on Zulip):

ok, so the fundamental issue I'm worried about here is that we can change what's captured in upvar.rs, but we can't change what's used until HAIR translation, so how will type checking work?

I don't quite understand this concern

nikomatsakis (Apr 23 2019 at 15:30, on Zulip):

but we now have to intercept arbitrary points

so we need some form of data structure that lists out those points, I think

ange (Apr 23 2019 at 15:30, on Zulip):

it's basically what you just said, I think

nikomatsakis (Apr 23 2019 at 15:30, on Zulip):

this seems relevant to the capture-path design, as well

ange (Apr 23 2019 at 15:30, on Zulip):

would the expr adjustments work?

ange (Apr 23 2019 at 15:30, on Zulip):

I mean, with a new adjustment type

nikomatsakis (Apr 23 2019 at 15:31, on Zulip):

i.e., if we keep the basic model of expr-use-visitor, then we're going to be getting various callbacks saying "this capture-path is accessed at this point"

nikomatsakis (Apr 23 2019 at 15:31, on Zulip):

we need to be able to revisit those points and say "map point X to upvar U"

ange (Apr 23 2019 at 15:31, on Zulip):

right. and we don't know what we want to rewrite it to until we've visited the whole closure body

ange (Apr 23 2019 at 15:31, on Zulip):

right

ange (Apr 23 2019 at 15:32, on Zulip):

well

nikomatsakis (Apr 23 2019 at 15:32, on Zulip):

yeah; I mean one thing we might do is just revisit the body again once we've got the answer, but we might also remember the points we need and adjust it as we go. that's an interesting question.

ange (Apr 23 2019 at 15:32, on Zulip):

I would say it's "map point X to upvar U + some sub path"

nikomatsakis (Apr 23 2019 at 15:32, on Zulip):

no, I wouldn't say that

nikomatsakis (Apr 23 2019 at 15:33, on Zulip):

both would be possible perhaps

nikomatsakis (Apr 23 2019 at 15:33, on Zulip):

but I think that e.g. if we have a.b.c.d, and we wind up deciding to capture a.b

ange (Apr 23 2019 at 15:33, on Zulip):

ok, but in the general case there will be some subpath, no?

nikomatsakis (Apr 23 2019 at 15:33, on Zulip):

we should not map a.b.c.d to "a.b + c.d`

nikomatsakis (Apr 23 2019 at 15:33, on Zulip):

we should instead remap the a.b location

nikomatsakis (Apr 23 2019 at 15:34, on Zulip):

i.e., if the input is something like:

we could map node 0 to upvar 0 + c.d

ange (Apr 23 2019 at 15:34, on Zulip):

I think we're saying the same thing? you would convert that to env.0.c.d

nikomatsakis (Apr 23 2019 at 15:34, on Zulip):

or we could map node 2 to upvar 0

ange (Apr 23 2019 at 15:34, on Zulip):

right

nikomatsakis (Apr 23 2019 at 15:34, on Zulip):

I think we're saying the same thing? you would convert that to env.0.c.d

the point is, the data structure that does the mapping is a map of NodeId -> UpvarId or something

nikomatsakis (Apr 23 2019 at 15:34, on Zulip):

it doesn't know anything about the "suffix"

ange (Apr 23 2019 at 15:35, on Zulip):

ah I see what you mean, yah

nikomatsakis (Apr 23 2019 at 15:35, on Zulip):

you asked about adjustments:

nikomatsakis (Apr 23 2019 at 15:35, on Zulip):

one interesting question is whether we would ever capture "mid adjustment"

ange (Apr 23 2019 at 15:35, on Zulip):

heh

nikomatsakis (Apr 23 2019 at 15:35, on Zulip):

it'd be nice if we didn't :)

ange (Apr 23 2019 at 15:36, on Zulip):

I'll take your word for it :-)

ange (Apr 23 2019 at 15:36, on Zulip):

that it's even possible, in fact

nikomatsakis (Apr 23 2019 at 15:36, on Zulip):

well I mean a simple example would be something like x.y -- if x: &mut Foo, then this has an "adjustment" like (*x).y

nikomatsakis (Apr 23 2019 at 15:36, on Zulip):

you could imagine us choosing to capture *x

nikomatsakis (Apr 23 2019 at 15:37, on Zulip):

e.g., if there is also a use of *x but not x itself

ange (Apr 23 2019 at 15:37, on Zulip):

ugh

nikomatsakis (Apr 23 2019 at 15:37, on Zulip):

but I .. think we could probably "equally well" choose x, in terms of whether the user cares

nikomatsakis (Apr 23 2019 at 15:37, on Zulip):

so we might want to take that into account or something

nikomatsakis (Apr 23 2019 at 15:37, on Zulip):

(although it might not be so bad if we did intercept mid-adjustment)

nikomatsakis (Apr 23 2019 at 15:37, on Zulip):

still, it sounds like a pain

ange (Apr 23 2019 at 15:37, on Zulip):

that warrants a TODO at the very least, yah

nikomatsakis (Apr 23 2019 at 15:39, on Zulip):

anyway, assuming we handle that

nikomatsakis (Apr 23 2019 at 15:39, on Zulip):

I think that MIR construction wouldnt' have to change much, if at all

nikomatsakis (Apr 23 2019 at 15:39, on Zulip):

the work is mostly going to be handled in the HIR -> HAIR lowering

nikomatsakis (Apr 23 2019 at 15:40, on Zulip):

now, MIR borrow check, on the other hand, may need to change, but I think primarily around the diagnostics

nikomatsakis (Apr 23 2019 at 15:40, on Zulip):

this is going to be somewhat non-trivial, likely

ange (Apr 23 2019 at 15:40, on Zulip):

I think we're saying the same thing? you would convert that to env.0.c.d

the point is, the data structure that does the mapping is a map of NodeId -> UpvarId or something

Hmm. How do you build that up, exactly? To decide what node you want to rewrite, you need to have seen all paths first, no?

ange (Apr 23 2019 at 15:40, on Zulip):

the work is mostly going to be handled in the HIR -> HAIR lowering

How will this work, exactly? AFAIU, if we change the Upvar but only do the rewriting of the access in the HAIR translation, won't the types mismatch in the meantime?

nikomatsakis (Apr 23 2019 at 15:40, on Zulip):

Hmm. How do you build that up, exactly? To decide what node you want to rewrite, you need to have seen all paths first, no?

Yes, we either have to walk the body again to build the map, or else we have to keep enough data from the first walk that we can build it

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

there is a bit of tension here in that the "capture path" would like to be "disconnected" from the specific place it appears sometimes but not always

nikomatsakis (Apr 23 2019 at 15:42, on Zulip):

i.e., if the capture-path included the id of the HIR it was built from or whatever, that might help us to construct the "upvar map" incrementally

ange (Apr 23 2019 at 15:42, on Zulip):

there is a bit of tension here in that the "capture path" would like to be "disconnected" from the specific place it appears sometimes but not always

right. it seems to be mostly logistics though

nikomatsakis (Apr 23 2019 at 15:42, on Zulip):

but I'm unpersuaded that is what we want, I think I would definitely consider a second walk

nikomatsakis (Apr 23 2019 at 15:42, on Zulip):

I guess i'm not really sure

nikomatsakis (Apr 23 2019 at 15:42, on Zulip):

but yeah it feels like "logistics"

nikomatsakis (Apr 23 2019 at 15:43, on Zulip):

this is all sounding decidedly non-trivial :)

nikomatsakis (Apr 23 2019 at 15:43, on Zulip):

not .. NLL-level of complexity, but like it will require a bit lot more attention from an experiented mentor than we've had thus far

ange (Apr 23 2019 at 15:43, on Zulip):

well. yah

ange (Apr 23 2019 at 15:44, on Zulip):

yah, that seems to be a significant consideration

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

(the next interesting thing is to think about the incremental steps)

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

anyway, I have to run now and get myself some lunch

ange (Apr 23 2019 at 15:44, on Zulip):

kk

nikomatsakis (Apr 23 2019 at 15:45, on Zulip):

I think the clear take-away thus far is:

nikomatsakis (Apr 23 2019 at 15:45, on Zulip):

(this feels like it would be a good candidate for the rustc design meeting, while i'm at it)

nikomatsakis (Apr 23 2019 at 15:45, on Zulip):

specifically talking about the details of how the capturing ought to work and so forth

ange (Apr 23 2019 at 15:46, on Zulip):

an open question on my end is how this would work if you want to rewrite the accesses during HAIR conversion. AFAIU, there would be a type mismatch in the meantime and doesn't type checking happen on the HIR?

ange (Apr 23 2019 at 15:46, on Zulip):

well, it's good that the tests seem to be somewhat actionable at least :-)

nikomatsakis (Apr 23 2019 at 16:02, on Zulip):

an open question on my end is how this would work if you want to rewrite the accesses during HAIR conversion. AFAIU, there would be a type mismatch in the meantime and doesn't type checking happen on the HIR?

type mismatch where and how? I don't yet see the problem you are concerned about =)

ange (Apr 23 2019 at 16:17, on Zulip):

Lemme try to explain: my concern is that we'll need to
1. change the number of upvars in upvar.rs (e.g. when two fields of a struct are captured rather than the struct) and, of course their types. I presume this would happen in closure_analyze in the same file.
2. change what is referenced in the closure body, as we talked about above. AFAIU you're suggesting we do that during HAIR conversion.
So the scenario I'm concerned about is that we do 1, then run type checking (for the HIR) but at that point what upvar does the closure body reference? The previous one. Hmm. Oh I see, typeck doesn't need to look at the captured upvars mentioned in TypeckTables at all

ange (Apr 23 2019 at 16:19, on Zulip):

so both capturing the upvars deduced in upvar.rsand rewriting the accesses to them would take place during HAIR conversion, that makes sense

ange (Apr 23 2019 at 16:27, on Zulip):

So, after this, I'm still unclear on "Respecting drop order", "Moving out of drop" and what exactly " &<Box<_> as Deref>::deref(&box_foo).b" (from the RFC) would be used for -- not sure whether you want to spend any cycles on that given the other open questions though

nikomatsakis (Apr 23 2019 at 17:56, on Zulip):

so, first of all, upvar analysis takes place after type-checking

nikomatsakis (Apr 23 2019 at 17:56, on Zulip):

that is, we're making all of these decisions after type-checking is complete

nikomatsakis (Apr 23 2019 at 17:57, on Zulip):

second, we're not really going to change what is referenced in the closure body -- well, it depends on your POV I guess, but we're going to be adding side annotations

nikomatsakis (Apr 23 2019 at 17:57, on Zulip):

but I guess you put it together, I see :)

nikomatsakis (Apr 23 2019 at 17:58, on Zulip):

So, after this, I'm still unclear on "Respecting drop order", "Moving out of drop" and what exactly " &<Box<_> as Deref>::deref(&box_foo).b" (from the RFC) would be used for -- not sure whether you want to spend any cycles on that given the other open questions though

well I think it'd be good to come up with some canonical examples

nikomatsakis (Apr 23 2019 at 17:58, on Zulip):

not sure if you mean that you are not sure on what the canonical examples would be?

nikomatsakis (Apr 23 2019 at 17:58, on Zulip):

So @WG-rfc-2229 -- it's our usual sync time :wave:

As you can see above, @ange and I spent some time already going through the "upvar conversion".

nikomatsakis (Apr 23 2019 at 17:59, on Zulip):

@blitzerr I'm assumign you're still occupied?

davidtwco (Apr 23 2019 at 17:59, on Zulip):

I think you pinged the wrong people.

nikomatsakis (Apr 23 2019 at 17:59, on Zulip):

Ah, so I did

nikomatsakis (Apr 23 2019 at 17:59, on Zulip):

Sorry!

davidtwco (Apr 23 2019 at 17:59, on Zulip):

I was thinking "didn't we just do that?"..

nikomatsakis (Apr 23 2019 at 17:59, on Zulip):

@WG-rfc-2229 is indeed what I meant =)

ange (Apr 23 2019 at 19:14, on Zulip):

so, first of all, upvar analysis takes place after type-checking

Oh that's right. I got confused because ty.needs_drop was complaining about inference types/regions, but that must have been because in my exploratory hack I was only lifting the specific type, but of course for this to work, writeback needs to have happened first. I think.

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

not sure if you mean that you are not sure on what the canonical examples would be?

I thought the canonical examples could be the ones inline in the dropbox paper? If not, then no, I'm not sure what they would be

ange (Apr 23 2019 at 19:16, on Zulip):

So @WG-rfc-2229 -- it's our usual sync time :wave:

Sorry for not being here, something came up

ange (Apr 23 2019 at 19:23, on Zulip):

(this feels like it would be a good candidate for the rustc design meeting, while i'm at it)

@nikomatsakis is this a thing that's going to happen? AFAIU, other than maybe looking at more testcases, I'm not sure what I should try to look at the next few days, if anything

ange (Apr 23 2019 at 19:23, on Zulip):

(no pressure of course, I can just, you know, not work during vacation, that sounds healthy)

nikomatsakis (Apr 23 2019 at 21:47, on Zulip):

the rustc design meeting will hopefully happen -- though there are some details to hammer out

nikomatsakis (Apr 23 2019 at 21:47, on Zulip):

discussing this idea there is not going to happen without more work :)

blitzerr (Apr 23 2019 at 23:10, on Zulip):

@nikomatsakis
I am here. Will be able to do something over the weekend going forward. Any updates for me ? Did we make some progress on the test failures.

ange (Apr 24 2019 at 06:56, on Zulip):

discussing this idea there is not going to happen without more work :)

@nikomatsakis well kindly let me know if there's anything I can help with in that direction then :-)

nikomatsakis (Apr 26 2019 at 19:55, on Zulip):

@blitzerr I never did figure out the test failure thing, but I would really like to get that PR of yours landed.

nikomatsakis (Apr 26 2019 at 19:55, on Zulip):

So it'd be nice to carve out some time for that

blitzerr (Apr 26 2019 at 19:58, on Zulip):

Sounds good. :+1:

Last update: Nov 17 2019 at 07:35UTC