Stream: t-compiler/wg-nll

Topic: issue-52034-liveness-only-for-things-with-regions


nikomatsakis (Jul 05 2018 at 17:38, on Zulip):

Ok, so @DPC, discussing https://github.com/rust-lang/rust/issues/52034 ...

nikomatsakis (Jul 05 2018 at 17:39, on Zulip):

the first step I think would be to refactor liveness

nikomatsakis (Jul 05 2018 at 17:39, on Zulip):

so as to introduce some indirection

nikomatsakis (Jul 05 2018 at 17:39, on Zulip):

e.g., right now it is always operating over bit sets of mir::Local -- that is, the full range of local variables

nikomatsakis (Jul 05 2018 at 17:39, on Zulip):

I would probably start by trying to refactor it (but not change anything else) to operate over a bit set of some generic type N: Idx

nikomatsakis (Jul 05 2018 at 17:39, on Zulip):

and then have a mapping fn from Local -> Option<N> (for now, we would always use it with N = Local and the mapping would be Some)

nikomatsakis (Jul 05 2018 at 17:40, on Zulip):

does that make sense at all?

DPC (Jul 05 2018 at 17:40, on Zulip):

by liveness you mean the generate function?

nikomatsakis (Jul 05 2018 at 17:40, on Zulip):

I mean the whole module pretty much

DPC (Jul 05 2018 at 17:40, on Zulip):

ah okay

nikomatsakis (Jul 05 2018 at 17:41, on Zulip):

so e.g. LivenessResult would be struct LivenessResult<N: Idx>

nikomatsakis (Jul 05 2018 at 17:41, on Zulip):

or maybe V (for "variable")

nikomatsakis (Jul 05 2018 at 17:41, on Zulip):

whatever

nikomatsakis (Jul 05 2018 at 17:41, on Zulip):

the field outs would go from

 pub outs: IndexVec<BasicBlock, LocalSet>,

to

 pub outs: IndexVec<BasicBlock, LocalSet<V>>,
DPC (Jul 05 2018 at 17:42, on Zulip):

ok got it

nikomatsakis (Jul 05 2018 at 17:42, on Zulip):

ok

nikomatsakis (Jul 05 2018 at 17:43, on Zulip):

the simulate_block callback would then change from

OP: FnMut(Location, &LocalSet)

to

OP: FnMut(Location, &LocalSet<V>)

I suppose

nikomatsakis (Jul 05 2018 at 17:43, on Zulip):

anyway I guess you get the idea :)

DPC (Jul 05 2018 at 17:43, on Zulip):

ya i have something to start with

nikomatsakis (Jul 05 2018 at 17:44, on Zulip):

let me know if other questions arise :)

nikomatsakis (Jul 05 2018 at 17:44, on Zulip):

ps a tip

nikomatsakis (Jul 05 2018 at 17:44, on Zulip):

for this sort of refactoring

nikomatsakis (Jul 05 2018 at 17:44, on Zulip):

I strongly recommend ./x.py check

nikomatsakis (Jul 05 2018 at 17:44, on Zulip):

much faster

nikomatsakis (Jul 05 2018 at 17:45, on Zulip):

and this ought to be the kind of thing where

nikomatsakis (Jul 05 2018 at 17:45, on Zulip):

as long as it compiles, it will work

DPC (Jul 05 2018 at 17:45, on Zulip):

i have issues with cmake on this system.

DPC (Jul 05 2018 at 17:45, on Zulip):

but i guess that doesn't matter for x.py check

nikomatsakis (Jul 05 2018 at 17:55, on Zulip):

well you do prob have to build LLVM?

nikomatsakis (Jul 05 2018 at 17:55, on Zulip):

not sure

DPC (Jul 05 2018 at 17:58, on Zulip):

it compiled file so i'm good :smile:

nikomatsakis (Jul 06 2018 at 14:43, on Zulip):

@DPC how goes btw? :)

nikomatsakis (Jul 06 2018 at 14:43, on Zulip):

feel free to open a [WIP] PR ...

nikomatsakis (Jul 06 2018 at 14:44, on Zulip):

sorry to hound you, this is just a high priority item :)

DPC (Jul 06 2018 at 14:44, on Zulip):

added the V's everywhere but i'm now stuck at someplace needing it

DPC (Jul 06 2018 at 14:44, on Zulip):

ya its fine. I understand

DPC (Jul 06 2018 at 14:48, on Zulip):

So i changed LivenessResultto LivenessResults<V: Idx>now but i get type annotation error https://github.com/Dylan-DPC/rust/blob/9363342be956d1bf7781a3b7455d80fc5d94b1f8/src/librustc_mir/borrow_check/nll/mod.rs#L104-L103

nikomatsakis (Jul 06 2018 at 14:49, on Zulip):

ah, yes, you probably need to do LivenessResults::<Local>::new or something tos tart

nikomatsakis (Jul 06 2018 at 14:49, on Zulip):

but you also will need some kind of "conversion" fn supplied somewhere, right?

nikomatsakis (Jul 06 2018 at 14:50, on Zulip):

(if you do open the PR, I can check it out locally)

DPC (Jul 06 2018 at 15:08, on Zulip):

conversion for?

nikomatsakis (Jul 06 2018 at 15:22, on Zulip):

I mean conversion from Local to Option<V>

nikomatsakis (Jul 06 2018 at 15:23, on Zulip):

@DPC can you send me your most recent commit or something?

nikomatsakis (Jul 06 2018 at 15:24, on Zulip):

(or just open a [WIP] PR)

DPC (Jul 06 2018 at 15:24, on Zulip):

yah will open a wip pr

nikomatsakis (Jul 06 2018 at 15:24, on Zulip):

ok, it'd be easier to take a look that way

DPC (Jul 06 2018 at 17:12, on Zulip):

https://github.com/rust-lang/rust/pull/52115

nikomatsakis (Jul 06 2018 at 19:00, on Zulip):

@DPC great! :) I had some errands to run but I'm back now

nikomatsakis (Jul 06 2018 at 19:00, on Zulip):

I'll take a quick look

nikomatsakis (Jul 06 2018 at 19:13, on Zulip):

@DPC left a review here, let me know what you think

DPC (Jul 06 2018 at 19:30, on Zulip):

looks good to me. The only reason i had to <V> most of the structs because one of the parameters needed a <V> which isn't needed if a <Local> it

nikomatsakis (Jul 06 2018 at 19:49, on Zulip):

yep :)

nikomatsakis (Jul 06 2018 at 19:49, on Zulip):

(this is just the first part, mind you, eventually some of those structs will use something other than Local)

nikomatsakis (Jul 06 2018 at 19:49, on Zulip):

but first we should be able to get it working with an identity mapping and that's like 90% of the work

DPC (Jul 07 2018 at 13:39, on Zulip):

(deleted)

DPC (Jul 07 2018 at 13:40, on Zulip):

you never completed your example :stuck_out_tongue:

nikomatsakis (Jul 07 2018 at 14:47, on Zulip):

whoops :)

nikomatsakis (Jul 07 2018 at 14:48, on Zulip):

@DPC fixed the comment

nikomatsakis (Jul 07 2018 at 20:41, on Zulip):

@DPC did this work out any better?

DPC (Jul 07 2018 at 20:42, on Zulip):

hey.. was about to ping you. having some type issues and other errors

DPC (Jul 07 2018 at 20:42, on Zulip):

i have dropped <V> for Local in some more places

nikomatsakis (Jul 07 2018 at 21:01, on Zulip):

ok, I'm gonna board flight soon but if you push a commit and errors I may be able to offer some tips :)

DPC (Jul 08 2018 at 07:09, on Zulip):

I had pushed last night. You can take a look when you are free :smiley:

DPC (Jul 08 2018 at 13:47, on Zulip):

@nikomatsakis I have pushed a commit that uses the V: LiveVariableMap instead of V:Idx. If this is fine, i'll change the others to reflect the same.

nikomatsakis (Jul 12 2018 at 05:17, on Zulip):

@DPC argh sorry! I was on the plane and havne't had time to open a computer till just now :)

nikomatsakis (Jul 13 2018 at 05:49, on Zulip):

@DPC left another round of feedback — I have to run for now again :) I might be back later though

DPC (Jul 13 2018 at 05:55, on Zulip):

thanks :)

nikomatsakis (Jul 13 2018 at 07:59, on Zulip):

let me know if it makes sense :) the key points are:

DPC (Jul 13 2018 at 08:47, on Zulip):

yeah it does. have made the changes will push in a sec

DPC (Jul 13 2018 at 09:43, on Zulip):

i'm getting:

116 | impl<V, M: LiveVariableMap<LiveVar = V>> LivenessResults<V> {
    |         ^ unconstrained type parameter
lqd (Jul 13 2018 at 11:55, on Zulip):

(@DPC by the way, if you fix the tidy failures on your PR, Travis will be able to continue, compile and run tests and in general be very helpful :)

DPC (Jul 13 2018 at 14:24, on Zulip):

@lqd Thanks. right now it is not ready and there are many changes left which make the test status of no relavance at this point.

nikomatsakis (Jul 13 2018 at 16:17, on Zulip):

@DPC I think the M parameter just has to be moved to the methods in question

nikomatsakis (Jul 13 2018 at 16:53, on Zulip):

Left a few more comments

DPC (Jul 13 2018 at 16:54, on Zulip):

yah saw it

DPC (Jul 14 2018 at 13:19, on Zulip):

How do I handle passing "local" or whatever to LocalSet in cases like this? https://github.com/Dylan-DPC/rust/blob/4c8605076f4f6c8d52c809d2f854ea43d520e5fd/src/librustc_mir/transform/generator.rs#L133

nikomatsakis (Jul 14 2018 at 14:34, on Zulip):

that case should just remain Local -- I'm not sure I 100% understand

nikomatsakis (Jul 14 2018 at 14:34, on Zulip):

(in particular that code is not part of MIR borrowck)

DPC (Jul 14 2018 at 14:57, on Zulip):

that errors because I added the bind on the type pub type LocalSet<V: LiveVariableMap> = IdxSetBuf<V::LiveVar>;

nikomatsakis (Jul 14 2018 at 15:55, on Zulip):

I think it should be pub type LocalSet<V> = IdxSetBuf<V>

nikomatsakis (Jul 14 2018 at 15:55, on Zulip):

sorry if I said something else

nikomatsakis (Jul 14 2018 at 15:55, on Zulip):

the idea would be: V is the type of the variable (at the moment, always Local, but later a new type)

nikomatsakis (Jul 14 2018 at 15:55, on Zulip):

the map type M we will only use as the parameter to methods

DPC (Jul 14 2018 at 18:45, on Zulip):

so because of IndexVec, I have to do V:Idx on LivenessResult and other places where I'm using LivenessResult.

DPC (Jul 14 2018 at 18:54, on Zulip):

but that will mean inserting <V: Idx> everywhere which doesn't seem to be the right thing to do

nikomatsakis (Jul 15 2018 at 03:46, on Zulip):

what do you mean by everywhere?

nikomatsakis (Jul 15 2018 at 03:47, on Zulip):

it should only be necessary to add it within the liveness code, which is now generic over the type V of indices used to describe variables

nikomatsakis (Jul 15 2018 at 03:51, on Zulip):

@DPC I've checked out your branch locally; I may have time to poke at it and show you what I mean a bit more directly, not sure :) depends how long it takes for x.py check to start giving me some errors

nikomatsakis (Jul 15 2018 at 04:33, on Zulip):

@DPC ok I had a few minutes so I took the liberty of pushing a commit to your branch which builds. You can check it out, but the idea is that liveness is generic over the type V, but everything else hardcodes Local. The next step then would be to make the MIR code:

There is perhaps some cleanup that might be worthwhile but is independent:

DPC (Jul 15 2018 at 13:07, on Zulip):

Thanks

nikomatsakis (Jul 15 2018 at 13:26, on Zulip):

@DPC np =) hope that makes senes

DPC (Jul 16 2018 at 11:42, on Zulip):

Define a new index type LocalWithRegion or something like that -- that corresponds to local variables that have regions.

Is this a type or a struct ? I couldn't find the defn of mir::Local to cross check with.

nikomatsakis (Jul 16 2018 at 11:43, on Zulip):

@DPC mir::Local is a struct, it's just an index

nikomatsakis (Jul 16 2018 at 11:44, on Zulip):

https://doc.rust-lang.org/nightly/nightly-rustc/rustc/mir/struct.Local.html

nikomatsakis (Jul 16 2018 at 11:44, on Zulip):

it is declared with newtype_index! which is why it is hard to find

nikomatsakis (Jul 16 2018 at 11:44, on Zulip):

we really need to change that macro

DPC (Jul 16 2018 at 11:44, on Zulip):

ah okay

nikomatsakis (Jul 16 2018 at 11:44, on Zulip):

specifically, it's an index into the MIR's local_decls vector

nikomatsakis (Jul 16 2018 at 11:44, on Zulip):

I do recommend searching with rustdoc incidentally

nikomatsakis (Jul 16 2018 at 11:45, on Zulip):

if you can't find something especially

DPC (Jul 16 2018 at 11:45, on Zulip):

will keep it in mind

nikomatsakis (Jul 16 2018 at 12:20, on Zulip):

@DPC (ps, I'm around now if you have any questions)

DPC (Jul 16 2018 at 12:21, on Zulip):

yeah i can see that you are online :P

nikomatsakis (Jul 16 2018 at 12:21, on Zulip):

I'm just doing my PR sweep and I saw your PR so I thought you :)

DPC (Jul 16 2018 at 13:08, on Zulip):

Create this LocalWithRegionMap early on -- before computing liveness etc -- and then pass it around where needed instead of IdentityMap (within librustc_mir::borrow_check, that is -- the other code that uses liveness can stay as it is)

Where exactly should I do it?

nikomatsakis (Jul 16 2018 at 13:10, on Zulip):

let me look...

nikomatsakis (Jul 16 2018 at 13:11, on Zulip):

probably around this line

nikomatsakis (Jul 16 2018 at 13:11, on Zulip):

currently we have something like:

    // Run the MIR type-checker.
    let liveness = &LivenessResults::compute(mir);
    let constraint_sets = type_check::type_check(...);
nikomatsakis (Jul 16 2018 at 13:12, on Zulip):

I imagine it would become something like

let liveness_map = NllLivenessMap::compute(mir);
let liveness = &LivenessResults::compute(mir, &liveness_map);
let constraint_sets = type_check::type_check(...);
nikomatsakis (Jul 16 2018 at 13:12, on Zulip):

I guess you could imagine us taking ownership of the map and packing it up with the LivenessResults

nikomatsakis (Jul 16 2018 at 13:13, on Zulip):

but I had initially imagined it more the way I wrote it: that the liveness code talks in terms of V mostly

nikomatsakis (Jul 16 2018 at 13:13, on Zulip):

and the caller maps it back to Local if they want

nikomatsakis (Jul 16 2018 at 13:13, on Zulip):

anyway minor nit, that's the place for it

nikomatsakis (Jul 16 2018 at 13:14, on Zulip):

point is that the type_check will also need this map -- either given to it as an extra parameter (it certainly has enough now) or else packaged up in the liveness variable

nikomatsakis (Jul 16 2018 at 13:14, on Zulip):

(is it clear what this NllLivenessMap is and how it would work?)

DPC (Jul 16 2018 at 13:26, on Zulip):

not really

nikomatsakis (Jul 16 2018 at 13:29, on Zulip):

@DPC ok so that is kind of the whole part of this PR :)

nikomatsakis (Jul 16 2018 at 13:30, on Zulip):

right now, in MIR, we have a list of variables, and each one has a type. So it might be like:

_0: u32
_1: Vec<&'x u32>
...
nikomatsakis (Jul 16 2018 at 13:30, on Zulip):

these indices (_0, _1) are the Local values

nikomatsakis (Jul 16 2018 at 13:30, on Zulip):

today, we compute liveness as a sets of these local values

nikomatsakis (Jul 16 2018 at 13:30, on Zulip):

so e.g. we might have -- at some basic block -- that _0 and _1 are both live

nikomatsakis (Jul 16 2018 at 13:31, on Zulip):

problem is -- in NLL, at least -- we don't really care whether _0 is live, because its type has no regions in it

nikomatsakis (Jul 16 2018 at 13:31, on Zulip):

we do care about _1 though

nikomatsakis (Jul 16 2018 at 13:31, on Zulip):

(with me so far?)

DPC (Jul 16 2018 at 13:31, on Zulip):

ah okay got it now

nikomatsakis (Jul 16 2018 at 13:31, on Zulip):

so the idea is to map these Local values to a smaller set of integers

nikomatsakis (Jul 16 2018 at 13:31, on Zulip):

containing just the ones with regions

nikomatsakis (Jul 16 2018 at 13:32, on Zulip):

so let's call that set RV0, RV1, etc

nikomatsakis (Jul 16 2018 at 13:32, on Zulip):

in that case, _0 maps to None

nikomatsakis (Jul 16 2018 at 13:32, on Zulip):

but _1 might map to Some(RV0)

nikomatsakis (Jul 16 2018 at 13:32, on Zulip):

(because it's the first Local that contains regions in its type)

DPC (Jul 16 2018 at 13:32, on Zulip):

got it. The "liveness" part in the name threw me off

nikomatsakis (Jul 16 2018 at 13:32, on Zulip):

ah I see yes

nikomatsakis (Jul 16 2018 at 13:32, on Zulip):

anyway, then we can compute liveness just over the RV values

nikomatsakis (Jul 16 2018 at 13:33, on Zulip):

(and map back to the original Local indices as needed)

nikomatsakis (Jul 16 2018 at 13:33, on Zulip):

/me stops explaining

DPC (Jul 16 2018 at 13:35, on Zulip):

got it. thanks

DPC (Jul 16 2018 at 18:22, on Zulip):

hey.. so what will be the contents of NllLivenessMap? currently I have :

pub(crate) struct NllLivenessMap {
    pub liveness: IndexVec<LocalWithRegion, LocalDecl>,

}

is this correct? (have to add the lifetime) @nikomatsakis

nikomatsakis (Jul 16 2018 at 18:30, on Zulip):

@DPC that doesn't look quite right

nikomatsakis (Jul 16 2018 at 18:30, on Zulip):

for one thing, we want to map indices to indices

nikomatsakis (Jul 16 2018 at 18:30, on Zulip):

so e.g., IndexVec<LocalWithRegion, Local>

DPC (Jul 16 2018 at 18:31, on Zulip):

ah so <LocalWithRegion, Local> ?

nikomatsakis (Jul 16 2018 at 18:31, on Zulip):

but I think you also want a reverse map

DPC (Jul 16 2018 at 18:31, on Zulip):

ah okay

nikomatsakis (Jul 16 2018 at 18:31, on Zulip):

so probably this:

DPC (Jul 16 2018 at 18:31, on Zulip):

i thought we were mapping it to the region from the Local

nikomatsakis (Jul 16 2018 at 18:31, on Zulip):
crate struct NllLivenessMap {
    from_local: IndexVec<Local, Option<LocalWithRegion>>,
    to_local: IndexVec<LocalWithRegion, LocalDecl>,
}
nikomatsakis (Jul 16 2018 at 18:32, on Zulip):

also, I think you will want to declare LocalWithRegion with newtype_index! (in case I didn't mention that before)

DPC (Jul 16 2018 at 18:32, on Zulip):

yeah I've done it already

DPC (Jul 16 2018 at 18:43, on Zulip):

so NllLivenessMap::compute(...) returns NllLivenesMap? and should that impl LiveVariableMap<LiveVar = V> ?

nikomatsakis (Jul 16 2018 at 18:45, on Zulip):

yes, except in this case V is LocalWithRegion

nikomatsakis (Jul 16 2018 at 18:45, on Zulip):

that is, it should impl LiveVariableMap<LiveVar = LocalWithRegion>

nikomatsakis (Jul 16 2018 at 18:45, on Zulip):

and I would expect compute to be something roughly like this:

nikomatsakis (Jul 16 2018 at 18:46, on Zulip):
fn compute(mir: &Mir<'tcx>) -> Self {
  let mut to_local = IndexVec::default();
  let from_local: IndexVec<_, _> = mir.local_decls.iter_enumerated.map(|local, local_decl| {
    if local_decl.ty.has_free_regions() { Some(to_local.push(local)) } else { None }
  }).collect();
  Self { from_local, to_local }
}
nikomatsakis (Jul 16 2018 at 18:47, on Zulip):

one question is how to concisely check whether the type of the local decl has free regions...

nikomatsakis (Jul 16 2018 at 18:47, on Zulip):

there should be some helper for that

nikomatsakis (Jul 16 2018 at 18:47, on Zulip):

@lqd did a recent PR, let me remember what we did there...

nikomatsakis (Jul 16 2018 at 18:48, on Zulip):

https://github.com/rust-lang/rust/pull/52037

nikomatsakis (Jul 16 2018 at 18:49, on Zulip):

ty.flags.intersects(TypeFlags::HAS_FREE_REGIONS)

nikomatsakis (Jul 16 2018 at 18:49, on Zulip):

anyway, the key idea is that we create a LocalWithRegion index for each local variable that has regions in its type...

nikomatsakis (Jul 17 2018 at 12:26, on Zulip):

@DPC were you able to make any further progress here btw?

DPC (Jul 17 2018 at 12:27, on Zulip):

yeah i'm working on it

DPC (Jul 17 2018 at 14:31, on Zulip):

so i'm assuming i have to write has_free_regions() in Ty which checks ty.flags.intersects(TypeFlags::HAS_FREE_REGIONS)?

nikomatsakis (Jul 17 2018 at 14:33, on Zulip):

you don't have to make the method, you can just insert the code as is

nikomatsakis (Jul 17 2018 at 14:33, on Zulip):

though a helper seems nice

nikomatsakis (Jul 17 2018 at 14:33, on Zulip):

there is actually probably a method like this

nikomatsakis (Jul 17 2018 at 14:34, on Zulip):

so, if you import the use rustc::ty::TypeFoldable trait

nikomatsakis (Jul 17 2018 at 14:34, on Zulip):

you could use has_free_regions

nikomatsakis (Jul 17 2018 at 14:34, on Zulip):

it's a bit "indirect" but ought to compile down to the same thing

nikomatsakis (Jul 17 2018 at 14:34, on Zulip):

if LLVM inlines enough :)

DPC (Jul 17 2018 at 17:40, on Zulip):

how do we handle the trait LiveVariableMap for NllLivenessMap?

DPC (Jul 17 2018 at 17:40, on Zulip):
impl LiveVariableMap for NllLivenessMap {
    type LiveVar = LocalWithRegion;

    fn from_local(&self, local: Local) -> Option<Self::LiveVar> {
    }

    fn from_live_var(&self, local: Self::LiveVar) -> Local {
    }

    fn num_variables(&self) -> usize {

    }
nikomatsakis (Jul 17 2018 at 18:04, on Zulip):

@DPC well

nikomatsakis (Jul 17 2018 at 18:05, on Zulip):

what is your def'n of NllLivenessMap? :)

nikomatsakis (Jul 17 2018 at 18:05, on Zulip):

if it is roughly the one I gave, then something like:

nikomatsakis (Jul 17 2018 at 18:05, on Zulip):
impl LiveVariableMap for NllLivenessMap {
    type LiveVar = LocalWithRegion;

    fn from_local(&self, local: Local) -> Option<Self::LiveVar> {
        self.from_local[local]
    }

    fn from_live_var(&self, local: Self::LiveVar) -> Local {
        self.to_local[local]
    }

    fn num_variables(&self) -> usize {
        self.to_local.len()
    }
}
DPC (Jul 17 2018 at 18:06, on Zulip):

ah uhmm

DPC (Jul 17 2018 at 18:38, on Zulip):

can i change dump_mir_results to accept a &LivenessResult<LocalWithRegion> instead of &LivenessResult<Local>?

nikomatsakis (Jul 17 2018 at 18:42, on Zulip):

@DPC yes

DPC (Jul 17 2018 at 19:01, on Zulip):

just have to settle some type (Local / LocalWithRegion) issues which i'll complete tomorrow :smile:

nikomatsakis (Jul 17 2018 at 19:11, on Zulip):

awesome!

DPC (Jul 18 2018 at 12:52, on Zulip):

hey. how to make let decl = &self.local_decls[local]; return a LocalWithRegion instead of Local?

DPC (Jul 18 2018 at 13:47, on Zulip):

ah solved it

nikomatsakis (Jul 18 2018 at 13:58, on Zulip):

sorry, was afk, back now

DPC (Jul 18 2018 at 15:30, on Zulip):

how to resolve this?

error[E0308]: mismatched types
   --> librustc/mir/visit.rs:379:42
    |
379 |                         self.visit_local(local, PlaceContext::StorageDead, location);
    |                                          ^^^^^ expected struct `mir::LocalWithRegion`, found struct `mir::Local`
...
819 | make_mir_visitor!(Visitor,);
    | ---------------------------- in this macro invocation
    |
    = note: expected type `&mir::LocalWithRegion`
               found type `&mir::Local`
nikomatsakis (Jul 18 2018 at 16:11, on Zulip):

@DPC can you push your latest diffs?

nikomatsakis (Jul 18 2018 at 16:11, on Zulip):

that error looks like you changed something you shouldn't have changed :)

DPC (Jul 18 2018 at 17:04, on Zulip):

done

nikomatsakis (Jul 18 2018 at 17:14, on Zulip):

pushed to your PR?

DPC (Jul 18 2018 at 17:16, on Zulip):

yep commit

nikomatsakis (Jul 18 2018 at 17:18, on Zulip):

ok -- the tl;dr is that you should not be modifying librustc/mir/mod.rs at all

nikomatsakis (Jul 18 2018 at 17:18, on Zulip):

Local remains the type for a local variable index

nikomatsakis (Jul 18 2018 at 17:18, on Zulip):

LocalWithRegion is a secondary index used only during NLL computation

DPC (Jul 18 2018 at 17:19, on Zulip):

ah thought so

DPC (Jul 18 2018 at 18:39, on Zulip):

Any clues how to resolve this?

error[E0271]: type mismatch resolving `<util::liveness::IdentityMap<'_, '_> as util::liveness::LiveVariableMap>::LiveVar == rustc::mir::LocalWithRegion`
   --> librustc_mir/borrow_check/nll/mod.rs:240:18
    |
240 |                 .simulate_block(&mir, bb, map, |location, local_set| {
    |                  ^^^^^^^^^^^^^^ expected struct `rustc::mir::Local`, found struct `rustc::mir::LocalWithRegion`
    |
    = note: expected type `rustc::mir::Local`
               found type `rustc::mir::LocalWithRegion`
nikomatsakis (Jul 18 2018 at 18:42, on Zulip):

@DPC latest diffs pushed?

DPC (Jul 18 2018 at 18:42, on Zulip):

not yet

nikomatsakis (Jul 18 2018 at 18:45, on Zulip):

would be helpful :)

nikomatsakis (Jul 18 2018 at 18:45, on Zulip):

sounds like though you have LivenessResults<Local> but you want LivenessResults<LocalWithRegion>

DPC (Jul 18 2018 at 18:46, on Zulip):

pushed

nikomatsakis (Jul 18 2018 at 18:47, on Zulip):

@DPC https://github.com/rust-lang/rust/pull/52115/files#r203489774

DPC (Jul 19 2018 at 11:52, on Zulip):

How to solve

error[E0308]: mismatched types
  --> librustc_mir/borrow_check/nll/type_check/liveness.rs:91:62
   |
91 |                     let live_local_ty = self.mir.local_decls[live_local].ty;
   |                                                              ^^^^^^^^^^ expected struct `rustc::mir::Local`, found struct `rustc::mir::LocalWithRegion`
   |
   = note: expected type `rustc::mir::Local`
              found type `rustc::mir::LocalWithRegion`
nikomatsakis (Jul 19 2018 at 12:01, on Zulip):

you'll need to translate that back to a Local using the map

nikomatsakis (Jul 19 2018 at 12:01, on Zulip):

that is, translate live_local back

DPC (Jul 19 2018 at 12:02, on Zulip):

ah okay

DPC (Jul 19 2018 at 15:25, on Zulip):

good news :tada:

DPC (Jul 19 2018 at 15:25, on Zulip):

it compiled without error :tada:

DPC (Jul 19 2018 at 15:25, on Zulip):

pushing right now

DPC (Jul 19 2018 at 15:41, on Zulip):

need to merge some conflicts and going off .. will push after an hour

nikomatsakis (Jul 19 2018 at 16:11, on Zulip):

@DPC awesome!

nikomatsakis (Jul 19 2018 at 18:05, on Zulip):

any update? :)

DPC (Jul 19 2018 at 18:08, on Zulip):

pushing rn

DPC (Jul 19 2018 at 18:30, on Zulip):

done

nikomatsakis (Jul 19 2018 at 18:43, on Zulip):

@DPC

tidy check
[00:03:46] tidy error: /checkout/src/librustc_mir/borrow_check/nll/mod.rs:411: line longer than 100 chars
[00:03:46] tidy error: /checkout/src/librustc_mir/util/liveness.rs:573: line longer than 100 chars
[00:03:46] tidy error: /checkout/src/librustc/mir/mod.rs: missing trailing newline
[00:03:47] some tidy checks failed
nikomatsakis (Jul 19 2018 at 18:48, on Zulip):

@DPC did you run tests locally at all? Looks great! I left a small nit

DPC (Jul 19 2018 at 18:48, on Zulip):

yeah i ran x.py check.

nikomatsakis (Jul 19 2018 at 18:49, on Zulip):

that just checks that it builds, I meant did you run e.g. x.py test --stage 1 src/test/ui or something like it

nikomatsakis (Jul 19 2018 at 18:49, on Zulip):

not a big deal, as travis will do it ;)

nikomatsakis (Jul 19 2018 at 18:49, on Zulip):

(but only once you fix the tidy errors)

nikomatsakis (Jul 19 2018 at 18:49, on Zulip):

I was hoping to do a try run soon to measure the impact

DPC (Jul 19 2018 at 18:49, on Zulip):

ah no i didn't run the test part yet

DPC (Jul 19 2018 at 18:57, on Zulip):

i usually just depend on travis for that xD

DPC (Jul 19 2018 at 19:06, on Zulip):

pushed the tidy part. Will push the nitbits tomorrow in the morning (you'll get them by the time you wake up )

nikomatsakis (Jul 20 2018 at 04:47, on Zulip):

@DPC I tried your branch locally and saw the problem, it was a 1-line fix so I pushed it

nikomatsakis (Jul 20 2018 at 04:47, on Zulip):

tests pass now

nikomatsakis (Jul 20 2018 at 04:47, on Zulip):

we were still using mir.local_decls.len() to get the number of live vars in one point

nikomatsakis (Jul 20 2018 at 04:48, on Zulip):

when we really out to be using map.num_variables()

nikomatsakis (Jul 21 2018 at 08:55, on Zulip):

perf results are available =)

nikomatsakis (Jul 21 2018 at 08:56, on Zulip):

er, I meant to say: there is one small change needed (delete some outdated tests, I think) before we can land this PR

DPC (Jul 21 2018 at 08:56, on Zulip):

ya i removed the liveness-* tests from mir-opt

nikomatsakis (Jul 21 2018 at 08:56, on Zulip):

oh, nice

nikomatsakis (Jul 21 2018 at 08:56, on Zulip):

still more failures

nikomatsakis (Jul 21 2018 at 08:57, on Zulip):

I guess we have to update the other tests

DPC (Jul 21 2018 at 08:57, on Zulip):

should i remove region-liveness-basic, region-liveness-two-disjoint-uses, region-subtyping-basic or those are good to stay?

nikomatsakis (Jul 21 2018 at 09:02, on Zulip):

hmm

nikomatsakis (Jul 21 2018 at 09:02, on Zulip):

honestly we can probably just remove them

nikomatsakis (Jul 21 2018 at 09:02, on Zulip):

I don't see them adding much value

nikomatsakis (Jul 21 2018 at 09:02, on Zulip):

or update them

nikomatsakis (Jul 21 2018 at 09:02, on Zulip):

I don't have a strong opinion :)

nikomatsakis (Jul 21 2018 at 09:02, on Zulip):

meh maybe keep them

nikomatsakis (Jul 21 2018 at 09:02, on Zulip):

do you know how to update them?

DPC (Jul 21 2018 at 09:03, on Zulip):

nah i haven't updated earlier. i'll just keep them as is i guess :D

nikomatsakis (Jul 21 2018 at 09:05, on Zulip):

well if they are passing, that's fine

nikomatsakis (Jul 21 2018 at 09:05, on Zulip):

but otherwise, there are some comments at the end of the test

nikomatsakis (Jul 21 2018 at 09:05, on Zulip):

these tests basically compare MIR output

nikomatsakis (Jul 21 2018 at 09:05, on Zulip):

so the comments have to be edited to match the current output

DPC (Jul 21 2018 at 11:43, on Zulip):

is there any convention for mod docblocks? I don't see any docblocs in other files.. //! or `///?

nikomatsakis (Jul 21 2018 at 11:44, on Zulip):

//!

nikomatsakis (Jul 21 2018 at 11:44, on Zulip):

you could also put those comments on some struct

DPC (Jul 21 2018 at 11:45, on Zulip):

i have //! on the mod docs and /// for structs, impls, fns

nikomatsakis (Jul 21 2018 at 11:47, on Zulip):

right

DPC (Jul 21 2018 at 11:55, on Zulip):

pushed :+1:

nikomatsakis (Jul 21 2018 at 13:14, on Zulip):

@DPC so we get this test failure still (just 1):

[00:55:52] ---- [mir-opt] mir-opt/nll/region-liveness-basic.rs stdout ----
[00:55:52] ---- [mir-opt] mir-opt/nll/region-liveness-basic.rs stdout ----
[00:55:52] thread '[mir-opt] mir-opt/nll/region-liveness-basic.rs' panicked at 'Did not find expected line, error: Mismatch in lines
[00:55:52] Current block: None
[00:55:52] Actual Line: "                                         | Live variables on entry to bb2[0]: []"
[00:55:52] Expected Line: "           | Live variables on entry to bb2[0]: [_1, _3]"
[00:55:52] Test Name: rustc.main.nll.0.mir
[00:55:52] Expected:
[00:55:52] ... (elided)
[00:55:52]    bb2: {
[00:55:52]            | Live variables on entry to bb2[0]: [_1, _3]
[00:55:52]        _2 = &'_#2r _1[_3];
[00:55:52]            | Live variables on entry to bb2[1]: [_2]
[00:55:52]        switchInt(const true) -> [false: bb4, otherwise: bb3];
[00:55:52]            | Live variables on exit from bb2: [_2]
[00:55:52] Actual:
[00:55:52] | Free Region Mapping

I would say either delete that test -- honestly, I think that's fine -- or else update it...

davidtwco (Jul 21 2018 at 13:14, on Zulip):

Wrong topic?

DPC (Jul 21 2018 at 13:14, on Zulip):

oops

DPC (Jul 21 2018 at 13:15, on Zulip):

sure will delete it

nikomatsakis (Jul 21 2018 at 13:15, on Zulip):

(yep, wrong topic, I moved to other thread)

davidtwco (Jul 21 2018 at 13:17, on Zulip):

@nikomatsakis Pushed the is_upvar_field_projection fix.

davidtwco (Jul 21 2018 at 13:18, on Zulip):

Do you have any thoughts on supporting the issue-7573 case?

Matthew Jasper (Jul 21 2018 at 13:19, on Zulip):

I ended up creating a function to determine if something is what the user thinks is an upvar in https://github.com/rust-lang/rust/pull/52405

nikomatsakis (Jul 21 2018 at 13:20, on Zulip):

@David Wood I think that if we examine the regions (and their categorizations) for which we are reporting the error, we ought to be able to handle issue-7573

nikomatsakis (Jul 21 2018 at 13:20, on Zulip):

I've been doing a local build though so I can inspect the debug output

nikomatsakis (Jul 21 2018 at 13:20, on Zulip):

it's just taking forever

nikomatsakis (Jul 21 2018 at 13:21, on Zulip):

@Matthew Jasper ah, interesting. which fn is that? I just r+'d that PR again btw, this time p=1

nikomatsakis (Jul 21 2018 at 13:21, on Zulip):

time to get some of this NLL work landed dang it

davidtwco (Jul 21 2018 at 13:23, on Zulip):

I was trying to work out how I'd check if a region was external or internal but couldn't see it anywhere.

nikomatsakis (Jul 21 2018 at 13:27, on Zulip):

universal_regions has some methods for that

nikomatsakis (Jul 21 2018 at 13:28, on Zulip):

e.g., is_local_free_region

davidtwco (Jul 21 2018 at 13:28, on Zulip):

Ah, great, not sure why I overlooked that.

nikomatsakis (Jul 21 2018 at 13:28, on Zulip):

or, more generally, region_classification

nikomatsakis (Jul 21 2018 at 13:28, on Zulip):

your branch is still building LLVM :( :(

davidtwco (Jul 21 2018 at 13:43, on Zulip):

I've got it to correctly classify the issue-7573.rs case now.

davidtwco (Jul 21 2018 at 13:48, on Zulip):

@nikomatsakis

error: borrowed data escapes outside of closure
  --> src/test/ui/borrowck/issue-7573.rs:32:9
   |
27 |     let mut lines_to_use: Vec<&CrateId> = Vec::new();
   |         ---------------- `lines_to_use` is declared here, outside of the closure body
28 |         //~^ NOTE cannot infer an appropriate lifetime
29 |     let push_id = |installed_id: &CrateId| {
   |                    ------------ `installed_id` is a reference that is only valid in the closure body
...
32 |         lines_to_use.push(installed_id);
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `installed_id` escapes the closure body here

error: aborting due to previous error
davidtwco (Jul 21 2018 at 13:49, on Zulip):

Still doesn't do anything for the E0621-does-not-trigger-for-closures.rs case that was in @pnkfelix's list in the issue.

davidtwco (Jul 21 2018 at 13:50, on Zulip):

Running tests then I'll push that change.

nikomatsakis (Jul 21 2018 at 13:58, on Zulip):

@David Wood that is awesome

davidtwco (Jul 21 2018 at 13:58, on Zulip):

However, there are other tests that it is updating the errors for, and some run into ICEs.

davidtwco (Jul 21 2018 at 13:59, on Zulip):

Seems related to what you were nervous about in the review.

davidtwco (Jul 21 2018 at 13:59, on Zulip):

It's struggling to find variable names for some stuff.

nikomatsakis (Jul 21 2018 at 13:59, on Zulip):

example?

Matthew Jasper (Jul 21 2018 at 14:01, on Zulip):

Three function is borriw_check:: mutability_errors::is_upvar, it will have to be moved if it's more generally useful.

Matthew Jasper (Jul 21 2018 at 14:01, on Zulip):

The function

davidtwco (Jul 21 2018 at 14:01, on Zulip):

Well, some of them have nicer messages but the spans aren't fantastic:

error: borrowed data escapes outside of closure
  --> src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.rs:33:20
   |
31 |     foo(cell, |cell_a, cell_x| {
   |                ------  ------ `cell_x` is a reference that is only valid in the closure body
   |                |
   |                `cell_a` is declared here, outside of the closure body
32 |         //~^ WARNING not reporting region error due to nll
33 |         cell_a.set(cell_x.get()); // forces 'x: 'a, error in closure
   |                    ^^^^^^^^^^^^ `cell_x` escapes the closure body here
davidtwco (Jul 21 2018 at 14:01, on Zulip):

And some just fail:

error: internal compiler error: librustc_mir/borrow_check/nll/region_infer/error_reporting/var_name.rs:40: can't find var name for free region '_#1r
  --> src/test/ui/nll/issue-48238.rs:21:5
   |
21 |     move || use_val(&orig); //~ ERROR
   |     ^^^^^^^^^^^^^^^^^^^^^^

thread 'rustc' panicked at 'Box<Any>', librustc_errors/lib.rs:502:9
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic
   7: rustc_errors::Handler::span_bug
   8: rustc::session::opt_span_bug_fmt::{{closure}}
   9: rustc::ty::context::tls::with_opt::{{closure}}
  10: rustc::ty::context::tls::with_context_opt
  11: rustc::ty::context::tls::with_opt
  12: rustc::session::opt_span_bug_fmt
  13: rustc::session::span_bug_fmt
  14: rustc_mir::borrow_check::nll::region_infer::error_reporting::var_name::<impl rustc_mir::borrow_check::nll::region_infer::RegionInferenceContext<'tcx>>::get_var_name_and_span_for_region::{{closure}}
  15: rustc_mir::borrow_check::nll::region_infer::error_reporting::var_name::<impl rustc_mir::borrow_check::nll::region_infer::RegionInferenceContext<'tcx>>::get_var_name_and_span_for_region
  16: rustc_mir::borrow_check::nll::region_infer::RegionInferenceContext::solve_inner
nikomatsakis (Jul 21 2018 at 14:02, on Zulip):

@David Wood it seems like cell_a the problem is more the label than the span

nikomatsakis (Jul 21 2018 at 14:02, on Zulip):

but that last one is I think the problem I was pointing out, yes

nikomatsakis (Jul 21 2018 at 14:02, on Zulip):

I suspect we can address that, too, by being careful with labels

davidtwco (Jul 21 2018 at 14:03, on Zulip):

I'll push this change anyway since it does address more cases and then we can work on fixing the regressions from this commit.

nikomatsakis (Jul 21 2018 at 14:03, on Zulip):

yeah

davidtwco (Jul 21 2018 at 14:05, on Zulip):

Is the fix for this just having it return Option<(Option<Symbol>, Span)> and then adding some cases to the labels for when we couldn't find a name?

nikomatsakis (Jul 21 2018 at 14:08, on Zulip):

well, maybe... the thing is, we can find the "thing that needs highlighting" (the return type) but it just doesn't have a name

nikomatsakis (Jul 21 2018 at 14:08, on Zulip):

so either we return None there, yes, and have some fallback

nikomatsakis (Jul 21 2018 at 14:08, on Zulip):

or maybe we can change the interface to be a bit more generic?

nikomatsakis (Jul 21 2018 at 14:08, on Zulip):

let's start with what we want the error messages to be, I think

nikomatsakis (Jul 21 2018 at 14:08, on Zulip):

and go from there

davidtwco (Jul 21 2018 at 14:09, on Zulip):

Well, we wouldn't know the span either? Since we get that from the upvar/argument index and that must be where the None is starting from for it to error where it is.

nikomatsakis (Jul 21 2018 at 14:09, on Zulip):

e.g., in that first case you mentioned, I think maybe something like

error: borrowed data escapes outside of closure
  --> src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.rs:33:20
   |
31 |     foo(cell, |cell_a, cell_x| {
   |                ------  ------ `cell_x` is a reference that is only valid in the closure body
   |                |
   |                XXX
32 |         //~^ WARNING not reporting region error due to nll
33 |         cell_a.set(cell_x.get()); // forces 'x: 'a, error in closure
   |                    ^^^^^^^^^^^^ `cell_x` escapes the closure body here

what should XXX be?

nikomatsakis (Jul 21 2018 at 14:09, on Zulip):

it seems like it'd be useful there to maybe print the fully elaborated type

nikomatsakis (Jul 21 2018 at 14:10, on Zulip):

e.g., "cell_a here has type Vec<&'1 u32>, where '1 is some lifetime outside the closure body"

davidtwco (Jul 21 2018 at 14:10, on Zulip):

I'm not sure, coming up with error messages or documentation comments isn't my forte.

nikomatsakis (Jul 21 2018 at 14:10, on Zulip):

one thing that makes me mildly nervous with the current style is that e.g. cell_x here has a type like Cell<&u32>

nikomatsakis (Jul 21 2018 at 14:11, on Zulip):

so saying that it "is" a reference feels a bit wrong

nikomatsakis (Jul 21 2018 at 14:11, on Zulip):

I could imagine something like:

csmoe (Jul 21 2018 at 14:11, on Zulip):

fully elaborated type issue
https://github.com/rust-lang/rust/issues/52533

nikomatsakis (Jul 21 2018 at 14:14, on Zulip):
error: borrowed data escapes outside of closure
  --> src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.rs:33:20
   |
31 |     foo(cell, |cell_a, cell_x| {
   |                ------  ------ `cell_x` has type `&'2 u32`, where `'2` is only valid in the closure body
   |                |
   |                `cell_a` has type `Vec<&'1 u32>`, where `'1` outlives the closure body
32 |         //~^ WARNING not reporting region error due to nll
33 |         cell_a.set(cell_x.get()); // forces 'x: 'a, error in closure
   |                    ^^^^^^^^^^^^ data with lifetime `'2` escapes the closure body here
nikomatsakis (Jul 21 2018 at 14:14, on Zulip):

@David Wood one option for this PR, if you like, is to put off some of these cases, just make it not ICE and we can revisit :)

DPC (Jul 21 2018 at 14:14, on Zulip):

@nikomatsakis should I delete region-liveness-two-disjoint-uses as well?

nikomatsakis (Jul 21 2018 at 14:15, on Zulip):

that said, if we were taking the approach I suggested with the Cell above, the return value case might then be something like:

error: internal compiler error: librustc_mir/borrow_check/nll/region_infer/error_reporting/var_name.rs:40: can't find var name for free region '_#1r
  --> src/test/ui/nll/issue-48238.rs:21:5
   |
21 |     move || use_val(&orig); //~ ERROR
   |          ^^ the return type is `&'1 u32`, where `'1` outlives the closure body
nikomatsakis (Jul 21 2018 at 14:15, on Zulip):

@DPC yes what the heck, do it :)

DPC (Jul 21 2018 at 14:15, on Zulip):

:D

nikomatsakis (Jul 21 2018 at 14:15, on Zulip):

sorry, gotta get a better template for that second one :)

davidtwco (Jul 21 2018 at 14:16, on Zulip):

I'm not sure. On the one hand I'd be nice to get it landed for the cases it does handle well. On the other hand, I'm not sure how to cleanly separate out the failing cases to fallback to something else.

nikomatsakis (Jul 21 2018 at 14:46, on Zulip):

yeah, true, I was wondering that

nikomatsakis (Jul 21 2018 at 14:46, on Zulip):

certainly for the ICE-like cases we could detect when there is no variable/upvar and return None back

davidtwco (Jul 21 2018 at 14:49, on Zulip):

I've got it so that nothing ICEs. There are a handful of not-fantastic cases with this change, but all improvements over what was there previously and nothing that is show-stopper enough for this PR, I reckon.

davidtwco (Jul 21 2018 at 14:51, on Zulip):

Pushed that.

nikomatsakis (Jul 21 2018 at 14:52, on Zulip):

I agree with your assessment =)

davidtwco (Jul 21 2018 at 15:02, on Zulip):

@nikomatsakis what about the error-codes/E0621-does-not-trigger-for-closures.rs case mentioned in the original issue?

nikomatsakis (Jul 21 2018 at 15:13, on Zulip):

uh...

nikomatsakis (Jul 21 2018 at 15:14, on Zulip):

what error does your branch get?

davidtwco (Jul 21 2018 at 15:15, on Zulip):

For what?

davidtwco (Jul 21 2018 at 15:15, on Zulip):

Oh.

davidtwco (Jul 21 2018 at 15:15, on Zulip):

Forgot I sent a message.

davidtwco (Jul 21 2018 at 15:15, on Zulip):

My branch doesn't change it at all AFAIK.

nikomatsakis (Jul 21 2018 at 15:16, on Zulip):

oh, is that your concern?

nikomatsakis (Jul 21 2018 at 15:16, on Zulip):

it doesn't seem to have a particularly nice error

nikomatsakis (Jul 21 2018 at 15:16, on Zulip):

in the AST land either

nikomatsakis (Jul 21 2018 at 15:16, on Zulip):

in fact it is sort of testing that you don't get a nice error:)

davidtwco (Jul 21 2018 at 15:17, on Zulip):

Yeah, I expected to see a change given that it was listed as one of the tests affected in the issue.

nikomatsakis (Jul 21 2018 at 15:18, on Zulip):

let me take a closer look, it seems like it could trigger

davidtwco (Jul 21 2018 at 15:20, on Zulip):

I reckon either way it's probably a issue for another PR, but curious nonetheless.

davidtwco (Jul 21 2018 at 17:46, on Zulip):

@nikomatsakis if you're happy with that PR, it's passed the tests now, could r+ and get it into the queue. :smiley:

davidtwco (Jul 21 2018 at 17:46, on Zulip):

(I wish we had a general compiler/lang stream in Zulip for the RFC 2008 stuff)

nikomatsakis (Jul 22 2018 at 12:33, on Zulip):

I guess we can make on :)

nikomatsakis (Jul 22 2018 at 12:34, on Zulip):

@DPC PR landed, nice!

DPC (Jul 22 2018 at 12:37, on Zulip):

yah i saw it :smiley:

DPC (Jul 22 2018 at 12:38, on Zulip):

thanks for the mentoring :+1:

nikomatsakis (Jul 22 2018 at 12:39, on Zulip):

you up for more? :)

DPC (Jul 22 2018 at 12:39, on Zulip):

maybe after a week

nikomatsakis (Jul 22 2018 at 12:40, on Zulip):

sounds good

davidtwco (Jul 22 2018 at 12:40, on Zulip):

I just realized I was the one in the wrong topic all of yesterday, oops, sorry.

DPC (Jul 22 2018 at 12:40, on Zulip):

haha.. @David Wood i saw it but didn't want to interrupt.

Last update: Nov 22 2019 at 00:05UTC