Stream: t-compiler/wg-nll

Topic: rustc-and-polonius integration


pnkfelix (May 18 2018 at 13:53, on Zulip):

one last thing

pnkfelix (May 18 2018 at 13:53, on Zulip):

maybe short

pnkfelix (May 18 2018 at 13:53, on Zulip):

maybe not

nikomatsakis (May 18 2018 at 13:53, on Zulip):

ok

pnkfelix (May 18 2018 at 13:53, on Zulip):

I've been chugging along on integration

nikomatsakis (May 18 2018 at 13:53, on Zulip):

ah right

nikomatsakis (May 18 2018 at 13:53, on Zulip):

you mentioned that

pnkfelix (May 18 2018 at 13:53, on Zulip):

But I'm worried my plan may be flawed

pnkfelix (May 18 2018 at 13:54, on Zulip):

My understanding is that polonius is meant to, "replace", both solving the region constraints and solving the borrowck dataflow

pnkfelix (May 18 2018 at 13:54, on Zulip):

so I had been thinking

pnkfelix (May 18 2018 at 13:54, on Zulip):

(and doing)

pnkfelix (May 18 2018 at 13:54, on Zulip):

1. refactor the code a bit so that those two stages can actually be swapped out and polonius swapped in

pnkfelix (May 18 2018 at 13:54, on Zulip):

2. implement swapping in polonius

pnkfelix (May 18 2018 at 13:55, on Zulip):

but while refactoring the code, I noted

pnkfelix (May 18 2018 at 13:55, on Zulip):

the actual data structure for the borrowck dataflow

pnkfelix (May 18 2018 at 13:55, on Zulip):

is the struct FlowAtLocation

pnkfelix (May 18 2018 at 13:56, on Zulip):

which wants want the ...

pnkfelix (May 18 2018 at 13:56, on Zulip):

well I'm working my way toward convincing myself this isn't a big problem

pnkfelix (May 18 2018 at 13:56, on Zulip):

but essentially

nikomatsakis (May 18 2018 at 13:56, on Zulip):

(on a related note, while chatting with @Santiago Pastorino on another topic, I was debating about how we might want to do the integration as well, in particular which relation we would want to use)

pnkfelix (May 18 2018 at 13:56, on Zulip):

the struct FlowAtLocation carries both the solved dataflow results

pnkfelix (May 18 2018 at 13:56, on Zulip):

as well as the original gen/kill sets used

pnkfelix (May 18 2018 at 13:57, on Zulip):

So i've been wondering/worrying

pnkfelix (May 18 2018 at 13:57, on Zulip):

whether I'm taking the wrong tack

nikomatsakis (May 18 2018 at 13:57, on Zulip):

hold on, I'm gonna branch a new topic ;)

nikomatsakis (May 18 2018 at 13:57, on Zulip):

meh never mind

pnkfelix (May 18 2018 at 13:57, on Zulip):

you can rename this ne

nikomatsakis (May 18 2018 at 13:57, on Zulip):

carry on

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

ok, well,

pnkfelix (May 18 2018 at 13:58, on Zulip):

somehow

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

oh, wow

pnkfelix (May 18 2018 at 13:58, on Zulip):

I won the race

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

neat

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

not sure how you did that

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

but i'm impressed

pnkfelix (May 18 2018 at 13:58, on Zulip):

its very cool

nikomatsakis (May 18 2018 at 13:59, on Zulip):

I guess you can "split off"?

pnkfelix (May 18 2018 at 13:59, on Zulip):

you ca choose

nikomatsakis (May 18 2018 at 13:59, on Zulip):

that is very cool

nikomatsakis (May 18 2018 at 13:59, on Zulip):

<3 Zulip

nikomatsakis (May 18 2018 at 13:59, on Zulip):

ok, so, anyway:

pnkfelix (May 18 2018 at 13:59, on Zulip):

whether to rename a single msg, all of the msgs, or all of the msgs inclduin and after the selected one

pnkfelix (May 18 2018 at 13:59, on Zulip):

okay so

pnkfelix (May 18 2018 at 13:59, on Zulip):

the whole point of this plan

nikomatsakis (May 18 2018 at 13:59, on Zulip):

did I tell you what I did before?

nikomatsakis (May 18 2018 at 13:59, on Zulip):

I think it's related

pnkfelix (May 18 2018 at 13:59, on Zulip):

would be to minimize how much code I have to rewrite/dup[licate

pnkfelix (May 18 2018 at 14:00, on Zulip):

You did tell me but I didn't look at it carefully

pnkfelix (May 18 2018 at 14:00, on Zulip):

maybe that was a big mistake

pnkfelix (May 18 2018 at 14:00, on Zulip):

I sort of jumped in with just getting the basics (debug flag, process spawning) going

pnkfelix (May 18 2018 at 14:00, on Zulip):

and figured the rest would come naturally

nikomatsakis (May 18 2018 at 14:00, on Zulip):

so what I did was to extend the "meta results" type (I forget what it's called)

nikomatsakis (May 18 2018 at 14:01, on Zulip):

to carry the borrow_live_at tuples

nikomatsakis (May 18 2018 at 14:01, on Zulip):

this btw is a rebased commit: link

nikomatsakis (May 18 2018 at 14:02, on Zulip):

I'm trying to find the part in question

pnkfelix (May 18 2018 at 14:02, on Zulip):

I guess the heart of my question: Is it sensible to try to convert the output of polonius into an instance of a struct FlowAtLocation

nikomatsakis (May 18 2018 at 14:02, on Zulip):

ah, Flows is the type I meant

nikomatsakis (May 18 2018 at 14:02, on Zulip):

I see

nikomatsakis (May 18 2018 at 14:02, on Zulip):

I did not do that

nikomatsakis (May 18 2018 at 14:02, on Zulip):

instead, I encapsulated (some of this may have landed, I forget) the access to the borrows field

pnkfelix (May 18 2018 at 14:02, on Zulip):

ah i see

nikomatsakis (May 18 2018 at 14:03, on Zulip):

we basically only (need to) ask one question: what borrows are in scope at the location L?

pnkfelix (May 18 2018 at 14:03, on Zulip):

you removed the borrows: FlowAtLocation

nikomatsakis (May 18 2018 at 14:03, on Zulip):

and that is readily accessible from the tuples

nikomatsakis (May 18 2018 at 14:03, on Zulip):

yep

pnkfelix (May 18 2018 at 14:03, on Zulip):

okay

nikomatsakis (May 18 2018 at 14:03, on Zulip):

you may not want to do that :)

pnkfelix (May 18 2018 at 14:03, on Zulip):

maybe the answer here

pnkfelix (May 18 2018 at 14:03, on Zulip):

is to abstract over the two

pnkfelix (May 18 2018 at 14:03, on Zulip):

via a trait

nikomatsakis (May 18 2018 at 14:03, on Zulip):

right

pnkfelix (May 18 2018 at 14:03, on Zulip):

okay

nikomatsakis (May 18 2018 at 14:03, on Zulip):

well, or a bool

nikomatsakis (May 18 2018 at 14:03, on Zulip):

but yes

nikomatsakis (May 18 2018 at 14:03, on Zulip):

somehow

pnkfelix (May 18 2018 at 14:03, on Zulip):

well

pnkfelix (May 18 2018 at 14:03, on Zulip):

my hope

nikomatsakis (May 18 2018 at 14:03, on Zulip):

one tricky bit

nikomatsakis (May 18 2018 at 14:03, on Zulip):

that is not too tricky

pnkfelix (May 18 2018 at 14:03, on Zulip):

is that I don't want a bunch of duplicated logic

nikomatsakis (May 18 2018 at 14:03, on Zulip):

is the generator code

pnkfelix (May 18 2018 at 14:03, on Zulip):

in the borrow-checker

nikomatsakis (May 18 2018 at 14:04, on Zulip):

right, you shouldn't need that

nikomatsakis (May 18 2018 at 14:04, on Zulip):

what I meant is

nikomatsakis (May 18 2018 at 14:04, on Zulip):

a bool in Flows

nikomatsakis (May 18 2018 at 14:04, on Zulip):

and basically a method like borrows_at_location(...)

pnkfelix (May 18 2018 at 14:04, on Zulip):

hmm about which thing to consult, I see

pnkfelix (May 18 2018 at 14:04, on Zulip):

yes okay

nikomatsakis (May 18 2018 at 14:04, on Zulip):

actually you wuld probably want it o track the current location then

nikomatsakis (May 18 2018 at 14:04, on Zulip):

(right now I think it doesn't)

nikomatsakis (May 18 2018 at 14:04, on Zulip):

I considered going that way but didn't

pnkfelix (May 18 2018 at 14:04, on Zulip):

alright, that's enough for me to jump back in I think

pnkfelix (May 18 2018 at 14:04, on Zulip):

I at least don't feel like I'm totally off base.

nikomatsakis (May 18 2018 at 14:04, on Zulip):

does this link work for you?

nikomatsakis (May 18 2018 at 14:04, on Zulip):

I just wanted to show you one bit

pnkfelix (May 18 2018 at 14:05, on Zulip):

yes it does

nikomatsakis (May 18 2018 at 14:05, on Zulip):

there are two bits of code that call with_outgoing_borrows

nikomatsakis (May 18 2018 at 14:05, on Zulip):

I rewrote them in terms of finding a point

nikomatsakis (May 18 2018 at 14:05, on Zulip):

which anyway I think is clearer

nikomatsakis (May 18 2018 at 14:05, on Zulip):

in the one case, it's a Yield terminator: and you basically want the borrows still live when we resume

nikomatsakis (May 18 2018 at 14:05, on Zulip):

in the other, it's a Return terminator, and the borrows live on entry suffice

nikomatsakis (May 18 2018 at 14:05, on Zulip):

I don't know why it was using with_outgoing_borrows anyway except to be less efficient

pnkfelix (May 18 2018 at 14:06, on Zulip):

I think ariel added that

pnkfelix (May 18 2018 at 14:06, on Zulip):

which means there was probably a good reason

pnkfelix (May 18 2018 at 14:06, on Zulip):

...

pnkfelix (May 18 2018 at 14:06, on Zulip):

let me look

nikomatsakis (May 18 2018 at 14:06, on Zulip):

I can't imagine what it would be, but maybe. I think it's actively wrong in the new analysis tho

nikomatsakis (May 18 2018 at 14:06, on Zulip):

I mean...there is no point after the return anyway

nikomatsakis (May 18 2018 at 14:06, on Zulip):

but if there was, then e.g. the slot _0 would be dead there

nikomatsakis (May 18 2018 at 14:06, on Zulip):

and we don't want that

nikomatsakis (May 18 2018 at 14:06, on Zulip):

anyway, it doesn't matter

nikomatsakis (May 18 2018 at 14:06, on Zulip):

you'll see, I just wanted to answer that question

nikomatsakis (May 18 2018 at 14:06, on Zulip):

before it came up

nikomatsakis (May 18 2018 at 14:06, on Zulip):

I have to run now too

nikomatsakis (May 18 2018 at 14:07, on Zulip):

but I agree with your assessment that if @arielb1 did it, there is prob a reason

pnkfelix (May 18 2018 at 14:07, on Zulip):

ok

pnkfelix (May 18 2018 at 14:08, on Zulip):

gotta run, ttyl

pnkfelix (May 18 2018 at 14:09, on Zulip):

(ah, the specific case of generators was Zoxc)

pnkfelix (May 18 2018 at 14:09, on Zulip):

https://github.com/nikomatsakis/rust/commit/55c6c88782cfcce9b593c431200dad5f05bd9125

pnkfelix (May 18 2018 at 14:10, on Zulip):

"across yield" may be the relevant thing there ...? They may have thought using the entry set would not suffice?

pnkfelix (May 18 2018 at 14:10, on Zulip):

(now I'm just trying to read minds... via git logs ... not a good sign.)

nikomatsakis (May 18 2018 at 14:11, on Zulip):

for yields it makes sense

nikomatsakis (May 18 2018 at 14:11, on Zulip):

and in that case I used the resume point

pnkfelix (May 18 2018 at 14:11, on Zulip):

oh okay, there's the other with Resume/Return/GeneratorDrop

nikomatsakis (May 18 2018 at 14:12, on Zulip):

that's on this line

pnkfelix (May 18 2018 at 14:12, on Zulip):

and that's this: https://github.com/nikomatsakis/rust/commit/af8c2339cfb26363610899e6bffbd1913e83db08

pnkfelix (May 18 2018 at 14:12, on Zulip):

in terms of what commit added it

pnkfelix (May 18 2018 at 14:12, on Zulip):

(right?)

pnkfelix (May 18 2018 at 14:13, on Zulip):

okay really gotta go, shouldn't have taken the bait! :)

nikomatsakis (May 23 2018 at 17:39, on Zulip):

so @qmx, @lqd, @Reed Koser, @Keith Yeung or anyone else — are any of you interested in working on the rustc polonius I described? I think somebody should be on this. @qmx you mentioned having free time on Friday, might be a good fit for that time... (That would also give a bit of time for @Reed Koser's PRs to land...)

nikomatsakis (May 23 2018 at 17:39, on Zulip):

otherwise, I think we should probably put more energy into the diagnostics, but we'll have to open more issues there

qmx (May 23 2018 at 17:39, on Zulip):

yep, I have the entire Friday for rust work

qmx (May 23 2018 at 17:40, on Zulip):

(and potentially the weekend also)

Santiago Pastorino (May 23 2018 at 17:40, on Zulip):

@nikomatsakis I can also help here

Santiago Pastorino (May 23 2018 at 17:40, on Zulip):

have plenty of time to do rustc

lqd (May 23 2018 at 17:41, on Zulip):

it's going to be tougher for me — I'm finishing the frontend now, and leaving for Rustfest on Friday

nikomatsakis (May 23 2018 at 17:41, on Zulip):

I guess we should break it into steps

nikomatsakis (May 23 2018 at 17:41, on Zulip):

I think I mentioned that we could start just by breaking out the AllFacts and having rustc generate that

Santiago Pastorino (May 23 2018 at 17:41, on Zulip):

we could even pair with @qmx if needed

nikomatsakis (May 23 2018 at 17:41, on Zulip):

(right now, they each have their own copy of that same data structure)

nikomatsakis (May 23 2018 at 17:42, on Zulip):

I'd be happy to be around on Friday to consult, particualrly in the morning hours

lqd (May 23 2018 at 17:42, on Zulip):

this link describes the expected crate organization I believe ? https://paper.dropbox.com/doc/Non-lexical-lifetimes-NLL-Triage-Em2cJrvxQMMFWLE7lE5Be#:uid=513600694358508882742191&h2=General-overview:

lqd (May 23 2018 at 17:42, on Zulip):

err, apart from it being the wrong paragraph :3

nikomatsakis (May 23 2018 at 17:49, on Zulip):

right so I guess the very first step is breaking out a polonius-engine crate that contains AllFacts and it's supporting things

nikomatsakis (May 23 2018 at 17:49, on Zulip):

maybe we should open up an issue on polonius for tracking this

nikomatsakis (May 23 2018 at 17:51, on Zulip):

oh, I am reminded that I wanted help landing https://github.com/rust-lang/rust/pull/50938

Santiago Pastorino (May 23 2018 at 17:51, on Zulip):

Niko, can I help you in some way with the removal of Location:All?

nikomatsakis (May 23 2018 at 17:54, on Zulip):

filed https://github.com/rust-lang-nursery/polonius/issues/44 which contains the rustc-polonius integration steps

nikomatsakis (May 23 2018 at 17:54, on Zulip):

broken down very fine-grain

nikomatsakis (May 23 2018 at 17:54, on Zulip):

@Santiago Pastorino you could :) I'll open a new topic :)

Reed Koser (May 23 2018 at 17:55, on Zulip):

I'll probably have time to work on splitting Polonius up before Friday, assuming @qmx doesn't mind =)

Reed Koser (May 23 2018 at 17:55, on Zulip):

at least the first task can be completed before my refactor lands

nikomatsakis (May 23 2018 at 17:55, on Zulip):

sounds good

nikomatsakis (May 23 2018 at 17:55, on Zulip):

in my ideal world, we'd actually get the first 1 or 2 done before Friday

nikomatsakis (May 23 2018 at 17:55, on Zulip):

since they are minor

Reed Koser (May 23 2018 at 17:56, on Zulip):

:+1: should definitely be doable. Hopefully I'll get to it either tonight or tomorrow

qmx (May 23 2018 at 17:58, on Zulip):

@Reed Koser I don't mind :)

Reed Koser (May 23 2018 at 18:00, on Zulip):

cool, I'll try to get a patch out either tonight or tomorrow then. I'll also have a lot of time to hack on rustc stuff over the weekend, we get Monday off =)

Keith Yeung (May 23 2018 at 18:10, on Zulip):

@nikomatsakis i'm guessing you want polonius to generate a library on top of a binary?

nikomatsakis (May 23 2018 at 18:11, on Zulip):

I was proposing that we have a shared subcrate called polonius-engine which will be that library

Keith Yeung (May 23 2018 at 18:42, on Zulip):

(obligatory :frog: comment)

Reed Koser (May 24 2018 at 03:19, on Zulip):

https://github.com/rust-lang-nursery/polonius/pull/46

Reed Koser (May 24 2018 at 03:19, on Zulip):

Depending on what order this lands in, it'll invalidate all other PRs because I moved things so that the repo could become a cargo workspace

Reed Koser (May 24 2018 at 03:30, on Zulip):

so either @lqd or myself is going to have some cleanup to do =)

Jake Goulding (May 24 2018 at 03:32, on Zulip):

https://www.youtube.com/watch?v=VQ1xHmQgXDM

Reed Koser (May 24 2018 at 03:34, on Zulip):

lol
It's not a huge deal either way, I think git should be able to figure out some of it (if not all of it) since most things are just file renames

lqd (May 24 2018 at 06:02, on Zulip):

yeah I thought this reorg would impact my PRs as I also switched to a cargo workspace :simple_smile: I thought we would leave the majority of the crate as-is — just moving AllFacts — to a structure similar to chalk's

Reed Koser (May 24 2018 at 06:10, on Zulip):

Indeed, that's what my PR does. I believe the eventual goal is to move all the computation into polonius-engine, but that's somewhat blocked on @Frank McSherry getting Datafrog licensed, transferred to rust-lang-nursery, and up on crates.io to my knowledge =P. We took slightly different approaches to moving things to workspaces (I moved the frontend into its own folder, looks like you just added a [workspace] tag. The merge should be relatively painless though.

Reed Koser (May 24 2018 at 06:12, on Zulip):

I'm not sure which PR makes the most sense to land first, but either way I'm willing to do the rebase. My impression is that you'll have time to get to it first though, so perhaps merging my PR and then yours, with you doing the rebase, makes more sense as that'll get work that depends on either unblocked faster. If you don't want to deal with the rebase, I would say merge your PR and then I'll rebase mine (probably around 7-8PM PDT) to include your changes

Reed Koser (May 24 2018 at 06:14, on Zulip):

or I could just not move everything. If that's the route we want to go I can probably get that committed while waiting for my eggs to fry tomorrow morning ;)

lqd (May 24 2018 at 06:15, on Zulip):

:simple_smile: — It's fine, whichever merges first, I don't mind

Reed Koser (May 24 2018 at 06:16, on Zulip):

I guess it'll be up to the gods cough @nikomatsakis cough then

lqd (May 24 2018 at 06:16, on Zulip):

(it looks you need to fix the tests TravIs is unhappy)

lqd (May 24 2018 at 06:17, on Zulip):

haha :simple_smile:

Reed Koser (May 24 2018 at 06:17, on Zulip):

oh I probably completely broke travis didn't I

lqd (May 24 2018 at 06:17, on Zulip):

probably just looking at the wrong folder to find the facts

Reed Koser (May 24 2018 at 06:18, on Zulip):

yep, I broke all 2 tests

Reed Koser (May 24 2018 at 06:37, on Zulip):

I redid my PR so that the repo structure ends up more like chalk's. It should pass tests now, and I won't be breaking git history as badly

lqd (May 24 2018 at 06:41, on Zulip):

I think you have some leftover code in there ? I see some timely analysis

Reed Koser (May 24 2018 at 06:45, on Zulip):

oh god that got picked up didn't it

Reed Koser (May 24 2018 at 06:46, on Zulip):

that was me playing around with timely a few weeks ago and I guess it was left sitting in my tree

Reed Koser (May 24 2018 at 06:46, on Zulip):

/me really needs to stop being so sloppy with patches...

Reed Koser (May 24 2018 at 06:46, on Zulip):

I think I accidentally downgraded :frog: too, I'll fix those

Reed Koser (May 24 2018 at 06:52, on Zulip):

should be fixed now. I really need to get in the habit of _carefully_ reviewing my own patches before sending them upstream

nikomatsakis (May 24 2018 at 09:33, on Zulip):

@Reed Koser ok merged — I changed authors of both projects to "The Rust Developers", but maybe it should be "The Polonius Developers"...?

nikomatsakis (May 24 2018 at 09:34, on Zulip):

if we really wanted to go crazy with crates, we could make that polonius-engine crate be polonius-facts — then we could start in right now on splitting out the datafrog code into polonius-engine, which seems interesting (and rustc could just depend on polonius-facts)

nikomatsakis (May 24 2018 at 09:34, on Zulip):

but I don't know if that's worth it :)

nikomatsakis (May 24 2018 at 09:34, on Zulip):

well, alternatively, we can just try rewriting the datafrog code to be generic

lqd (May 24 2018 at 09:39, on Zulip):

I had put authors = ["Rust Compiler Team", "Polonius developers"] in the polonius-parser crate, I hope that's ok

lqd (May 24 2018 at 09:40, on Zulip):

(very similar to Chalk in any case)

nikomatsakis (May 24 2018 at 09:42, on Zulip):

yeah, seems also ok

nikomatsakis (May 24 2018 at 09:42, on Zulip):

it's not that important in the end :)

nikomatsakis (May 24 2018 at 09:42, on Zulip):

I guess it depends on whether we consider the polonius devs a distinct set from rust devs :)

nikomatsakis (May 24 2018 at 09:42, on Zulip):

so I missed around a bit with making stuff generic

nikomatsakis (May 24 2018 at 09:42, on Zulip):

it is pretty easy to do but we have to do a bit of cleanup:

nikomatsakis (May 24 2018 at 09:43, on Zulip):

separating out the dump code for example — or else doing a more involved bit of genericity

nikomatsakis (May 24 2018 at 09:43, on Zulip):

I..oh I have a few more minutes, maybe I'll just keep hacking on this for fun

nikomatsakis (May 24 2018 at 09:43, on Zulip):

/me should probably be doing something else :)

nikomatsakis (May 24 2018 at 09:50, on Zulip):

nah, I don't have time to do this properly, i'll just stop; I'm not sure where Dump ought to go

nikomatsakis (May 24 2018 at 09:51, on Zulip):

https://github.com/nikomatsakis/borrow-check/tree/generic-over-r-l-p in case you're curious

nikomatsakis (May 24 2018 at 09:51, on Zulip):

mostly just works fine

nikomatsakis (May 24 2018 at 09:52, on Zulip):

I think I would like to introduce a

trait Atoms {
  type Region;
  type Loan;
  type Point;
}

and maybe have that trait also contain enough stuff so that the dump code can operate

nikomatsakis (May 24 2018 at 09:53, on Zulip):

in particular, the "untern" logic

nikomatsakis (May 24 2018 at 09:53, on Zulip):

seems like rustc would also want to be able to dump its results? maybe? or maybe not...

nikomatsakis (May 24 2018 at 09:53, on Zulip):

before I was integrating them into mir-dump, so maybe we should just pull the dump code out into polonius and make it not be generic

nikomatsakis (May 24 2018 at 09:59, on Zulip):

(ok, that works)

nikomatsakis (May 24 2018 at 10:01, on Zulip):

@lqd are you not a member of the org...?

lqd (May 24 2018 at 10:01, on Zulip):

no, just an innocent bystander :3

nikomatsakis (May 24 2018 at 12:22, on Zulip):

@lqd ok I think I will merge https://github.com/rust-lang-nursery/polonius/pull/47 then (I removed the stats option)

lqd (May 24 2018 at 12:23, on Zulip):

:thumbs_up: (and then I'll rebase the frontend again :p)

Keith Yeung (May 24 2018 at 12:25, on Zulip):

hmm... how do you include external crates into rustc?

Keith Yeung (May 24 2018 at 12:25, on Zulip):

should we include them as git submodules?

Keith Yeung (May 24 2018 at 12:30, on Zulip):

wait nvm, we can definitely just import the crate via cargo

Keith Yeung (May 24 2018 at 12:31, on Zulip):

but i don't think polonius is published on crates.io yet, so we'll have to use it as a git dependency

nikomatsakis (May 24 2018 at 12:52, on Zulip):

but i don't think polonius is published on crates.io yet, so we'll have to use it as a git dependency

now that we have a polonius-engine we can push to crates.io

Reed Koser (May 24 2018 at 14:56, on Zulip):

No strong opinions on the authorship here, Rust Project and Polonius developers seems to make sense (since I don't think everyone that hacks on Polonius also hacks on rustc? I could be wrong though)

nikomatsakis (May 24 2018 at 14:57, on Zulip):

I view them both as part of the Rust umbrella :) but yeah

nikomatsakis (May 24 2018 at 14:58, on Zulip):

in general I want to move towards rustc as a kind of a "federation" of projects in any case

nikomatsakis (May 24 2018 at 14:58, on Zulip):

so e.g. chalk, polonius would be a part of rustc of course but carry their own distinct identities

nikomatsakis (May 24 2018 at 14:59, on Zulip):

(as I understand it, this is sort of how servo development works)

Jake Goulding (May 24 2018 at 15:15, on Zulip):

should be fixed now. I really need to get in the habit of _carefully_ reviewing my own patches before sending them upstream

I use git add -pu to review before ever committing

Reed Koser (May 24 2018 at 15:18, on Zulip):

Yeah, I have the tools. It's jus a matter of developing the muscle memory and mental fortitude to review my own patches without having my eyes glaze over =P

Reed Koser (May 24 2018 at 15:20, on Zulip):

I've been writing code for 5 or 6 years, but it's only in the last 12 months or so that I've started collaborating on larger projects. Part of the reason I started working on Rust back in January was just to try and keep those skills sharp

Reed Koser (May 24 2018 at 15:21, on Zulip):

Unfortunately the CS program I attend (and probably most others, at least in the US) tend to focus more on developing individual skill and not on how to be an effective member of a large software organization

Reed Koser (May 24 2018 at 15:22, on Zulip):

I guess s/keep skills sharp/develop them at all honestly

nikomatsakis (May 24 2018 at 16:06, on Zulip):

so @qmx you were asking about tomorrow =) we have extracted the AllFacts out now into a subcrate polonius-engine

nikomatsakis (May 24 2018 at 16:06, on Zulip):

I think the next step was to make rustc use it

nikomatsakis (May 24 2018 at 16:07, on Zulip):

currently our fact definition is here

nikomatsakis (May 24 2018 at 16:07, on Zulip):

we should be able to just add polonius-engine as a pre-req

nikomatsakis (May 24 2018 at 16:07, on Zulip):

this could be done today

nikomatsakis (May 24 2018 at 16:07, on Zulip):

should publish polonius_engine to crates.io...

qmx (May 24 2018 at 16:08, on Zulip):

the crate is named with dashes right?

qmx (May 24 2018 at 16:08, on Zulip):

/me always forgets the naming rules

nikomatsakis (May 24 2018 at 16:08, on Zulip):

heh :)

nikomatsakis (May 24 2018 at 16:08, on Zulip):

crate names with dashes

nikomatsakis (May 24 2018 at 16:08, on Zulip):

is my preference :)

nikomatsakis (May 24 2018 at 16:08, on Zulip):

though my true preference is to make crates.io translate them silently

nikomatsakis (May 24 2018 at 16:08, on Zulip):

i.e., you give either - or _, you see -

Jake Goulding (May 24 2018 at 16:08, on Zulip):

https://github.com/rust-lang-nursery/api-guidelines/issues/29

qmx (May 24 2018 at 16:09, on Zulip):

mandatory would be even better

nikomatsakis (May 24 2018 at 16:09, on Zulip):

I just don't want to have to think about it ever

nikomatsakis (May 24 2018 at 16:09, on Zulip):

but I want to see - :)

nikomatsakis (May 24 2018 at 16:09, on Zulip):

anyway, whatever..

Jake Goulding (May 24 2018 at 16:09, on Zulip):

cargo add translates them

Jake Goulding (May 24 2018 at 16:09, on Zulip):

but i agree with dashes

nikomatsakis (May 24 2018 at 16:10, on Zulip):

there are some fights in which I am (thanfully) a bystander

nikomatsakis (May 24 2018 at 16:10, on Zulip):

and this is one :)

Jake Goulding (May 24 2018 at 16:10, on Zulip):

for now

nikomatsakis (May 24 2018 at 16:12, on Zulip):

I forget what we decided re: authors

nikomatsakis (May 24 2018 at 16:13, on Zulip):

authors = ["The Rust Project Developers", "Polonius Developers"]

nikomatsakis (May 24 2018 at 16:13, on Zulip):

?

nikomatsakis (May 24 2018 at 16:13, on Zulip):

personally I feel polonius is a piece of the Rust project, so those are redundant, but it seems ok :)

nikomatsakis (May 24 2018 at 16:13, on Zulip):

(I was gonna push this crate to crates.io)

qmx (May 24 2018 at 16:13, on Zulip):

push it

Santiago Pastorino (May 24 2018 at 16:15, on Zulip):

@nikomatsakis about the integration let me know how can I help

Santiago Pastorino (May 24 2018 at 16:15, on Zulip):

or if I can help @qmx in any way

nikomatsakis (May 24 2018 at 16:15, on Zulip):

I will push both polonius (executable) and polonius-engine (library)

nikomatsakis (May 24 2018 at 16:16, on Zulip):

the latter just to save the name :)

Jake Goulding (May 24 2018 at 16:19, on Zulip):

@nikomatsakis while you are doing housekeeping, you should add the Polonus quote and a nice Wikipedia link to the README :heart_eyes_cat:

nikomatsakis (May 24 2018 at 16:20, on Zulip):

I kinda wanted people to google it for themselves :)

nikomatsakis (May 24 2018 at 16:20, on Zulip):

but I guess I wouldn't turn away a PR ;)

Jake Goulding (May 24 2018 at 16:21, on Zulip):

Subtle hint noted!

nikomatsakis (May 24 2018 at 16:21, on Zulip):

oh, we can't publish polonius until datafrog is published

nikomatsakis (May 24 2018 at 16:21, on Zulip):

but polonius-engine is fine

Jake Goulding (May 24 2018 at 16:21, on Zulip):

Also, this thread title is "polonus" and you just said "polonius". May be worth 2x checking?

nikomatsakis (May 24 2018 at 16:22, on Zulip):

https://crates.io/crates/polonius-engine

nikomatsakis (May 24 2018 at 16:22, on Zulip):

it is polonius

Jake Goulding (May 24 2018 at 16:22, on Zulip):

whew

qmx (May 24 2018 at 16:23, on Zulip):

@nikomatsakis can't you just publish datafrog and put @Frank McSherry as an owner also?

qmx (May 24 2018 at 16:23, on Zulip):

so we save the name

nikomatsakis (May 24 2018 at 16:23, on Zulip):

I suppose I could :)

nikomatsakis (May 24 2018 at 16:23, on Zulip):

or I could just fork it or something

nikomatsakis (May 24 2018 at 16:23, on Zulip):

he seemed to indicate that he wants to transfer it over anyway

nikomatsakis (May 24 2018 at 16:24, on Zulip):

I could also make a dummy project

nikomatsakis (May 24 2018 at 16:24, on Zulip):

to save the name ;)

Jake Goulding (May 24 2018 at 16:28, on Zulip):

sed -i /-engine// Cargo.toml && cargo publish

nikomatsakis (May 24 2018 at 16:30, on Zulip):

indeed

nikomatsakis (May 24 2018 at 16:30, on Zulip):

@Santiago Pastorino so if you wanted to port rustc in prep for tomorrow I'd be in favor of that

nikomatsakis (May 24 2018 at 16:31, on Zulip):

do you know what I mean by that?

Santiago Pastorino (May 24 2018 at 16:31, on Zulip):

I guess ...

nikomatsakis (May 24 2018 at 16:32, on Zulip):

basically what I wrote here

Santiago Pastorino (May 24 2018 at 16:33, on Zulip):

for some reason the link does not navigate for me

qmx (May 24 2018 at 16:33, on Zulip):

yeah, link fail

nikomatsakis (May 24 2018 at 16:34, on Zulip):

https://rust-lang.zulipchat.com/#narrow/stream/122657-wg-nll/subject/rustc-and-polonus.20integration/near/127035763

nikomatsakis (May 24 2018 at 16:34, on Zulip):

curious

nikomatsakis (May 24 2018 at 16:34, on Zulip):

at 12:06pm :)

Santiago Pastorino (May 24 2018 at 16:36, on Zulip):

the links does not work for me

Santiago Pastorino (May 24 2018 at 16:36, on Zulip):

any of them

Santiago Pastorino (May 24 2018 at 16:36, on Zulip):

it may be a Firefox plugin or something

Santiago Pastorino (May 24 2018 at 16:37, on Zulip):

anyway ... will tackle that

qmx (May 24 2018 at 16:39, on Zulip):

yea, link does not work

nikomatsakis (May 24 2018 at 16:40, on Zulip):

huh, that's a big minus for Zulip :)

Santiago Pastorino (May 24 2018 at 16:40, on Zulip):

and also the time for me is 1.06pm :P

nikomatsakis (May 24 2018 at 16:41, on Zulip):

clearly it should auto-translate things that look like times between timezones

Santiago Pastorino (May 24 2018 at 16:41, on Zulip):

hehehe

nikomatsakis (May 24 2018 at 18:14, on Zulip):

https://crates.io/crates/datafrog

nikomatsakis (May 24 2018 at 18:14, on Zulip):

it's a thing :)

nikomatsakis (May 24 2018 at 18:16, on Zulip):

as is https://crates.io/crates/polonius

nikomatsakis (May 24 2018 at 18:26, on Zulip):

@Santiago Pastorino asked me over privmsg:

is rustc currently using a thing similar to IntererTables and that stuff in Polonius?

you shouldn't need that stuff

nikomatsakis (May 24 2018 at 18:27, on Zulip):

or maybe I'm not sure 100% sure what you mean

nikomatsakis (May 24 2018 at 18:27, on Zulip):

but the goal of that stuff was to convert strings into integers

nikomatsakis (May 24 2018 at 18:27, on Zulip):

but in rustc the types are already small integers...

nikomatsakis (May 24 2018 at 18:27, on Zulip):

...and the dumping code I think already has some logic for converting them to strings that I guess can be used just fine?

Santiago Pastorino (May 24 2018 at 19:20, on Zulip):

@nikomatsakis back

Santiago Pastorino (May 24 2018 at 19:20, on Zulip):

I see

Santiago Pastorino (May 24 2018 at 19:21, on Zulip):

I needed to implement Atom

Santiago Pastorino (May 24 2018 at 19:21, on Zulip):

but yeah, I can make stuff simpler

Santiago Pastorino (May 24 2018 at 19:22, on Zulip):

just return self

Santiago Pastorino (May 24 2018 at 19:24, on Zulip):

isn't there already an index() method for RegionVid and such

nikomatsakis (May 24 2018 at 19:54, on Zulip):

@Santiago Pastorino yes not sure what it's called though

nikomatsakis (May 24 2018 at 19:54, on Zulip):

you wrote the dang macro :stuck_out_tongue:

Santiago Pastorino (May 24 2018 at 19:54, on Zulip):

yes

Santiago Pastorino (May 24 2018 at 19:54, on Zulip):

newindex_something

Santiago Pastorino (May 24 2018 at 19:54, on Zulip):

newindex_idx

nikomatsakis (May 24 2018 at 19:55, on Zulip):

I think maybe @Santiago Pastorino you have to import the Idx trait

Santiago Pastorino (May 24 2018 at 19:56, on Zulip):

to do what?

Santiago Pastorino (May 24 2018 at 19:56, on Zulip):

to have index() available?

nikomatsakis (May 24 2018 at 19:56, on Zulip):

gain access to the method

Santiago Pastorino (May 24 2018 at 19:56, on Zulip):

yeap, anyway there are other issues

Santiago Pastorino (May 24 2018 at 19:57, on Zulip):
69 | impl Atom for RegionVid {
   | ^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate
nikomatsakis (May 24 2018 at 19:58, on Zulip):

ah yeah that's annoying, you have to move the impl into librustc

nikomatsakis (May 24 2018 at 19:58, on Zulip):

but we can do that

Santiago Pastorino (May 24 2018 at 19:58, on Zulip):

but also if I bring index in scope to have it available on RegionVid then I need to implement index :P

nikomatsakis (May 24 2018 at 19:58, on Zulip):

RegionVid does implement Idx

Santiago Pastorino (May 24 2018 at 19:58, on Zulip):

ah yeah that's annoying, you have to move the impl into librustc

yes, I was doing that

Santiago Pastorino (May 24 2018 at 19:59, on Zulip):

RegionVid does implement Idx

and what do you mean by that?

nikomatsakis (May 24 2018 at 20:00, on Zulip):

@Santiago Pastorino I mean this

Santiago Pastorino (May 24 2018 at 20:01, on Zulip):

yes, I know that, unsure what are you trying to say :P

Santiago Pastorino (May 24 2018 at 20:01, on Zulip):

doing use Idx make index() available

nikomatsakis (May 24 2018 at 20:01, on Zulip):

but also if I bring index in scope to have it available on RegionVid then I need to implement index :P

you wrote this

nikomatsakis (May 24 2018 at 20:02, on Zulip):

I'm confused what you meant by "implement index"

Santiago Pastorino (May 24 2018 at 20:02, on Zulip):

well better said ... I need to implement Atom trait which requires an index method

Santiago Pastorino (May 24 2018 at 20:02, on Zulip):

and I already have one but I'm not implementing the trait

Santiago Pastorino (May 24 2018 at 20:03, on Zulip):

there's where I'm lacking of basic Rust knowledge

Santiago Pastorino (May 24 2018 at 20:03, on Zulip):

hehe

Santiago Pastorino (May 24 2018 at 20:03, on Zulip):

in this kind of cases there are a lot of solutions unsure what's possible in Rust

nikomatsakis (May 24 2018 at 20:04, on Zulip):

I think maybe the confusion is because two traits have methods with the same name?

Santiago Pastorino (May 24 2018 at 20:04, on Zulip):

yes

nikomatsakis (May 24 2018 at 20:04, on Zulip):

I see. That's not really important.

nikomatsakis (May 24 2018 at 20:04, on Zulip):

though may impact your ability to write foo.index()

nikomatsakis (May 24 2018 at 20:04, on Zulip):

you can write Idx::index(foo)

nikomatsakis (May 24 2018 at 20:04, on Zulip):

if you want to specify which trait method is called

Santiago Pastorino (May 24 2018 at 20:05, on Zulip):

I see that's fine

Santiago Pastorino (May 24 2018 at 20:05, on Zulip):

thanks

nikomatsakis (May 24 2018 at 20:05, on Zulip):

though may impact your ability to write foo.index()

because of ambiguity

Santiago Pastorino (May 24 2018 at 20:05, on Zulip):

yep

Santiago Pastorino (May 24 2018 at 20:05, on Zulip):

make complete sense

Santiago Pastorino (May 24 2018 at 20:41, on Zulip):
error[E0599]: no function or associated item named `default` found for type `polonius_engine::AllFacts<rustc::ty::RegionVid, dataflow::move_paths::indexes::BorrowIndex, borrow_check::location::LocationIndex>` in the current scope
   --> librustc_mir/borrow_check/nll/mod.rs:101:14
    |
101 |         Some(AllFacts::default())
    |              ^^^^^^^^^^^^^^^^^ function or associated item not found in `polonius_engine::AllFacts<rustc::ty::RegionVid, dataflow::move_paths::indexes::BorrowIndex, borrow_check::location::LocationIndex>`
    |
    = note: the method `default` exists but the following trait bounds were not satisfied:
            `polonius_engine::AllFacts<rustc::ty::RegionVid, dataflow::move_paths::indexes::BorrowIndex, borrow_check::location::LocationIndex> : std::default::Default`

error[E0609]: no field `universal_region` on type `&mut _`
   --> librustc_mir/borrow_check/nll/mod.rs:108:14
    |
108 |             .universal_region
    |              ^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors
Santiago Pastorino (May 24 2018 at 20:41, on Zulip):

two errors to go ... I need to leave for a while will continue later

nikomatsakis (May 24 2018 at 20:54, on Zulip):

@Santiago Pastorino I guess we didn't #[derive(Default)] — we should do that :)

nikomatsakis (May 24 2018 at 20:55, on Zulip):

if you like, you can make these edits locally and temporary redirect rustc

nikomatsakis (May 24 2018 at 20:55, on Zulip):

this is what I usually do

nikomatsakis (May 24 2018 at 20:55, on Zulip):

basically clone polonius somewhere and edit the Cargo.toml to just be a path dependency

Santiago Pastorino (May 24 2018 at 20:55, on Zulip):

hey

Santiago Pastorino (May 24 2018 at 20:55, on Zulip):

back

nikomatsakis (May 24 2018 at 20:55, on Zulip):

that's the easiest thing to do

Santiago Pastorino (May 24 2018 at 20:55, on Zulip):

AllFacts has Default derived

Santiago Pastorino (May 24 2018 at 20:56, on Zulip):

the thing is ...

Santiago Pastorino (May 24 2018 at 20:56, on Zulip):
  = note: the method `default` exists but the following trait bounds were not satisfied:
            `polonius_engine::AllFacts<rustc::ty::RegionVid, dataflow::move_paths::indexes::BorrowIndex, borrow_check::location::LocationIndex> : std::default::Default`
Santiago Pastorino (May 24 2018 at 20:56, on Zulip):

I guess all the indexes need to derive default and it's ok to do it

nikomatsakis (May 24 2018 at 20:56, on Zulip):

@Santiago Pastorino oh, that's a bug

nikomatsakis (May 24 2018 at 20:56, on Zulip):

we should not derive default

nikomatsakis (May 24 2018 at 20:56, on Zulip):

but rather write our own impl

nikomatsakis (May 24 2018 at 20:57, on Zulip):

those types should not require Default and I would rather they didn't

Santiago Pastorino (May 24 2018 at 20:57, on Zulip):

ok

nikomatsakis (May 24 2018 at 20:57, on Zulip):

because it's not sensible to have a "default" region vid

nikomatsakis (May 24 2018 at 20:57, on Zulip):

(what region do you mean?)

Santiago Pastorino (May 24 2018 at 20:57, on Zulip):

right

Santiago Pastorino (May 24 2018 at 20:57, on Zulip):

makes sense

nikomatsakis (May 24 2018 at 20:57, on Zulip):

if we modify polonius-engine and write the default-impl ourselves, we'll find we don't need those types to have Default

nikomatsakis (May 24 2018 at 20:57, on Zulip):

because e.g. Vec<R> implements Default even if R does not

Santiago Pastorino (May 24 2018 at 20:57, on Zulip):

I see

nikomatsakis (May 24 2018 at 20:58, on Zulip):

it's annoying that there is no way to make #[derive] understand that

Santiago Pastorino (May 24 2018 at 20:58, on Zulip):

makes sense

nikomatsakis (May 24 2018 at 20:58, on Zulip):

there was an RFC for it

nikomatsakis (May 24 2018 at 20:58, on Zulip):

not sure what has happened with it

nikomatsakis (May 24 2018 at 20:58, on Zulip):

might be good to push on it :)

Santiago Pastorino (May 24 2018 at 20:59, on Zulip):

:)

Santiago Pastorino (May 24 2018 at 21:43, on Zulip):

I have the thing working

nikomatsakis (May 24 2018 at 21:43, on Zulip):

:tada:

Santiago Pastorino (May 24 2018 at 21:44, on Zulip):

@nikomatsakis first we need this https://github.com/rust-lang-nursery/polonius/pull/49

Santiago Pastorino (May 24 2018 at 21:44, on Zulip):

and then ... are we going to depend on polonius from github? or are you releasing a new version?

qmx (May 24 2018 at 21:45, on Zulip):

nice!

nikomatsakis (May 24 2018 at 21:45, on Zulip):

and then ... are we going to depend on polonius from github? or are you releasing a new version?

I'll release a new version

Santiago Pastorino (May 24 2018 at 21:45, on Zulip):

ok

Santiago Pastorino (May 24 2018 at 21:46, on Zulip):

and let me know so I can change the code and open a PR on rustc

Santiago Pastorino (May 24 2018 at 21:46, on Zulip):

@qmx we can continue tomorrow pairing if @nikomatsakis has something else we can do

nikomatsakis (May 24 2018 at 21:46, on Zulip):

and let me know so I can change the code and open a PR on rustc

I'll do it right now

Santiago Pastorino (May 24 2018 at 21:47, on Zulip):

I know there are tons of things to do, but I prefer to ask so we tackle what you consider more urgent

qmx (May 24 2018 at 21:48, on Zulip):

I'm wrapping up dayjob stuff now, do we have a gameplan for tomorrow?

Santiago Pastorino (May 24 2018 at 21:48, on Zulip):

that's what I'm asking Niko :)

Santiago Pastorino (May 24 2018 at 21:48, on Zulip):

@qmx what's your time schedule for tomorrow?

Santiago Pastorino (May 24 2018 at 21:48, on Zulip):

well ... let's talk about this details in private so we don't bother the rest

nikomatsakis (May 24 2018 at 21:51, on Zulip):

@Santiago Pastorino published v0.1.1

nikomatsakis (May 24 2018 at 21:51, on Zulip):

I think that should be semver compatible :)

Santiago Pastorino (May 24 2018 at 21:51, on Zulip):

already assumed 0.1.1 :P

Santiago Pastorino (May 24 2018 at 21:51, on Zulip):

was winning time meanwhile you were pushing

nikomatsakis (May 24 2018 at 21:52, on Zulip):

this is so exciting :double_exclamation_mark:

Santiago Pastorino (May 24 2018 at 21:55, on Zulip):

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

Santiago Pastorino (May 24 2018 at 21:55, on Zulip):

pushed so you can start seeing it but I've rebased and I'm compiling again

Santiago Pastorino (May 24 2018 at 21:55, on Zulip):

should be good but ...

Santiago Pastorino (May 24 2018 at 22:13, on Zulip):

thing is ok

nikomatsakis (May 24 2018 at 22:47, on Zulip):

@Santiago Pastorino you have to add the crate to the whitelist

Santiago Pastorino (May 24 2018 at 22:57, on Zulip):

@nikomatsakis done!

nikomatsakis (May 24 2018 at 23:01, on Zulip):

@Santiago Pastorino gotta run but I delegated to you so — once travis is green — you can do @bors r=nikomatsakis

Santiago Pastorino (May 24 2018 at 23:03, on Zulip):

ok

Santiago Pastorino (May 24 2018 at 23:03, on Zulip):

@nikomatsakis what can we do with @qmx? how can we continue?

Santiago Pastorino (May 24 2018 at 23:03, on Zulip):

if you're leaving don't worry but let us know when you're back later/tomorrow

nikomatsakis (May 24 2018 at 23:05, on Zulip):

@Santiago Pastorino see the steps laid out in #44

Santiago Pastorino (May 24 2018 at 23:05, on Zulip):

:+1:

Santiago Pastorino (May 25 2018 at 00:32, on Zulip):

@nikomatsakis saw you r+, :+1:

nikomatsakis (May 25 2018 at 12:28, on Zulip):

hey @Santiago Pastorino and @qmx, morning =)

Santiago Pastorino (May 25 2018 at 12:30, on Zulip):

morning

Santiago Pastorino (May 25 2018 at 12:31, on Zulip):

will wait a bit for Douglas so we can start

Santiago Pastorino (May 25 2018 at 12:31, on Zulip):

there's no rush, I can have breakfast before :P

Santiago Pastorino (May 25 2018 at 12:31, on Zulip):
move the Output and datafrog code into polonius-engine
    hopefully the datafrog version we need is in crates.io by then
Santiago Pastorino (May 25 2018 at 12:32, on Zulip):

first task would be to move Output with Algorithm from cli module and datafrog to polonius-engine

Santiago Pastorino (May 25 2018 at 12:32, on Zulip):

datafrog is already published if I'm not wrong

Santiago Pastorino (May 25 2018 at 12:32, on Zulip):

so that should be easy

nikomatsakis (May 25 2018 at 12:32, on Zulip):

yes, that's right

nikomatsakis (May 25 2018 at 12:32, on Zulip):

and I already made the Output code generic

nikomatsakis (May 25 2018 at 12:32, on Zulip):

so in principle should be straightforward

Santiago Pastorino (May 25 2018 at 12:33, on Zulip):

and I already made the Output code generic

so even easier :)

qmx (May 25 2018 at 13:08, on Zulip):

I'm here

nikomatsakis (May 25 2018 at 13:14, on Zulip):

o/

nikomatsakis (May 25 2018 at 13:14, on Zulip):

not sure where @Santiago Pastorino is

nikomatsakis (May 25 2018 at 13:14, on Zulip):

but did the first step of moving Output into polonius-engine make sense to you?

qmx (May 25 2018 at 13:15, on Zulip):

reading, will tell in a few minutes :P

qmx (May 25 2018 at 13:19, on Zulip):

yea, makes sense

Santiago Pastorino (May 25 2018 at 13:28, on Zulip):

here

qmx (May 25 2018 at 13:34, on Zulip):

so what have you done so far?

Santiago Pastorino (May 25 2018 at 13:38, on Zulip):

so ... there's the PR from yesterday

Santiago Pastorino (May 25 2018 at 13:38, on Zulip):

and we need to start moving things

Santiago Pastorino (May 25 2018 at 13:38, on Zulip):

hold on ...

Santiago Pastorino (May 25 2018 at 13:38, on Zulip):

we can start, let me ping you in private

Santiago Pastorino (May 25 2018 at 14:09, on Zulip):
error[E0658]: `crate` visibility modifier is experimental (see issue #45388)                                                                                                                                │······
  --> polonius-engine/src/output/mod.rs:93:5                                                                                                                                                                │······
   |                                                                                                                                                                                                        │······
93 |     crate fn regions_live_at(&self, location: Point) -> &[Region] {                                                                                                                                    │······
   |     ^^^^^                                                                                                                                                                                              │······
   |                                                                                                                                                                                                        │······
   = help: add #![feature(crate_visibility_modifier)] to the crate attributes to enable
Santiago Pastorino (May 25 2018 at 14:09, on Zulip):

@nikomatsakis ^^^

Santiago Pastorino (May 25 2018 at 14:09, on Zulip):

this is happening besides we already added the feature

Santiago Pastorino (May 25 2018 at 14:09, on Zulip):

is there something else to do?

nikomatsakis (May 25 2018 at 14:09, on Zulip):

you probably added it wrong

Santiago Pastorino (May 25 2018 at 14:09, on Zulip):

:P

nikomatsakis (May 25 2018 at 14:10, on Zulip):

though not obviously so :)

Santiago Pastorino (May 25 2018 at 14:10, on Zulip):

we added the thing on polonius-engine/src/lib.rs

nikomatsakis (May 25 2018 at 14:10, on Zulip):

I don't know what else would be needed... that should be enough :)

Santiago Pastorino (May 25 2018 at 14:10, on Zulip):

#![feature(crate_visibility_modifier)]

Santiago Pastorino (May 25 2018 at 14:10, on Zulip):

ok

Santiago Pastorino (May 25 2018 at 14:10, on Zulip):

let's see

nikomatsakis (May 25 2018 at 14:12, on Zulip):

spying on your ssh session, it looks like you are getting other errors too?

nikomatsakis (May 25 2018 at 14:12, on Zulip):

maybe you just need to add more feature gates?

nikomatsakis (May 25 2018 at 14:12, on Zulip):

you could probably just lift the ones from polonius/src/main.rs

qmx (May 25 2018 at 14:12, on Zulip):

just copy them all

Santiago Pastorino (May 25 2018 at 14:12, on Zulip):

figured

Santiago Pastorino (May 25 2018 at 14:12, on Zulip):

the issue was that the thing was below doc comments

Santiago Pastorino (May 25 2018 at 14:12, on Zulip):

we were getting a warning

nikomatsakis (May 25 2018 at 14:16, on Zulip):

oh, I thought it might be something weird like that

Santiago Pastorino (May 25 2018 at 14:30, on Zulip):

@nikomatsakis output/tracking.rs is not implemented in a generic way

nikomatsakis (May 25 2018 at 14:30, on Zulip):

what is that?

Santiago Pastorino (May 25 2018 at 14:31, on Zulip):

I don't even remember what tracking is about

nikomatsakis (May 25 2018 at 14:31, on Zulip):

maybe just delete it:)

Santiago Pastorino (May 25 2018 at 14:31, on Zulip):

:D

nikomatsakis (May 25 2018 at 14:31, on Zulip):

I think that may be related to the tracking on in- and out-degrees?

qmx (May 25 2018 at 14:31, on Zulip):

yea, that was for finding the degree counts

nikomatsakis (May 25 2018 at 14:31, on Zulip):

let me check

nikomatsakis (May 25 2018 at 14:31, on Zulip):

if so, I killed that code

nikomatsakis (May 25 2018 at 14:31, on Zulip):

because it no longer seemed esp. relevant

qmx (May 25 2018 at 14:31, on Zulip):

:kitchen_knife:

Santiago Pastorino (May 25 2018 at 14:31, on Zulip):

ok

lqd (May 25 2018 at 14:31, on Zulip):

I was about to say that :) I had missed we could remove it in the review; this and the histo crate dependency etc

Santiago Pastorino (May 25 2018 at 14:32, on Zulip):

:+1:

Santiago Pastorino (May 25 2018 at 14:56, on Zulip):

so, we are confused about something right now

Santiago Pastorino (May 25 2018 at 14:56, on Zulip):

https://github.com/rust-lang-nursery/polonius/blob/master/src/dump.rs#L227

nikomatsakis (May 25 2018 at 14:56, on Zulip):

ok

Santiago Pastorino (May 25 2018 at 14:56, on Zulip):

and

Santiago Pastorino (May 25 2018 at 14:56, on Zulip):

https://github.com/rust-lang-nursery/polonius/blob/master/polonius-engine/src/facts.rs#L44

Santiago Pastorino (May 25 2018 at 14:57, on Zulip):

I understand from rustc point of view

Santiago Pastorino (May 25 2018 at 14:57, on Zulip):

but why do polonius redefine Atom?

Santiago Pastorino (May 25 2018 at 14:57, on Zulip):

I mean, it's clear how to do in rustc + polonius-engine

Santiago Pastorino (May 25 2018 at 14:57, on Zulip):

but unsure what are we trying to do in polonius

Santiago Pastorino (May 25 2018 at 14:58, on Zulip):

it's just that in polonius we should don't care about polonius-engine's Atom and roll our own?

nikomatsakis (May 25 2018 at 14:58, on Zulip):

oh, they are just independent traits

nikomatsakis (May 25 2018 at 14:58, on Zulip):

I should have renamed the Atom in polonius

nikomatsakis (May 25 2018 at 14:58, on Zulip):

but I don't know why this impacts you

nikomatsakis (May 25 2018 at 14:58, on Zulip):

that dump code should not be in the engine

nikomatsakis (May 25 2018 at 14:58, on Zulip):

in fact, I specifically moved it out from the output module for that reason

Santiago Pastorino (May 25 2018 at 14:58, on Zulip):

yeah, it's not

Santiago Pastorino (May 25 2018 at 14:59, on Zulip):

maybe we did some use wrong or something

nikomatsakis (May 25 2018 at 14:59, on Zulip):

(well, I should have renamed it or otherwise refactored)

Santiago Pastorino (May 25 2018 at 14:59, on Zulip):

let me see all again

nikomatsakis (May 25 2018 at 14:59, on Zulip):

/me peeks

Santiago Pastorino (May 25 2018 at 15:02, on Zulip):

fixed

Santiago Pastorino (May 25 2018 at 15:02, on Zulip):

Region, Loan, Point in dump_output in src/dump.rs need to be bound to polonius_engine:Atom

Santiago Pastorino (May 25 2018 at 15:03, on Zulip):

but we have to Atom

Santiago Pastorino (May 25 2018 at 15:03, on Zulip):

that's why the confusion

nikomatsakis (May 25 2018 at 15:10, on Zulip):

ok I don't understand but seems fine =)

Santiago Pastorino (May 25 2018 at 15:39, on Zulip):

@nikomatsakis https://github.com/rust-lang-nursery/polonius/pull/52

Santiago Pastorino (May 25 2018 at 15:40, on Zulip):

we need to fix this https://github.com/rust-lang-nursery/polonius/pull/52/files#diff-dba02791b4baf8a328939029ac920005R51

Santiago Pastorino (May 25 2018 at 15:40, on Zulip):

we can continue a bit more but opening the PR so you can check how goes :)

qmx (May 25 2018 at 15:40, on Zulip):

I'll do that on a follow-up PR

nikomatsakis (May 25 2018 at 15:41, on Zulip):

looks pretty good, left one nit

qmx (May 25 2018 at 15:54, on Zulip):

I've dropped the changes on a follow-up PR since @Santiago Pastorino stepped out for a bit

qmx (May 25 2018 at 15:55, on Zulip):

https://github.com/rust-lang-nursery/polonius/pull/53

nikomatsakis (May 25 2018 at 15:58, on Zulip):

@qmx merged!

qmx (May 25 2018 at 15:59, on Zulip):

:tada:

Santiago Pastorino (May 25 2018 at 16:17, on Zulip):

@nikomatsakis added a tick to the task here https://github.com/rust-lang-nursery/polonius/issues/44

Santiago Pastorino (May 25 2018 at 16:17, on Zulip):

@nikomatsakis should we continue porting the stuff now to rustc?

Santiago Pastorino (May 25 2018 at 16:18, on Zulip):

we need to fix the enums hack from previous PR and then we can continue

qmx (May 25 2018 at 16:18, on Zulip):

I'm working on fixing the enum thing now

Santiago Pastorino (May 25 2018 at 16:19, on Zulip):

share the thing so I can join you :)

nikomatsakis (May 25 2018 at 16:52, on Zulip):

@nikomatsakis should we continue porting the stuff now to rustc?

go for it!

Santiago Pastorino (May 25 2018 at 16:57, on Zulip):

:+1:

Santiago Pastorino (May 25 2018 at 16:57, on Zulip):

we are fixing the hack

Santiago Pastorino (May 25 2018 at 16:57, on Zulip):

and will continue with that in a little while

qmx (May 25 2018 at 17:00, on Zulip):

@nikomatsakis another one https://github.com/rust-lang-nursery/polonius/pull/54

nikomatsakis (May 25 2018 at 17:01, on Zulip):

@qmx merged

qmx (May 25 2018 at 17:02, on Zulip):

I'm moving to rustc integration now

qmx (May 25 2018 at 17:02, on Zulip):

are you going to do a polonius release?

nikomatsakis (May 25 2018 at 17:10, on Zulip):

I can yeah

nikomatsakis (May 25 2018 at 17:10, on Zulip):

I'll go ahead and call this 0.2.0

nikomatsakis (May 25 2018 at 17:10, on Zulip):

even though it's probably semver-compatible

qmx (May 25 2018 at 17:11, on Zulip):

ok

nikomatsakis (May 25 2018 at 17:14, on Zulip):

@qmx done https://crates.io/crates/polonius-engine

qmx (May 25 2018 at 17:15, on Zulip):

:tada:

qmx (May 25 2018 at 17:15, on Zulip):

building a fresh rust

nikomatsakis (May 25 2018 at 17:17, on Zulip):

will be afk for a bit (fyi)

qmx (May 25 2018 at 17:17, on Zulip):

ok

qmx (May 25 2018 at 17:49, on Zulip):

I'm reading your integration commit here, but will probably need some walkthrough when you're back

Santiago Pastorino (May 25 2018 at 18:07, on Zulip):

@nikomatsakis we are starting with @qmx

Santiago Pastorino (May 25 2018 at 18:07, on Zulip):

we are basing the work on my current unmerged PR about AllFacts

Santiago Pastorino (May 25 2018 at 18:07, on Zulip):

I guess this is the way to go

nikomatsakis (May 25 2018 at 19:04, on Zulip):

@qmx @Santiago Pastorino I'm back now :)

nikomatsakis (May 25 2018 at 19:04, on Zulip):

¿qué pása?

nikomatsakis (May 25 2018 at 19:05, on Zulip):

I don't know how to say it in Portuguese but I hope Spanish's close enough ;)

nikomatsakis (May 25 2018 at 19:05, on Zulip):

also I think I got the accents wrong

nikomatsakis (May 25 2018 at 19:05, on Zulip):

oh well :)

nikomatsakis (May 25 2018 at 19:05, on Zulip):

so what's up?

qmx (May 25 2018 at 19:05, on Zulip):

that was spanish

qmx (May 25 2018 at 19:05, on Zulip):

:)

nikomatsakis (May 25 2018 at 19:05, on Zulip):

I'm aware :)

nikomatsakis (May 25 2018 at 19:05, on Zulip):

hey @Santiago Pastorino speaks Spanish

qmx (May 25 2018 at 19:05, on Zulip):

yep, he's really good at it! I'm definitely not :P

qmx (May 25 2018 at 19:05, on Zulip):

so I'm reading your branch diff

qmx (May 25 2018 at 19:06, on Zulip):

and having a hard time following all the changes there

nikomatsakis (May 25 2018 at 19:06, on Zulip):

yep, he's really good at it! I'm definitely not :P

heh. One of the things that amuses me about learning a foreign language is that basically the best complement you can have is that you are indistinguishable from the average person

qmx (May 25 2018 at 19:06, on Zulip):

yep, he's really good at it! I'm definitely not :P

heh. One of the things that amuses me about learning a foreign language is that basically the best complement you can have is that you are indistinguishable from the average person

so true :joy:

nikomatsakis (May 25 2018 at 19:06, on Zulip):

ok, what have you done so far?

nikomatsakis (May 25 2018 at 19:06, on Zulip):

want to maybe try to pair briefly?

nikomatsakis (May 25 2018 at 19:07, on Zulip):

(or just chat over Zulip, either wfm)

qmx (May 25 2018 at 19:07, on Zulip):

pairing would be nice

nikomatsakis (May 25 2018 at 19:07, on Zulip):

ok gimme 2 min

qmx (May 25 2018 at 19:08, on Zulip):

k

Santiago Pastorino (May 25 2018 at 20:00, on Zulip):

@nikomatsakis @qmx you around?

Santiago Pastorino (May 25 2018 at 20:03, on Zulip):

@qmx needed to leave for a while, I'm back

Santiago Pastorino (May 25 2018 at 20:04, on Zulip):

did you already paired?

nikomatsakis (May 25 2018 at 20:11, on Zulip):

yeah we made a video I plan to upload

Santiago Pastorino (May 25 2018 at 20:12, on Zulip):

cool

Santiago Pastorino (May 25 2018 at 20:12, on Zulip):

did you finish the task?

Santiago Pastorino (May 25 2018 at 20:12, on Zulip):

I was kind of tackling it also

nikomatsakis (May 25 2018 at 20:13, on Zulip):

heh no but we sketched out the steps :)

Santiago Pastorino (May 25 2018 at 20:13, on Zulip):

cool

Santiago Pastorino (May 25 2018 at 20:13, on Zulip):

@qmx wanna continue with me?

qmx (May 25 2018 at 20:13, on Zulip):

can't wait for your PR to land @Santiago Pastorino

Santiago Pastorino (May 25 2018 at 20:13, on Zulip):

hehe I can give you some time to rest :P

nikomatsakis (May 25 2018 at 20:51, on Zulip):

@Santiago Pastorino what I recommended to @qmx was that — roughly at the spot where we presently dump out the NLL facts to disk — we would do the polonius computation (this is in nll::compute_regions()). The resulting Output would be returned (not stored in the RegionInferenceContext)

nikomatsakis (May 25 2018 at 20:51, on Zulip):

we would then put that Output into the Flows data structure that currently stores the borrows set

Santiago Pastorino (May 25 2018 at 20:51, on Zulip):

yes I was looking exactly at compute_regions

nikomatsakis (May 25 2018 at 20:51, on Zulip):

and (if the switch is enabled) use it instead to answer the functions borrows_live_at and with_outgoing_borrows

nikomatsakis (May 25 2018 at 20:51, on Zulip):

this is somewhat different from what I did in my commit, but similar

Santiago Pastorino (May 25 2018 at 20:52, on Zulip):

right now compute_regions returns ...

Santiago Pastorino (May 25 2018 at 20:52, on Zulip):
) -> (
    RegionInferenceContext<'tcx>,
    Option<ClosureRegionRequirements<'gcx>>,
) {
nikomatsakis (May 25 2018 at 20:53, on Zulip):

yeah you would return a 3rd thing

Santiago Pastorino (May 25 2018 at 20:53, on Zulip):

you mean to add another thing to that tuple?

Santiago Pastorino (May 25 2018 at 20:53, on Zulip):

:D

Santiago Pastorino (May 25 2018 at 20:53, on Zulip):

beautiful :P

qmx (May 25 2018 at 20:53, on Zulip):

@nikomatsakis is this somewhat the same we discussed on the call?

qmx (May 25 2018 at 20:53, on Zulip):

or something different?

Santiago Pastorino (May 25 2018 at 20:54, on Zulip):

@qmx I think it's the same, I'm trying to catch up :)

nikomatsakis (May 25 2018 at 20:54, on Zulip):

@qmx same

Santiago Pastorino (May 25 2018 at 20:54, on Zulip):

still need to see the video when is up

nikomatsakis (May 25 2018 at 20:54, on Zulip):

encoding just finished

Santiago Pastorino (May 25 2018 at 20:54, on Zulip):

take your time to have lunch

qmx (May 25 2018 at 20:54, on Zulip):

so now I'm worried about overlap

Santiago Pastorino (May 25 2018 at 20:54, on Zulip):

are you working on this thing now?

qmx (May 25 2018 at 20:55, on Zulip):

yup, adding the flag atm, but I'll have to stop for getting lunch :P

qmx (May 25 2018 at 20:55, on Zulip):

(waaaay past overdue :P)

Santiago Pastorino (May 25 2018 at 20:56, on Zulip):

ahh thought we were going to pair

Santiago Pastorino (May 25 2018 at 20:56, on Zulip):

I wasn't coding was waiting for you

Santiago Pastorino (May 25 2018 at 20:56, on Zulip):

no worries, you can continue with the task

Santiago Pastorino (May 25 2018 at 20:56, on Zulip):

I will try to do something else

qmx (May 25 2018 at 20:57, on Zulip):

the pairing with niko was a walkthrough of all those codepaths

Santiago Pastorino (May 25 2018 at 20:58, on Zulip):

:+1:

nikomatsakis (May 25 2018 at 20:59, on Zulip):

jfyi video with @qmx is going to be at youtube at https://youtu.be/EM_j4suuA1Y

nikomatsakis (May 25 2018 at 21:00, on Zulip):

in case you want to review something :)

nikomatsakis (May 25 2018 at 21:00, on Zulip):

I'm not sure where to catalog and list these videos :)

Santiago Pastorino (May 25 2018 at 21:06, on Zulip):

@nikomatsakis creating a channel and or a playlist?

nikomatsakis (May 25 2018 at 21:06, on Zulip):

I guess, I don't know what those things are :) though I can guess ;)

nikomatsakis (May 25 2018 at 21:06, on Zulip):

/me not very YouTube savvy apparently

Santiago Pastorino (May 25 2018 at 21:07, on Zulip):

hehehe

Santiago Pastorino (May 25 2018 at 21:07, on Zulip):

you know what is a borrow checker but you don't what's a playlist? :P

nikomatsakis (May 25 2018 at 21:09, on Zulip):

confirm:)

nikomatsakis (May 25 2018 at 21:09, on Zulip):

I only ever used youtube by getting a link :)

nikomatsakis (May 25 2018 at 21:10, on Zulip):

I understand there are other .. things .. on there ..

Santiago Pastorino (May 25 2018 at 21:10, on Zulip):

:)

nikomatsakis (May 25 2018 at 21:25, on Zulip):

ok I made a rustc pair programming playlist :)

qmx (May 26 2018 at 00:49, on Zulip):

@nikomatsakis so we've managed to do all the steps but pass the polonius output into FlowState

nikomatsakis (May 26 2018 at 00:49, on Zulip):

ok

qmx (May 26 2018 at 00:49, on Zulip):

http://github.com/qmx/rust/tree/use_polonius_Output

Santiago Pastorino (May 26 2018 at 00:50, on Zulip):

is it really important for now to pass through FlowState?

Santiago Pastorino (May 26 2018 at 00:50, on Zulip):

I guess we could land something and then try to refactor

nikomatsakis (May 26 2018 at 00:51, on Zulip):

it doesn't have to go into flow state

qmx (May 26 2018 at 00:51, on Zulip):

maybe that's why you stored it initially inside the MirBorrowContext

nikomatsakis (May 26 2018 at 00:51, on Zulip):

what problem are you hitting exactly?

Santiago Pastorino (May 26 2018 at 00:51, on Zulip):

@nikomatsakis one thing ... in your changes you changed how each_borrow_involving_path works

qmx (May 26 2018 at 00:51, on Zulip):

not knowing how/where to store the polonius output

Santiago Pastorino (May 26 2018 at 00:51, on Zulip):

you were suggesting to do the same and use polonius output there

Santiago Pastorino (May 26 2018 at 00:52, on Zulip):

this is changing in the way things will work

Santiago Pastorino (May 26 2018 at 00:52, on Zulip):

are you doing that because is exactly equivalent?

Santiago Pastorino (May 26 2018 at 00:52, on Zulip):

I mean, from the top of my head I don't know what is this function actually checking

qmx (May 26 2018 at 00:52, on Zulip):

if we don't store it inside FlowState, how can we reach it later inside borrows_in_scope(Location)?

nikomatsakis (May 26 2018 at 00:53, on Zulip):

right — acutally I gotta run to put my daughter to bed — but you could either put it in the MirBorrowckCtxt, as I did in the commit, or the flow state

nikomatsakis (May 26 2018 at 00:53, on Zulip):

@qmx if you don't put it in there, you have to not call that routine, basically

nikomatsakis (May 26 2018 at 00:53, on Zulip):

I think it shouldn't be hard to put it in the flow-state though...

qmx (May 26 2018 at 00:53, on Zulip):

I'll try now on MirBorrowCtx

nikomatsakis (May 26 2018 at 00:53, on Zulip):

but maybe try on the mir borrowck ctxt first

nikomatsakis (May 26 2018 at 00:53, on Zulip):

(since there is the model from that commit)

nikomatsakis (May 26 2018 at 00:53, on Zulip):

anyway bbl

qmx (May 26 2018 at 00:53, on Zulip):

k, thanks

nikomatsakis (May 26 2018 at 01:02, on Zulip):

ok back

Santiago Pastorino (May 26 2018 at 01:02, on Zulip):

I figured that my question was silly :)

Santiago Pastorino (May 26 2018 at 01:03, on Zulip):

the answer is things are equivalent

Santiago Pastorino (May 26 2018 at 01:03, on Zulip):

just in case asking

nikomatsakis (May 26 2018 at 01:03, on Zulip):

(do you have a textmate session or something?)

Santiago Pastorino (May 26 2018 at 01:03, on Zulip):

because I'm not 100% what polonius is not covering

Santiago Pastorino (May 26 2018 at 01:04, on Zulip):

actually we were pairing with qmx but then we splitted work

Santiago Pastorino (May 26 2018 at 01:04, on Zulip):

I'm trying to investigate and help him

Santiago Pastorino (May 26 2018 at 01:04, on Zulip):

maybe tomorrow we can pair again

Santiago Pastorino (May 26 2018 at 01:04, on Zulip):

unsure

nikomatsakis (May 26 2018 at 01:05, on Zulip):

I was mentioned to him — not sure if this is how you split it — that if you have the Output

nikomatsakis (May 26 2018 at 01:05, on Zulip):

it makes sense to augment the output from -Zdump-mir

nikomatsakis (May 26 2018 at 01:05, on Zulip):

with the derived tuples at each point

nikomatsakis (May 26 2018 at 01:05, on Zulip):

(which that PR also does...)

nikomatsakis (May 26 2018 at 01:05, on Zulip):

er, that commit

Santiago Pastorino (May 26 2018 at 01:05, on Zulip):

ok

nikomatsakis (May 26 2018 at 01:05, on Zulip):

would be a sort of independent thing

Santiago Pastorino (May 26 2018 at 01:06, on Zulip):

I think Douglas figured this thing out

Santiago Pastorino (May 26 2018 at 01:06, on Zulip):

I'm also have something on my machine

nikomatsakis (May 26 2018 at 01:06, on Zulip):

@qmx so this is the flow_state type, Flows (here)

Santiago Pastorino (May 26 2018 at 01:06, on Zulip):

when we can, we can sync again

nikomatsakis (May 26 2018 at 01:06, on Zulip):

one problem might be that borrows_in_scope presently returns an impl Iterator

nikomatsakis (May 26 2018 at 01:07, on Zulip):

which is sometimes a pain when you want to return two different kinds of iterators

nikomatsakis (May 26 2018 at 01:07, on Zulip):

we could change it to a "callback" form

Santiago Pastorino (May 26 2018 at 01:07, on Zulip):

@qmx so this is the flow_state type, Flows (here)

why did you mention this?

nikomatsakis (May 26 2018 at 01:07, on Zulip):

where it invokes a closure with each BorrowIndex`

qmx (May 26 2018 at 01:07, on Zulip):

and take a closure as callback?

Santiago Pastorino (May 26 2018 at 01:07, on Zulip):

I may be missing something :)

qmx (May 26 2018 at 01:07, on Zulip):

boom

nikomatsakis (May 26 2018 at 01:07, on Zulip):

@Santiago Pastorino I was just jumping back to what @qmx asked earlier

qmx (May 26 2018 at 01:09, on Zulip):

I've managed to get the output inside MirBorrowCtx

nikomatsakis (May 26 2018 at 01:09, on Zulip):

@qmx something like

fn borrows_in_scope(&self, location: PointIndex, each_borrow: impl FnMut(BorrowIndex)) {
    if polonius {
        for borrow in polonius.errors_at(location) { each_borrow(borrow) }
    } else {
        self.borrows.iter_incoming().for_each(each_borrow)
    }
}
nikomatsakis (May 26 2018 at 01:09, on Zulip):

oh, ok

qmx (May 26 2018 at 01:09, on Zulip):

at least the build didn't break so far :P

qmx (May 26 2018 at 01:09, on Zulip):

(still running)

nikomatsakis (May 26 2018 at 01:09, on Zulip):

er, I guess the main method is still borrow_live_at?

qmx (May 26 2018 at 01:10, on Zulip):

I think the closure version might be easier to deal with

nikomatsakis (May 26 2018 at 01:10, on Zulip):

I thought we landed a PR that changed that

nikomatsakis (May 26 2018 at 01:10, on Zulip):

but I see this

qmx (May 26 2018 at 01:10, on Zulip):

hmmm

nikomatsakis (May 26 2018 at 01:11, on Zulip):

oh, no we did change it

nikomatsakis (May 26 2018 at 01:11, on Zulip):

the Output type just wasn't reorganized it seems

nikomatsakis (May 26 2018 at 01:11, on Zulip):

in this PR https://github.com/rust-lang-nursery/polonius/pull/42/

nikomatsakis (May 26 2018 at 01:11, on Zulip):

the "main" type is potential_errors

nikomatsakis (May 26 2018 at 01:11, on Zulip):

which is now misnamed

nikomatsakis (May 26 2018 at 01:11, on Zulip):

the main field that is

nikomatsakis (May 26 2018 at 01:11, on Zulip):

should just be errors, not potential_errors

qmx (May 26 2018 at 01:11, on Zulip):

so now every change in mir is taking ~7 minutes to build

nikomatsakis (May 26 2018 at 01:12, on Zulip):

@qmx if you just want to know if it builds, I suggest ./x.py check

nikomatsakis (May 26 2018 at 01:12, on Zulip):

it's way faster

nikomatsakis (May 26 2018 at 01:12, on Zulip):

I tend to iterate with that

qmx (May 26 2018 at 01:12, on Zulip):

RUST_BACKTRACE=full ./x.py build -i --stage 1 --keep-stage 0 src/libstd

nikomatsakis (May 26 2018 at 01:12, on Zulip):

until I am ready to run tests

Santiago Pastorino (May 26 2018 at 01:12, on Zulip):

./x.py check what does it do?

nikomatsakis (May 26 2018 at 01:13, on Zulip):

it just checks if it compiles or not

qmx (May 26 2018 at 01:13, on Zulip):

I've forgot about it

nikomatsakis (May 26 2018 at 01:13, on Zulip):

but doens't produce teh binary

qmx (May 26 2018 at 01:13, on Zulip):

that's awesome

Santiago Pastorino (May 26 2018 at 01:13, on Zulip):

ok

Santiago Pastorino (May 26 2018 at 01:13, on Zulip):

makes sense

nikomatsakis (May 26 2018 at 01:13, on Zulip):

that said, if all your changes are local to librustc_mir, there's not as much advantage, since you could just kill the build once that succeeds

nikomatsakis (May 26 2018 at 01:13, on Zulip):

but with check you don't have to remember to do so

nikomatsakis (May 26 2018 at 01:13, on Zulip):

but if you e.g. edit librustc

nikomatsakis (May 26 2018 at 01:14, on Zulip):

and want to work through the ramifications through the various crates

nikomatsakis (May 26 2018 at 01:14, on Zulip):

x.py check is a real winner

qmx (May 26 2018 at 01:15, on Zulip):

true

nikomatsakis (May 26 2018 at 01:16, on Zulip):

@qmx well I think either putting it in MirBorrowckCtxt or FlowState is fine

qmx (May 26 2018 at 01:16, on Zulip):

I'd love to have a rustfmt that would only format lines you changed

qmx (May 26 2018 at 01:16, on Zulip):

vs re-doing the whole file

qmx (May 26 2018 at 01:18, on Zulip):

lol I was looking at the wrong struct, that's why it was so hard to put it inside FlowState

qmx (May 26 2018 at 01:18, on Zulip):

will move it there

nikomatsakis (May 26 2018 at 01:19, on Zulip):

d'oh

nikomatsakis (May 26 2018 at 01:19, on Zulip):

I think rustfmt can do that, but editors aren't using it very well

nikomatsakis (May 26 2018 at 01:20, on Zulip):

not sure tho

qmx (May 26 2018 at 01:20, on Zulip):

also RLS worked inside rustc for a while, it was glorious - then it crashed badly :P

nikomatsakis (May 26 2018 at 01:20, on Zulip):

heh

nikomatsakis (May 26 2018 at 01:21, on Zulip):

never dared try

Santiago Pastorino (May 26 2018 at 01:24, on Zulip):

we are you able to get it working @qmx?

Santiago Pastorino (May 26 2018 at 01:24, on Zulip):

have the thing compiling here

nikomatsakis (May 26 2018 at 01:24, on Zulip):

ok I'm off to bed soon — if you all have any last-minute questions let me know soon :)

Santiago Pastorino (May 26 2018 at 01:24, on Zulip):

anyway I need to leave now but if you’re available tomorrow we can pair or something

qmx (May 26 2018 at 01:25, on Zulip):

yea, I'm trying to move it to FlowState

qmx (May 26 2018 at 01:25, on Zulip):

have a good night y'all, I'll still hack for a little bit more

Santiago Pastorino (May 26 2018 at 12:52, on Zulip):

@qmx what do you have by now? wanna continue pairing?

Santiago Pastorino (May 26 2018 at 12:54, on Zulip):

or how can we continue? maybe split in some way?

qmx (May 26 2018 at 14:16, on Zulip):

I'm in a bad spot for pairing now

Santiago Pastorino (May 26 2018 at 14:26, on Zulip):

:+1:, how can we split the task?

qmx (May 26 2018 at 14:28, on Zulip):

@nikomatsakis that's where I'm getting stuck:

error[E0412]: cannot find type `PointIndex` in this scope
  --> librustc_mir/borrow_check/flows.rs:64:48
   |
64 |     crate fn borrows_in_scope(&self, location: PointIndex, each_borrow: impl FnMut(BorrowIndex)) {
   |                                                ^^^^^^^^^^ not found in this scope

error[E0308]: mismatched types
  --> librustc_mir/borrow_check/flows.rs:66:76
   |
66 |             for borrow in polonius.regions_live_at(location) { each_borrow(borrow) }
   |                                                                            ^^^^^^ expected struct `dataflow::move_paths::indexe
s::BorrowIndex`, found reference
   |
   = note: expected type `dataflow::move_paths::indexes::BorrowIndex`
              found type `&rustc::ty::RegionVid`

lemme unpack this
1) you mentioned that I should use polonius.errors or something like this - and mentioned that probable_errors wasn't a good name - shall I go and cut a PR for polonius-engine with those changes? Was the field the expected way of accessing this data?

2) I can't find PointIndex anywhere, did you meant another type?

3) I need the MirBorrowCtx inside FlowState::borrows_in_scope for resolving the indexes - is it ok if I capture it via closure?

qmx (May 26 2018 at 14:32, on Zulip):

changes are in this branch

Santiago Pastorino (May 26 2018 at 14:39, on Zulip):

1) I think we need to provide a method in polonius-engine similar to regions_live_at(location) but that returns potential_errors and change the name from potential_errors to errors

Santiago Pastorino (May 26 2018 at 14:40, on Zulip):

2) I think it's LocationIndex what you need

Santiago Pastorino (May 26 2018 at 15:45, on Zulip):

@nikomatsakis @qmx didn't have much time and I'm with the baby but I did this https://github.com/rust-lang-nursery/polonius/pull/56 I think we would need something like this so we can do output.errors_at(location) from rustc

Santiago Pastorino (May 26 2018 at 16:56, on Zulip):

@qmx I was rebasing your work on top of master

Santiago Pastorino (May 26 2018 at 16:56, on Zulip):

there are some conflicts

Santiago Pastorino (May 26 2018 at 16:56, on Zulip):

do you want me to push what I have?

Santiago Pastorino (May 26 2018 at 16:57, on Zulip):

I should have said ... rebasing because my PR landed

Santiago Pastorino (May 26 2018 at 16:57, on Zulip):

probably better to rebase than continue because otherwise would generate more conflicts

qmx (May 26 2018 at 17:01, on Zulip):

if you already rebased plese push it somewhere, otherwise I'll rebase

Santiago Pastorino (May 26 2018 at 17:10, on Zulip):

yep

Santiago Pastorino (May 26 2018 at 17:10, on Zulip):

https://github.com/spastorino/rust/commits/make_borrowck_use_output

qmx (May 26 2018 at 17:10, on Zulip):

:+1:

Santiago Pastorino (May 26 2018 at 17:10, on Zulip):

left out the wip commit

Santiago Pastorino (May 26 2018 at 17:11, on Zulip):

but I guess you can put my branch in yours again and add that on top :)

qmx (May 26 2018 at 17:11, on Zulip):

yea, I'll take care of that

Santiago Pastorino (May 26 2018 at 17:18, on Zulip):

actually have added your wip commit

Santiago Pastorino (May 26 2018 at 17:18, on Zulip):

it needed a little change

Santiago Pastorino (May 26 2018 at 17:19, on Zulip):

take a look at the stuff and feel free to get what you need or throw away :)

Santiago Pastorino (May 27 2018 at 23:03, on Zulip):

@qmx @nikomatsakis I've pushed a couple of fixes on top of the last qmx commit here https://github.com/spastorino/rust/commits/make_borrowck_use_output

Santiago Pastorino (May 27 2018 at 23:03, on Zulip):

I think the thing is almost there

Santiago Pastorino (May 27 2018 at 23:03, on Zulip):

couldn't spend any time during the weekend on this thing, just had a couple of minutes and could fix the stuff you see on the last commit

Santiago Pastorino (May 27 2018 at 23:04, on Zulip):

anyway it's giving some compiling errors as I think @nikomatsakis predicted

Santiago Pastorino (May 27 2018 at 23:04, on Zulip):
error[E0308]: `if let` arms have incompatible types
  --> librustc_mir/borrow_check/flows.rs:65:9
   |
65 | /         if let Some(polonius) = self.polonius_output {
66 | |             polonius.borrows_in_scope_at(location)
67 | |         } else {
68 | |             self.borrows.iter_incoming()
69 | |         }
   | |_________^ expected reference, found struct `std::iter::Peekable`
   |
   = note: expected type `&[dataflow::move_paths::indexes::BorrowIndex]`
              found type `std::iter::Peekable<rustc_data_structures::indexed_set::Iter<'_, dataflow::move_paths::indexes::BorrowIndex>>`
note: `if let` arm with an incompatible type
  --> librustc_mir/borrow_check/flows.rs:67:16
   |
67 |           } else {
   |  ________________^
68 | |             self.borrows.iter_incoming()
69 | |         }
   | |_________^

error: aborting due to previous error
Santiago Pastorino (May 27 2018 at 23:04, on Zulip):

Niko suggested to use callbacks for that but in order to use callbacks with the new code I think it needs a tiny refactor

Santiago Pastorino (May 27 2018 at 23:04, on Zulip):

couldn't look into that but tomorrow I can continue with this

Santiago Pastorino (May 27 2018 at 23:04, on Zulip):

if @qmx want we can pair again and try to finish this

qmx (May 27 2018 at 23:13, on Zulip):

Sure, tomorrow is rust day again :crab:

nikomatsakis (May 28 2018 at 10:11, on Zulip):

hey y'all — I'm around today, didn't really have access to a computer last few days, it turned out

nikomatsakis (May 28 2018 at 10:11, on Zulip):

is @Santiago Pastorino's branch still the latest thing?

nikomatsakis (May 28 2018 at 10:21, on Zulip):

Regarding the diffs in this HEAD commit:

Maybe the best option is to add a dependency on the Either crate. This... seems like a harmless enough crate to rely on. Then we could change the method body to:

    crate fn borrows_in_scope(&self, location: LocationIndex) -> impl Iterator<Item = BorrowIndex> + '_ {
        if let Some(polonius) = self.polonius_output {
            Either::Left(polonius.borrows_in_scope_at(location))
        } else {
            Either::Right(self.borrows.iter_incoming())
        }
}
Santiago Pastorino (May 28 2018 at 10:45, on Zulip):

hey y'all — I'm around today, didn't really have access to a computer last few days, it turned out

no worries, I only had a few minutes during the weekend

nikomatsakis (May 28 2018 at 10:45, on Zulip):

(technically, today is a holiday too, but I was planning to work at least somewhat to try and get on top of things)

Santiago Pastorino (May 28 2018 at 10:45, on Zulip):

is @Santiago Pastorino's branch still the latest thing?

is the last thing, yeah

Santiago Pastorino (May 28 2018 at 10:46, on Zulip):

(technically, today is a holiday too, but I was planning to work at least somewhat to try and get on top of things)

what happens today in US?

nikomatsakis (May 28 2018 at 10:47, on Zulip):

"Memorial Day" — officially for remembering those who died in war. Or, more commonly, to have a BBQ and enjoy summer weather.

nikomatsakis (May 28 2018 at 10:47, on Zulip):

Today it's raining, though, and I don't eat meat :)

Santiago Pastorino (May 28 2018 at 10:48, on Zulip):

hehehe

Santiago Pastorino (May 28 2018 at 10:52, on Zulip):

Using LocationIndex is correct. If needed, it can be imported from borrow_check::location. The different is that LocationIndex can refer to either the start or mid point of a Location; it's also the way that the facts from Polonius are organized. I was confused with the name and thought i had called it PointIndex (maybe we should rename it...).

did you mention this for some reason?

Santiago Pastorino (May 28 2018 at 10:52, on Zulip):

I mean, what's in the code is right or I understood your comment wrong?

nikomatsakis (May 28 2018 at 10:58, on Zulip):

I mean, what's in the code is right or I understood your comment wrong?

your code is correct

nikomatsakis (May 28 2018 at 10:58, on Zulip):

I was just confirming that :)

Santiago Pastorino (May 28 2018 at 10:58, on Zulip):

ok

Santiago Pastorino (May 28 2018 at 10:58, on Zulip):

:+1:

Santiago Pastorino (May 28 2018 at 10:58, on Zulip):

Either ... hehe, sounds useful for this case

Santiago Pastorino (May 28 2018 at 10:58, on Zulip):

had no idea such a thing existed

Santiago Pastorino (May 28 2018 at 10:59, on Zulip):

I guess the purpose is for cases like this?

Santiago Pastorino (May 28 2018 at 11:00, on Zulip):

README doesn't say the motivation for the existence of this thing

nikomatsakis (May 28 2018 at 11:02, on Zulip):

I guess the purpose is for cases like this?

it is one purpose, at least

nikomatsakis (May 28 2018 at 11:02, on Zulip):

yeah the docs are not great

nikomatsakis (May 28 2018 at 11:02, on Zulip):

but if you look at https://docs.rs/either/1.5.0/either/enum.Either.html you will see it has an impl of Iterator

nikomatsakis (May 28 2018 at 11:02, on Zulip):

https://docs.rs/either/1.5.0/either/enum.Either.html#impl-Iterator

Santiago Pastorino (May 28 2018 at 11:09, on Zulip):

ahh I see

Santiago Pastorino (May 28 2018 at 11:32, on Zulip):
error[E0271]: type mismatch resolving `<either::Either<std::slice::Iter<'_, dataflow::move_paths::indexes::BorrowIndex>, std::iter::Peekable<rustc_data_structures::indexed_set::Iter<'_, dataflow::move_paths::indexes::BorrowIndex>>> as std::iter::Iterator>::Item == dataflow::move_paths::indexes::BorrowIndex`
  --> librustc_mir/borrow_check/flows.rs:65:66
   |
65 |     crate fn borrows_in_scope(&self, location: LocationIndex) -> impl Iterator<Item = BorrowIndex> + '_ {
   |                                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected reference, found struct `dataflow::move_paths::indexes::BorrowIndex`
   |
   = note: expected type `&dataflow::move_paths::indexes::BorrowIndex`
              found type `dataflow::move_paths::indexes::BorrowIndex`
   = note: the return type of a function must have a statically known size

error[E0271]: type mismatch resolving `<std::iter::Peekable<rustc_data_structures::indexed_set::Iter<'_, dataflow::move_paths::indexes::BorrowIndex>> as std::iter::Iterator>::Item == &dataflow::move_paths::indexes::BorrowIndex`
  --> librustc_mir/borrow_check/flows.rs:65:66
   |
65 |     crate fn borrows_in_scope(&self, location: LocationIndex) -> impl Iterator<Item = BorrowIndex> + '_ {
   |                                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `dataflow::move_paths::indexes::BorrowIndex`, found reference
   |
   = note: expected type `dataflow::move_paths::indexes::BorrowIndex`
              found type `&dataflow::move_paths::indexes::BorrowIndex`
   = note: required because of the requirements on the impl of `std::iter::Iterator` for `either::Either<std::slice::Iter<'_, dataflow::move_paths::indexes::BorrowIndex>, std::iter::Peekable<rustc_data_structures::indexed_set::Iter<'_, dataflow::move_paths::indexes::BorrowIndex>>>`
   = note: the return type of a function must have a statically known size
nikomatsakis (May 28 2018 at 11:32, on Zulip):

can you send me the source? :)

Santiago Pastorino (May 28 2018 at 11:34, on Zulip):

https://github.com/spastorino/rust/commits/make_borrowck_use_output

Santiago Pastorino (May 28 2018 at 11:35, on Zulip):

just added the Either thing on top of what we talked about

nikomatsakis (May 28 2018 at 11:35, on Zulip):

yeah; so the problem is that your iterators — or at least some of them — are yielding up &BorrowIndex and not BorrowIndex

nikomatsakis (May 28 2018 at 11:35, on Zulip):

I suspect the problem is the new one :)

nikomatsakis (May 28 2018 at 11:36, on Zulip):

in particular .iter() over a Vec<T> (or any other collection) will yield up &T

nikomatsakis (May 28 2018 at 11:36, on Zulip):

to fix, just add .cloned()

nikomatsakis (May 28 2018 at 11:36, on Zulip):

.iter().cloned()

nikomatsakis (May 28 2018 at 11:36, on Zulip):

you can read more about .cloned() in rustdoc

nikomatsakis (May 28 2018 at 11:37, on Zulip):

/me senses a good blog post topic

Santiago Pastorino (May 28 2018 at 11:37, on Zulip):

:)

Santiago Pastorino (May 28 2018 at 11:37, on Zulip):

yeah have seen all this

Santiago Pastorino (May 28 2018 at 11:45, on Zulip):

this thing is compiling now

Santiago Pastorino (May 28 2018 at 11:45, on Zulip):

:+1:

nikomatsakis (May 28 2018 at 12:09, on Zulip):

@Santiago Pastorino nice!

Santiago Pastorino (May 28 2018 at 12:12, on Zulip):

gonna run tests

Santiago Pastorino (May 28 2018 at 12:16, on Zulip):

@nikomatsakis https://github.com/rust-lang-nursery/polonius/pull/56

Santiago Pastorino (May 28 2018 at 12:16, on Zulip):

we can call that from rustc

Santiago Pastorino (May 28 2018 at 12:16, on Zulip):

not sure if you've seen the PR

qmx (May 28 2018 at 12:19, on Zulip):

I was actually thinking that the callback version would have nothing as return

Santiago Pastorino (May 28 2018 at 12:19, on Zulip):

brb, will sync with Douglas when ... hehe :)

Santiago Pastorino (May 28 2018 at 12:19, on Zulip):

was writing this and you showed up

Santiago Pastorino (May 28 2018 at 12:19, on Zulip):

we are connected :P

qmx (May 28 2018 at 12:23, on Zulip):

Will ping once properly cafffeinated

nikomatsakis (May 28 2018 at 12:50, on Zulip):

@qmx we replaced the callbacks with an iterator using Either, which solves the same problem but perhaps more nicely

Santiago Pastorino (May 28 2018 at 12:50, on Zulip):

how do you pass -Zpolonius to tests?

Santiago Pastorino (May 28 2018 at 12:51, on Zulip):
$ RUST_BACKTRACE=full RUSTFLAGS=-Zpolonius ./x.py test --stage 1 --incremental src/test/{ui,run-pass,compile-fail}
Updating only changed submodules
Submodules updated in 0.03 seconds
    Finished dev [unoptimized] target(s) in 0.24s
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `/home/santiago/src/oss/rust1/build/bootstrap/debug/rustc - --crate-name ___ --print=file-names -Zpolonius --target x86_64-unknown-linux-gnu --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro` (exit code: 101)
--- stderr
error: unknown debugging option: `polonius`
Santiago Pastorino (May 28 2018 at 12:51, on Zulip):

gonna be back in a bit but wanted to leave this thing running :)

nikomatsakis (May 28 2018 at 12:51, on Zulip):

I mentioned this when talking to @qmx — I don't think there is a nice way, actually, you probably have to add a compare-mode to the test harness

Santiago Pastorino (May 28 2018 at 12:52, on Zulip):

ok

Santiago Pastorino (May 28 2018 at 12:53, on Zulip):

and the other thing I wanted to talk about is https://github.com/rust-lang-nursery/polonius/pull/56 and change to use errors_at in rustc

Santiago Pastorino (May 28 2018 at 12:53, on Zulip):

I'm not sure if current code make sense if you want to use errors_at

Santiago Pastorino (May 28 2018 at 12:53, on Zulip):

in particular

Santiago Pastorino (May 28 2018 at 12:54, on Zulip):
    crate fn borrows_in_scope(&self, location: LocationIndex) -> impl Iterator<Item = BorrowIndex> + '_ {
        if let Some(ref polonius) = self.polonius_output {
            Either::Left(polonius.borrows_in_scope_at(location).iter().cloned())
        } else {
            Either::Right(self.borrows.iter_incoming())
        }
    }
Santiago Pastorino (May 28 2018 at 12:54, on Zulip):

would be returning errors in the if part and borrows in the else part

Santiago Pastorino (May 28 2018 at 12:54, on Zulip):

well I guess we can do that if before calling borrows_in_scope

Santiago Pastorino (May 28 2018 at 12:55, on Zulip):

leave borrows_in_scope at it was

Santiago Pastorino (May 28 2018 at 12:55, on Zulip):

and use polonius output in the caller or something like that

pnkfelix (May 28 2018 at 13:03, on Zulip):

@Santiago Pastorino if you want help with the compare-mode stuff, I might be able to help

Santiago Pastorino (May 28 2018 at 13:17, on Zulip):

@pnkfelix :+1:

Santiago Pastorino (May 28 2018 at 13:17, on Zulip):

@qmx wanna do this with me?

Santiago Pastorino (May 28 2018 at 13:17, on Zulip):

will be available in ~ 20mins

qmx (May 28 2018 at 13:17, on Zulip):

sure

Santiago Pastorino (May 28 2018 at 13:18, on Zulip):

:+1:

Santiago Pastorino (May 28 2018 at 13:18, on Zulip):

will be back in ~ 20

nikomatsakis (May 28 2018 at 13:22, on Zulip):

I'm back-ish now

nikomatsakis (May 28 2018 at 13:22, on Zulip):

(fyi)

Santiago Pastorino (May 28 2018 at 13:58, on Zulip):

@qmx back

Santiago Pastorino (May 28 2018 at 13:59, on Zulip):

@qmx are you available to start with this?

qmx (May 28 2018 at 14:01, on Zulip):

yup, looking at your branch now

Santiago Pastorino (May 28 2018 at 14:02, on Zulip):

:+1:

Santiago Pastorino (May 28 2018 at 14:02, on Zulip):

I added one commit

Santiago Pastorino (May 28 2018 at 14:02, on Zulip):

then squashed with yours

Santiago Pastorino (May 28 2018 at 14:02, on Zulip):

we could fix the history to make a PR at some point

nikomatsakis (May 28 2018 at 14:03, on Zulip):

@Santiago Pastorino wait what was this about errors_at or whatever?

Santiago Pastorino (May 28 2018 at 14:03, on Zulip):

https://rust-lang.zulipchat.com/#narrow/stream/122657-wg-nll/subject/rustc-and-polonius.20integration/near/127204545

Santiago Pastorino (May 28 2018 at 14:03, on Zulip):

9.53am my time, I think it's 8.53am your time

nikomatsakis (May 28 2018 at 14:04, on Zulip):

ok. sadly links don't really work here for some reason.

Santiago Pastorino (May 28 2018 at 14:04, on Zulip):

and the other thing I wanted to talk about is https://github.com/rust-lang-nursery/polonius/pull/56 and change to use errors_at in rustc

starts at this point ^

nikomatsakis (May 28 2018 at 14:04, on Zulip):

ok I see

Santiago Pastorino (May 28 2018 at 14:04, on Zulip):

just scroll up a bit

nikomatsakis (May 28 2018 at 14:04, on Zulip):

would be returning errors in the if part and borrows in the else part

yes, that's right, and I believe it makes sense.

Santiago Pastorino (May 28 2018 at 14:05, on Zulip):

yeah, sorry, as I said on private, don't want to insist but right now it's impossible to follow for me, I can't imagine how is for you that follow way more than I do :)

nikomatsakis (May 28 2018 at 14:05, on Zulip):

well I mean you are pointing out a real inconsistency

nikomatsakis (May 28 2018 at 14:05, on Zulip):

however

nikomatsakis (May 28 2018 at 14:05, on Zulip):

the reason it makes sense is that we are basically interested only in borrows that lead to errors

nikomatsakis (May 28 2018 at 14:05, on Zulip):

it might make sense to rename the methods

nikomatsakis (May 28 2018 at 14:05, on Zulip):

this is a case where -- eventually -- polonius can be more efficient

nikomatsakis (May 28 2018 at 14:05, on Zulip):

basically right now we do something like:

qmx (May 28 2018 at 14:05, on Zulip):

yea, let's rename this for the sake of clarity

qmx (May 28 2018 at 14:06, on Zulip):

or at least comment heavily

nikomatsakis (May 28 2018 at 14:06, on Zulip):
for point P in all_points() {
  for loan L in scope at P {
    if is_error(L, P) {
      report_error(L, P);
    }
  }
}
nikomatsakis (May 28 2018 at 14:06, on Zulip):

however with polonius to generate the Invalidates facts, we've already figured out some parts of that

Santiago Pastorino (May 28 2018 at 14:06, on Zulip):

you mean borrows_in_scope to possible_errors_in_scope or something like that?

nikomatsakis (May 28 2018 at 14:06, on Zulip):

in particular we know which loans would be errors if they are in scope

nikomatsakis (May 28 2018 at 14:06, on Zulip):

that's what the Invalidates facts represent

nikomatsakis (May 28 2018 at 14:07, on Zulip):

so eventually we could just do

nikomatsakis (May 28 2018 at 14:07, on Zulip):
for (L, P) in errors {
    report_error(L, P);
}
Santiago Pastorino (May 28 2018 at 14:07, on Zulip):

yes errors in polonius and possible errors when not in polonius

nikomatsakis (May 28 2018 at 14:07, on Zulip):

but for now it's harmless enough to do

for point P in all_points() {
  for L in errors at P {
    if is_error(L, P) { // always true
      report_error(L, P);
    }
  }
}
nikomatsakis (May 28 2018 at 14:08, on Zulip):

yeah, possible_errors_in_scope_at(...) or whatever seems ok

Santiago Pastorino (May 28 2018 at 14:08, on Zulip):

yep so your last snippet would match just leaving things as they are renaming to ... exactly

Santiago Pastorino (May 28 2018 at 14:08, on Zulip):

and calling errors_at

Santiago Pastorino (May 28 2018 at 14:08, on Zulip):

well to call errors_at we need you to merge the PR and then to release :)

Santiago Pastorino (May 28 2018 at 14:09, on Zulip):

or we could point the thing to github so we don't bother you everytime we need something like this

Santiago Pastorino (May 28 2018 at 14:09, on Zulip):

what you prefer :)

qmx (May 28 2018 at 14:09, on Zulip):

I think we kinda wanted to avoid git dependencies

nikomatsakis (May 28 2018 at 14:10, on Zulip):

yeah, we do, but Either is tiny ;)

nikomatsakis (May 28 2018 at 14:10, on Zulip):

I don't really know how to draw the line here

nikomatsakis (May 28 2018 at 14:10, on Zulip):

on the one hand I dont' want to re-invent the wheel, on the other I don't want to go crazy cloning the world

qmx (May 28 2018 at 14:10, on Zulip):

isn't Either a crate already?

nikomatsakis (May 28 2018 at 14:11, on Zulip):

right

nikomatsakis (May 28 2018 at 14:11, on Zulip):

I maybe answering the wrong thing :)

nikomatsakis (May 28 2018 at 14:11, on Zulip):

I thought you were reacting to using the Either crate

nikomatsakis (May 28 2018 at 14:11, on Zulip):

and I'm saying that I think we might as well use it

nikomatsakis (May 28 2018 at 14:11, on Zulip):

though in general I try to avoid adding deps

qmx (May 28 2018 at 14:11, on Zulip):

no, I'm talking about you merging @Santiago Pastorino 's PR on polonius and cutting another release

qmx (May 28 2018 at 14:11, on Zulip):

:)

nikomatsakis (May 28 2018 at 14:11, on Zulip):

oh :)

nikomatsakis (May 28 2018 at 14:12, on Zulip):

yes I can do that

nikomatsakis (May 28 2018 at 14:12, on Zulip):

we should avoid a git dep

nikomatsakis (May 28 2018 at 14:12, on Zulip):

though if you want to just make a git dep to keep hacking while I do so

nikomatsakis (May 28 2018 at 14:12, on Zulip):

seems fine

nikomatsakis (May 28 2018 at 14:12, on Zulip):

we can always cut a final release just before landing

qmx (May 28 2018 at 14:12, on Zulip):

we're kinda ready for a PR if I'm understanding correctly

nikomatsakis (May 28 2018 at 14:12, on Zulip):

point is, there may be more ;)

nikomatsakis (May 28 2018 at 14:12, on Zulip):

anyway I merged the PD

qmx (May 28 2018 at 14:12, on Zulip):

point taken

nikomatsakis (May 28 2018 at 14:13, on Zulip):

and I can happily issue a release, it's no big deal

pnkfelix (May 28 2018 at 14:13, on Zulip):

(I don't personally see the point of using Either; isn't that a trivial enum to write, and more readable since you can choose names that have meaning tailored to the usage?

Santiago Pastorino (May 28 2018 at 14:13, on Zulip):

though if you want to just make a git dep to keep hacking while I do so

yes I was referring to this, not to keep it as a long term thing

Santiago Pastorino (May 28 2018 at 14:13, on Zulip):

whatever works better for you

pnkfelix (May 28 2018 at 14:13, on Zulip):

Maybe I'm missing some benefit encoded in the either crate

nikomatsakis (May 28 2018 at 14:14, on Zulip):

impl<L: Iterator, R: Iterator> Iterator for Either<L, R>

Santiago Pastorino (May 28 2018 at 14:14, on Zulip):

I agree pnkfelix I think

nikomatsakis (May 28 2018 at 14:14, on Zulip):

we can rewrite that code, but why?

nikomatsakis (May 28 2018 at 14:14, on Zulip):

indeed, the reason that Either exists is precisely to encode this basically :) it got pulled out of itertools so that rayon could depend on it (and only it)

Santiago Pastorino (May 28 2018 at 14:14, on Zulip):

we don't rewrite we just copy and paste :joy:

pnkfelix (May 28 2018 at 14:15, on Zulip):

@nikomatsakis oh it has something special when the two have the same Iterator::Item? (Yoiu left that detail out I think)

nikomatsakis (May 28 2018 at 14:15, on Zulip):

sure :)

Santiago Pastorino (May 28 2018 at 14:15, on Zulip):

but if it's a common thing that people use maybe is worth leaving the dep

nikomatsakis (May 28 2018 at 14:15, on Zulip):

I think it requires them to have the same Item

pnkfelix (May 28 2018 at 14:15, on Zulip):

(otherwise ... it wouldn't make sense...?)

nikomatsakis (May 28 2018 at 14:15, on Zulip):

I feel torn overall. I like the idea of rustc "playing nice" with the ecosystem, as I said, and using stuff that makes sense. But I don't want us to go crazy and pull in a bunch of big, unvettable deps.

nikomatsakis (May 28 2018 at 14:16, on Zulip):

(otherwise ... it wouldn't make sense...?)

yeah, not really, though I guess you could yield up Either<L::Item, R::Item> as your item. But I think nobody wants that in practice.

nikomatsakis (May 28 2018 at 14:16, on Zulip):

in practice it's really useful for -> impl Iterator<Item = ..>, when you might have multiple ways to fulfill it

nikomatsakis (May 28 2018 at 14:17, on Zulip):

I feel torn overall. I like the idea of rustc "playing nice" with the ecosystem, as I said, and using stuff that makes sense. But I don't want us to go crazy and pull in a bunch of big, unvettable deps.

to complete this thought: this seems like a sample case that exists and hence is easy to vet (and I trust bluss)

pnkfelix (May 28 2018 at 14:17, on Zulip):

Oh true, I wonder what the category theorists would say is "more natural"

nikomatsakis (May 28 2018 at 14:17, on Zulip):

I wouldn't be surprised if bluss were happy to move to rust-lang-nursery, tbh

pnkfelix (May 28 2018 at 14:18, on Zulip):

anyway if in this case the Left and Right do indeed have Iterators with the same Item, then I retract my objection to using the either crate here.

Santiago Pastorino (May 28 2018 at 14:20, on Zulip):

what other usage in practice you see the either crate having?

Santiago Pastorino (May 28 2018 at 14:20, on Zulip):

I mean, not talking inside rustc, talking in general

nikomatsakis (May 28 2018 at 14:21, on Zulip):

this is the primary usage :) but it is also used in itertools in some cases where you are gluing two sources together

nikomatsakis (May 28 2018 at 14:22, on Zulip):

can't remember what

nikomatsakis (May 28 2018 at 14:22, on Zulip):

I know though that several times in rustc I have wanted the ability to return either A or B for iterators

nikomatsakis (May 28 2018 at 14:22, on Zulip):

exactly like this case

nikomatsakis (May 28 2018 at 14:22, on Zulip):

I usually just workaround it some other way because I've been too lazy to add the either crate as a dep ;)

qmx (May 28 2018 at 14:23, on Zulip):

do you think it's good to cut a PR with what we have already?

nikomatsakis (May 28 2018 at 14:23, on Zulip):

have you tried it on any tests yet? :) like, by hand?

nikomatsakis (May 28 2018 at 14:23, on Zulip):

but yes, I think it's basically at roughly the right spot

nikomatsakis (May 28 2018 at 14:23, on Zulip):

we can cover the rest in review I suppose

nikomatsakis (May 28 2018 at 14:23, on Zulip):

I would like to see the compare-mode tests so we can actually test it

pnkfelix (May 28 2018 at 14:24, on Zulip):

I would be entirely happy if -Z polonius landed before compare-mode

Santiago Pastorino (May 28 2018 at 14:24, on Zulip):

gonna work on the compare-mode now

nikomatsakis (May 28 2018 at 14:24, on Zulip):

just seems like it's like a 1-line diff to add :)

pnkfelix (May 28 2018 at 14:24, on Zulip):

compare-mode support itself seems pretty easy to add. The hard part will be the tests

nikomatsakis (May 28 2018 at 14:24, on Zulip):

right, I don't feel the need to add those

pnkfelix (May 28 2018 at 14:24, on Zulip):

(and even there, you could just bless them all)

nikomatsakis (May 28 2018 at 14:25, on Zulip):

I see, maybe you do want to add them

Santiago Pastorino (May 28 2018 at 14:25, on Zulip):

I can add the mode and run by hand then

pnkfelix (May 28 2018 at 14:25, on Zulip):

what we don't want to do

pnkfelix (May 28 2018 at 14:25, on Zulip):

is accidentally turn on compare-mode=polonius in bors

nikomatsakis (May 28 2018 at 14:25, on Zulip):

one problem with compare-mode as is

nikomatsakis (May 28 2018 at 14:25, on Zulip):

is that it compares against the "default"

nikomatsakis (May 28 2018 at 14:25, on Zulip):

whereas I would prefer for polonius to compare against "nll" :)

nikomatsakis (May 28 2018 at 14:25, on Zulip):

I wonder how hard that would be to change

pnkfelix (May 28 2018 at 14:25, on Zulip):

oh... hmm...

nikomatsakis (May 28 2018 at 14:26, on Zulip):

now that I think about the complications I think I agree with @pnkfelix that we should land the PR w/o compare-mode first

nikomatsakis (May 28 2018 at 14:26, on Zulip):

or at least open it

nikomatsakis (May 28 2018 at 14:26, on Zulip):

but maybe compare-mode is the next logical thing to hack on ...

pnkfelix (May 28 2018 at 14:27, on Zulip):

@nikomatsakis I imagine what you mean is that we want a fallback chain

qmx (May 28 2018 at 14:27, on Zulip):

good, I'll cut a PR then

pnkfelix (May 28 2018 at 14:27, on Zulip):

@nikomatsakis where we would search first if test.polonius.stderr exists, use it. If not, look for test.nll.stderr. And then finally test.stderr

pnkfelix (May 28 2018 at 14:28, on Zulip):

I could probably whip something up

nikomatsakis (May 28 2018 at 14:28, on Zulip):

@Santiago Pastorino @qmx remind me, does your PR include mir-dump integration?

Santiago Pastorino (May 28 2018 at 14:28, on Zulip):

no

Santiago Pastorino (May 28 2018 at 14:28, on Zulip):

I think we need to add that

pnkfelix (May 28 2018 at 14:28, on Zulip):

once the basic -Z polonius (with or without --compare-mode=polonius) lands

nikomatsakis (May 28 2018 at 14:28, on Zulip):

ok, so I think there are at least 2 (independent) things to potentially follow up on then

Santiago Pastorino (May 28 2018 at 14:28, on Zulip):

and probably change to use errors_at

nikomatsakis (May 28 2018 at 14:28, on Zulip):

compare-mode + MIr-dump integration

Santiago Pastorino (May 28 2018 at 14:28, on Zulip):

seems good

Santiago Pastorino (May 28 2018 at 14:28, on Zulip):

then we should just change to errors_at?

nikomatsakis (May 28 2018 at 14:29, on Zulip):

fyi https://github.com/rust-lang-nursery/polonius/pull/57

Santiago Pastorino (May 28 2018 at 14:29, on Zulip):

or do you prefer to land it this way and then change to errors_at later?

nikomatsakis (May 28 2018 at 14:29, on Zulip):

@Santiago Pastorino do you mean the method on Flows or whatever?

nikomatsakis (May 28 2018 at 14:29, on Zulip):

if so, I think we should rename before landing

Santiago Pastorino (May 28 2018 at 14:29, on Zulip):

yes

Santiago Pastorino (May 28 2018 at 14:29, on Zulip):

ok

Santiago Pastorino (May 28 2018 at 14:29, on Zulip):

going to do this

Santiago Pastorino (May 28 2018 at 14:29, on Zulip):

and we can cut the PR

Santiago Pastorino (May 28 2018 at 14:30, on Zulip):

will be just a couple of minutes

nikomatsakis (May 28 2018 at 14:31, on Zulip):

@Santiago Pastorino published

Santiago Pastorino (May 28 2018 at 14:32, on Zulip):

:+1:

Santiago Pastorino (May 28 2018 at 14:32, on Zulip):

compiling the changes :)

Santiago Pastorino (May 28 2018 at 14:45, on Zulip):

so ... I'm hitting one issue

Santiago Pastorino (May 28 2018 at 14:46, on Zulip):

we had a &[Loan] thing and now have a Cow<'_, Vec<Loan>>

Santiago Pastorino (May 28 2018 at 14:47, on Zulip):

hitting Rust newbies problems :D

Santiago Pastorino (May 28 2018 at 14:47, on Zulip):
error[E0597]: borrowed value does not live long enough
  --> librustc_mir/borrow_check/flows.rs:67:26
   |
67 |             Either::Left(polonius.errors_at(location).iter().cloned())
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ temporary value does not live long enough
68 |         } else {
   |         - temporary value only lives until here
   |
note: borrowed value must be valid for the anonymous lifetime #1 defined on the method body at 65:5...
  --> librustc_mir/borrow_check/flows.rs:65:5
   |
65 | /     crate fn borrows_in_scope(&self, location: LocationIndex) -> impl Iterator<Item = BorrowIndex> + '_ {
66 | |         if let Some(ref polonius) = self.polonius_output {
67 | |             Either::Left(polonius.errors_at(location).iter().cloned())
68 | |         } else {
69 | |             Either::Right(self.borrows.iter_incoming())
70 | |         }
71 | |     }
   | |_____^

error: aborting due to previous error
Santiago Pastorino (May 28 2018 at 14:49, on Zulip):

I don't understand what's going on very well because the data comes from the same place so unsure what's happening with lifetimes and what Cow changes

nikomatsakis (May 28 2018 at 14:50, on Zulip):

try into_iter and not iter

nikomatsakis (May 28 2018 at 14:50, on Zulip):

this is definitely a good blog post (and probably something we should improve error messages for)

nikomatsakis (May 28 2018 at 14:51, on Zulip):

what's going on is that errors_at is returning a temporary (the Cow) which maybe owns some data

nikomatsakis (May 28 2018 at 14:51, on Zulip):

when you invoke iter(), it borrows the thing you are iterating over — so when you return that iterator, it is still borrowing from the stack

nikomatsakis (May 28 2018 at 14:51, on Zulip):

this worked out before because you had a &[] you were iterating over, which was always borrowed

nikomatsakis (May 28 2018 at 14:51, on Zulip):

but now the Cow maybe owns it

Santiago Pastorino (May 28 2018 at 14:51, on Zulip):

when you invoke iter(), it borrows the thing you are iterating over — so when you return that iterator, it is still borrowing from the stack

ahh I see

nikomatsakis (May 28 2018 at 14:52, on Zulip):

so you get an error because you would be dropping the Cow while still iterating over it

nikomatsakis (May 28 2018 at 14:52, on Zulip):

using into_iter avoids that borrow

nikomatsakis (May 28 2018 at 14:52, on Zulip):

that said, Cow may not implement into_iter...?

nikomatsakis (May 28 2018 at 14:52, on Zulip):

so this may not actually work

Santiago Pastorino (May 28 2018 at 14:52, on Zulip):
error[E0507]: cannot move out of borrowed content
  --> librustc_mir/borrow_check/flows.rs:67:26
   |
67 |             Either::Left(polonius.errors_at(location).into_iter())
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot move out of borrowed content

error: aborting due to previous error

For more information about this error, try `rustc --explain E0507`.
error: Could not compile `rustc_mir`.
nikomatsakis (May 28 2018 at 14:52, on Zulip):

yeah hmm I wonder what's the right way to do this

nikomatsakis (May 28 2018 at 14:52, on Zulip):

:)

nikomatsakis (May 28 2018 at 14:53, on Zulip):

(why are we returning Cow anyway?)

nikomatsakis (May 28 2018 at 14:53, on Zulip):

oh, we shouldn't be :)

nikomatsakis (May 28 2018 at 14:53, on Zulip):

that's the simple fix

nikomatsakis (May 28 2018 at 14:53, on Zulip):

I'll open a PR to show you what I mean ;)

Santiago Pastorino (May 28 2018 at 14:54, on Zulip):

yep

Santiago Pastorino (May 28 2018 at 14:54, on Zulip):

I guessed we should always return a borrow from polonius-engine

Santiago Pastorino (May 28 2018 at 14:54, on Zulip):

I saw there are a lot of cases doing Vec::default() so owning the thing and using Cow for that purposes

nikomatsakis (May 28 2018 at 14:54, on Zulip):

also, we should not be assering dump_enabled

nikomatsakis (May 28 2018 at 14:54, on Zulip):

if this is the primary output, then dump_enabled should not have to be true

Santiago Pastorino (May 28 2018 at 14:54, on Zulip):

:+1:

Santiago Pastorino (May 28 2018 at 14:55, on Zulip):

to be honest that part I just copy pasted the whole thing :P

Santiago Pastorino (May 28 2018 at 14:55, on Zulip):

now that I'm think I guess we should always borrow stuff from there

nikomatsakis (May 28 2018 at 14:56, on Zulip):

https://github.com/rust-lang-nursery/polonius/pull/59

nikomatsakis (May 28 2018 at 14:57, on Zulip):

@Santiago Pastorino well slices are a bit .. special

Santiago Pastorino (May 28 2018 at 14:57, on Zulip):

hehe yes, makes a lot of sense

Santiago Pastorino (May 28 2018 at 14:57, on Zulip):

yeah

nikomatsakis (May 28 2018 at 14:57, on Zulip):

that is, you can make an empty slice &[] with any lifetime because it is a compile-time constant

nikomatsakis (May 28 2018 at 14:57, on Zulip):

the other things return hashmaps and so forth

nikomatsakis (May 28 2018 at 14:57, on Zulip):

they require a Cow

Santiago Pastorino (May 28 2018 at 14:57, on Zulip):

ahhhh

nikomatsakis (May 28 2018 at 14:57, on Zulip):

though eventually they should not

Santiago Pastorino (May 28 2018 at 14:57, on Zulip):

yeah so you can't do the same trick for the rest

nikomatsakis (May 28 2018 at 14:57, on Zulip):

(that is, eventually we should be able to execute HashMap::new at compile time to get a constant, but we can't yet)

nikomatsakis (May 28 2018 at 14:58, on Zulip):

anyway it's easier if you don't need Cow

Santiago Pastorino (May 28 2018 at 14:58, on Zulip):

yes

Santiago Pastorino (May 28 2018 at 14:58, on Zulip):

makes a lot of sense

Santiago Pastorino (May 28 2018 at 14:58, on Zulip):

you need to cut another release :P

Santiago Pastorino (May 28 2018 at 14:58, on Zulip):

0.3.0 wasted :P

nikomatsakis (May 28 2018 at 14:59, on Zulip):

heh see this is why I said we ought to work off master first ;)

nikomatsakis (May 28 2018 at 14:59, on Zulip):

(no big thing)

Santiago Pastorino (May 28 2018 at 15:00, on Zulip):

yes and I agree

Santiago Pastorino (May 28 2018 at 15:00, on Zulip):

if you want I can make the thing point to master until we are in a stable point

nikomatsakis (May 28 2018 at 15:04, on Zulip):

@Santiago Pastorino I landed the changes on master, if you want to try that first

nikomatsakis (May 28 2018 at 15:05, on Zulip):

I can also issue a new release though

Santiago Pastorino (May 28 2018 at 15:06, on Zulip):

can try with master

Santiago Pastorino (May 28 2018 at 15:39, on Zulip):

@nikomatsakis the thing compiled

Santiago Pastorino (May 28 2018 at 15:39, on Zulip):

you want me to provide a PR depending on github? or you want now the cut the release?

nikomatsakis (May 28 2018 at 15:45, on Zulip):

I'll cut the release @Santiago Pastorino

nikomatsakis (May 28 2018 at 15:45, on Zulip):

done

Santiago Pastorino (May 28 2018 at 15:51, on Zulip):

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

nikomatsakis (May 28 2018 at 15:53, on Zulip):

nice job @Santiago Pastorino and @qmx

Santiago Pastorino (May 28 2018 at 15:54, on Zulip):

@nikomatsakis so ... didn't get quite correct if you wanted to do something about compare or not

Santiago Pastorino (May 28 2018 at 15:55, on Zulip):

and what about dump_mir

Santiago Pastorino (May 28 2018 at 15:55, on Zulip):

but I'm ready to start something else on top of this

nikomatsakis (May 28 2018 at 15:57, on Zulip):

@Santiago Pastorino I think @pnkfelix and I were saying let's do the compare-mode in a separate PR

nikomatsakis (May 28 2018 at 15:57, on Zulip):

but I think it's a good thing to get started with

Santiago Pastorino (May 28 2018 at 15:57, on Zulip):

cool

Santiago Pastorino (May 28 2018 at 15:57, on Zulip):

better to do that or dump-mir?

nikomatsakis (May 28 2018 at 15:58, on Zulip):

if I had my druthers, you would do compare-mode and @qmx would do dump-mir :)

nikomatsakis (May 28 2018 at 15:58, on Zulip):

or vice versa :)

nikomatsakis (May 28 2018 at 15:58, on Zulip):

because both seem imp't

nikomatsakis (May 28 2018 at 15:58, on Zulip):

but really either is good

qmx (May 28 2018 at 16:00, on Zulip):

sounds good

qmx (May 28 2018 at 16:00, on Zulip):

tell me more about dump-mir

nikomatsakis (May 28 2018 at 16:01, on Zulip):

well the idea is to take the add'l bits of Output and integrate them into the -Zdump-mir output

nikomatsakis (May 28 2018 at 16:01, on Zulip):

because both seem imp't

to clarify: both seem imp't, and yet they seem like nicely splittable tasks

nikomatsakis (May 28 2018 at 16:02, on Zulip):

right now I thikn the Output data is mostly organized by Point

nikomatsakis (May 28 2018 at 16:02, on Zulip):

this is because it's a useful way to "read" it as a human

qmx (May 28 2018 at 16:03, on Zulip):

so when you say additional data, is all the contents of the hashmaps?

qmx (May 28 2018 at 16:03, on Zulip):

is there an expected format for dumping all that info?

nikomatsakis (May 28 2018 at 16:05, on Zulip):

@qmx if you look at my old commit, you can see what I was doing before

nikomatsakis (May 28 2018 at 16:05, on Zulip):

it's kind of up to you

nikomatsakis (May 28 2018 at 16:05, on Zulip):

the format I mean

nikomatsakis (May 28 2018 at 16:06, on Zulip):

in particular down here, when we dump out a given location,

nikomatsakis (May 28 2018 at 16:06, on Zulip):

I was also going over the start/mid points associated with it,

nikomatsakis (May 28 2018 at 16:06, on Zulip):

and dumping out the related tuples

qmx (May 28 2018 at 16:07, on Zulip):

aaaah, now it makes more sense

qmx (May 28 2018 at 16:07, on Zulip):

:+1:

Jake Goulding (May 28 2018 at 16:08, on Zulip):

what other usage in practice you see the either crate having?

Futures, for the same reason. Although I think they reimplemented it internally :shrug:

nikomatsakis (May 28 2018 at 16:15, on Zulip):

probably unaware of the crate

nikomatsakis (May 28 2018 at 16:16, on Zulip):

or just didn't feel like adding the dep I guess

qmx (May 28 2018 at 16:55, on Zulip):

@Santiago Pastorino there are tidy errors, fixes here https://github.com/qmx/rust/compare/make_borrowck_use_output...qmx:fix-tidy?expand=1

Santiago Pastorino (May 28 2018 at 16:56, on Zulip):

already pushed

lqd (May 28 2018 at 16:56, on Zulip):

(The 51133 PR fails CI because of tidy and I think the :frog: whitelisting)

Santiago Pastorino (May 28 2018 at 16:56, on Zulip):

yeah, I've already pushed fixes

qmx (May 28 2018 at 16:56, on Zulip):

awesome

lqd (May 28 2018 at 16:56, on Zulip):

:) (sorry, backlogging on my phone)

Santiago Pastorino (May 28 2018 at 16:57, on Zulip):

no worries, thanks for letting me know :)

Santiago Pastorino (May 28 2018 at 16:57, on Zulip):

gonna read the log of this channel now :)

Santiago Pastorino (May 28 2018 at 16:59, on Zulip):

if I had my druthers, you would do compare-mode and @qmx would do dump-mir :)

@nikomatsakis @qmx sounds like a plan

nikomatsakis (May 28 2018 at 17:57, on Zulip):

@Santiago Pastorino @qmx have you tried this PR on some inputs?

nikomatsakis (May 28 2018 at 17:57, on Zulip):

e.g., did you try running on issue-47680.rs from the polonius repo etc?

nikomatsakis (May 28 2018 at 17:57, on Zulip):

it looks basically right to me

nikomatsakis (May 28 2018 at 17:57, on Zulip):

i think I'll build and try it out though

Santiago Pastorino (May 28 2018 at 18:11, on Zulip):

@nikomatsakis yes, I've tried it

Santiago Pastorino (May 28 2018 at 18:12, on Zulip):

let me run it again for you

Santiago Pastorino (May 28 2018 at 18:12, on Zulip):
[santiago@archlinux oss]$ rustc +stage1 polonius/inputs/issue-47680/issue-47680.rs
error[E0499]: cannot borrow `*temp` as mutable more than once at a time
  --> polonius/inputs/issue-47680/issue-47680.rs:13:15
   |
13 |         match temp.maybe_next() {
   |               ^^^^
   |               |
   |               mutable borrow starts here in previous iteration of loop
   |               borrow later used here

error: aborting due to previous error

For more information about this error, try `rustc --explain E0499`.
[santiago@archlinux oss]$ rustc +stage1 -Zpolonius polonius/inputs/issue-47680/issue-47680.rs
[santiago@archlinux oss]$
nikomatsakis (May 28 2018 at 18:14, on Zulip):

@Santiago Pastorino I presume that test has #![feature(nll)]?

nikomatsakis (May 28 2018 at 18:14, on Zulip):

if so, nice!

Santiago Pastorino (May 28 2018 at 18:16, on Zulip):

yes

Santiago Pastorino (May 28 2018 at 18:18, on Zulip):

can we do something like #![feature(nll, polonius)]

Santiago Pastorino (May 28 2018 at 18:18, on Zulip):

or something like that?

Santiago Pastorino (May 28 2018 at 18:18, on Zulip):

unsure if makes sense

Santiago Pastorino (May 28 2018 at 18:18, on Zulip):

would be a temporary thing

nikomatsakis (May 28 2018 at 18:19, on Zulip):

we could... but yeah I'm not sure if it makes sense

Santiago Pastorino (May 28 2018 at 23:27, on Zulip):

@nikomatsakis seems like https://github.com/rust-lang/rust/pull/51133 is green, needs an r+

nikomatsakis (May 28 2018 at 23:51, on Zulip):

@Santiago Pastorino interestingly, my local runs reveal a number of tests passing that ought not to — the failures look very familiar. But I don't think it's the fault of the integration, more that it's revealing bugs in polonius. One thing I was thinking: we should add a way to select the polonius algorithm. I was considering maybe an environment var? RUSTC_POLONIUS_ALGORITHM=naive or something? (Something rustc could check)

nikomatsakis (May 28 2018 at 23:51, on Zulip):

or we could make it -Zpolonius=foo, but that makes compare-mode more complex

nikomatsakis (May 28 2018 at 23:51, on Zulip):

(cc @qmx)

Santiago Pastorino (May 28 2018 at 23:53, on Zulip):

yes

Santiago Pastorino (May 28 2018 at 23:53, on Zulip):

I can add that tomorrow I guess

Santiago Pastorino (May 28 2018 at 23:53, on Zulip):

will ping you tomorrow

nikomatsakis (May 29 2018 at 00:04, on Zulip):

I'm gonna r+ as is since the queue is kinda empty right now

nikomatsakis (May 29 2018 at 00:05, on Zulip):

can always do as a follow up

nikomatsakis (May 29 2018 at 08:21, on Zulip):

@Vytautas Astrauskas you can see my comment earler in this thread

nikomatsakis (May 29 2018 at 08:21, on Zulip):

but actually before that

nikomatsakis (May 29 2018 at 08:21, on Zulip):

there is some work needed on polonius side

nikomatsakis (May 29 2018 at 08:22, on Zulip):

nothing too major :)

nikomatsakis (May 29 2018 at 08:22, on Zulip):

I think we need some function to take an algorithm string and select an algorithm

nikomatsakis (May 29 2018 at 08:23, on Zulip):

currently we have two definitions of the Algorithm enum, one of them in the cli interface and one in the engine itself

nikomatsakis (May 29 2018 at 08:23, on Zulip):

I think we should just have one — in the engine

nikomatsakis (May 29 2018 at 08:23, on Zulip):

and it should have a FromStr impl

nikomatsakis (May 29 2018 at 08:24, on Zulip):

used by the CLI

nikomatsakis (May 29 2018 at 08:24, on Zulip):

I'm not 100% sure what is needed to make clap integration work mega smoothly but presumably it is possible

nikomatsakis (May 29 2018 at 08:24, on Zulip):

(this is what that arg_enum! macro does)

nikomatsakis (May 29 2018 at 08:25, on Zulip):

docs for arg_enum!

nikomatsakis (May 29 2018 at 08:25, on Zulip):

(we could then add another variant to do comparisons)

Vytautas Astrauskas (May 29 2018 at 08:29, on Zulip):

@nikomatsakis I will try to fix this the selection of the algorithm.

nikomatsakis (May 29 2018 at 09:52, on Zulip):

@Santiago Pastorino think you or @qmx would have time to rebase https://github.com/rust-lang/rust/pull/51133 ? if not, I can do it

nikomatsakis (May 29 2018 at 09:52, on Zulip):

in fact, maybe I just will do it

nikomatsakis (May 29 2018 at 09:52, on Zulip):

seeing as the queue is kinda empty right now :)

nikomatsakis (May 29 2018 at 09:54, on Zulip):

(force pushed)

Vytautas Astrauskas (May 29 2018 at 10:29, on Zulip):

@nikomatsakis A pull request that removesAlgorithmOpt enum: https://github.com/rust-lang-nursery/polonius/pull/60. I have also added an option to select the algorithm via POLONIUS_ALGORITHM environment variable.

Vytautas Astrauskas (May 29 2018 at 10:39, on Zulip):

Next steps: add Compare to the Algorithm enum that compares errors reported by DatafrogOpt with the ones reported by Naive. @nikomatsakis what you would like to happen when computed errors differ? Panic with bug!?

Santiago Pastorino (May 29 2018 at 11:55, on Zulip):

@Santiago Pastorino think you or @qmx would have time to rebase https://github.com/rust-lang/rust/pull/51133 ? if not, I can do it

rebased

qmx (May 29 2018 at 11:55, on Zulip):

oh, niko did it already

Santiago Pastorino (May 29 2018 at 11:55, on Zulip):

ahh ok

Santiago Pastorino (May 29 2018 at 11:55, on Zulip):

nevermind then

qmx (May 29 2018 at 11:56, on Zulip):

...and we're back to the bottom of the queue :cry:

nikomatsakis (May 29 2018 at 12:17, on Zulip):

@qmx no more :)

nikomatsakis (May 29 2018 at 12:23, on Zulip):

@Vytautas Astrauskas how goes btw ?

Vytautas Astrauskas (May 29 2018 at 12:38, on Zulip):

@nikomatsakis

@Vytautas Astrauskas how goes btw ?

Currently writing the code that compares errors returned by Naïve and Optimized.

Vytautas Astrauskas (May 29 2018 at 12:38, on Zulip):

I am thinking to report differences via error! and then panic!.

nikomatsakis (May 29 2018 at 12:40, on Zulip):

seems fine for now; we could also make the computation return a Result

Vytautas Astrauskas (May 29 2018 at 12:43, on Zulip):

If computation returned a Result, I would expect that result to be either the borrow checking succeeded or a list of borrow errors.

Vytautas Astrauskas (May 29 2018 at 12:44, on Zulip):

In other words, I think that it would be confusing API to use Err to indicate a bug.

nikomatsakis (May 29 2018 at 12:44, on Zulip):

well, you might expect that, but that doesn't have to be what it is ;)

nikomatsakis (May 29 2018 at 12:45, on Zulip):

but I think panic is fine for now

nikomatsakis (May 29 2018 at 12:45, on Zulip):

and .. probably the right thing

nikomatsakis (May 29 2018 at 13:09, on Zulip):

argh @Santiago Pastorino https://github.com/rust-lang/rust/pull/51133 needs rebase again

nikomatsakis (May 29 2018 at 13:09, on Zulip):

it's just Cargo.lock conflicts I think

nikomatsakis (May 29 2018 at 13:09, on Zulip):

are you able to do it?

Santiago Pastorino (May 29 2018 at 13:26, on Zulip):

ohh saw you were pinging me here

Santiago Pastorino (May 29 2018 at 13:27, on Zulip):

starting to read this :)

Santiago Pastorino (May 29 2018 at 13:30, on Zulip):

@nikomatsakis what happened now https://github.com/rust-lang/rust/pull/51133#issuecomment-392775090 ?

nikomatsakis (May 29 2018 at 13:31, on Zulip):

@Santiago Pastorino looks like an error when merging the cargo.lock ?

nikomatsakis (May 29 2018 at 13:31, on Zulip):

[00:01:16] error: failed to parse lock file at: /checkout/src/Cargo.lock

nikomatsakis (May 29 2018 at 13:32, on Zulip):

usually what I do when there is a Cargo.lock conflict is to rerun x.py

nikomatsakis (May 29 2018 at 13:32, on Zulip):

which will re-generate it

nikomatsakis (May 29 2018 at 13:32, on Zulip):

I usually kill it before it gets too far :)

nikomatsakis (May 29 2018 at 13:32, on Zulip):

you may need to do that

nikomatsakis (May 29 2018 at 13:33, on Zulip):

though I don't see any obvious problem

nikomatsakis (May 29 2018 at 13:34, on Zulip):

but I get the same error locally

Santiago Pastorino (May 29 2018 at 13:35, on Zulip):

yes

Santiago Pastorino (May 29 2018 at 13:35, on Zulip):

was surprised that git rebase master didn't fail

Santiago Pastorino (May 29 2018 at 13:35, on Zulip):

I mean, it was what I did when merging

Santiago Pastorino (May 29 2018 at 13:35, on Zulip):

checkout the lock and rerun x.py

Santiago Pastorino (May 29 2018 at 13:36, on Zulip):

but this last merge conflicts didn't conflict locally and for some reason did the wrong thing

Santiago Pastorino (May 29 2018 at 13:36, on Zulip):

anyway ... fixing it

nikomatsakis (May 29 2018 at 13:37, on Zulip):

I just pushed a fix..

nikomatsakis (May 29 2018 at 13:37, on Zulip):

(sorry, race condition)

nikomatsakis (May 29 2018 at 13:37, on Zulip):

it seems like something went wrong with rustc-hash

Santiago Pastorino (May 29 2018 at 13:37, on Zulip):

hehe

Santiago Pastorino (May 29 2018 at 13:37, on Zulip):

no worries

nikomatsakis (May 29 2018 at 13:37, on Zulip):

the way I fixed was: git checkout $(git merge-base rust-lang/master HEAD) Cargo.lock and then rerun x.py build

nikomatsakis (May 29 2018 at 13:37, on Zulip):

which generated that diff :)

Santiago Pastorino (May 29 2018 at 13:38, on Zulip):

:+1:

Vytautas Astrauskas (May 29 2018 at 14:43, on Zulip):

@nikomatsakis I have opened a pull request: https://github.com/rust-lang-nursery/polonius/pull/64.

Vytautas Astrauskas (May 29 2018 at 14:43, on Zulip):

I need to catch my train now, I'll address issues tomorrow morning (if you have any).

Santiago Pastorino (May 29 2018 at 16:52, on Zulip):

@nikomatsakis I guess we need to mark the last checkbox here https://github.com/rust-lang-nursery/polonius/issues/44

nikomatsakis (May 30 2018 at 15:02, on Zulip):

Rustc has DatafrogOpt hard coded. We could change it to something like:

let algorithm: Algorithm = env::var("POLONIUS_ALGORITHM").unwrap_or("DatafrogOpt").into();

However, for that we need a new version of polonius-engine to be released and it probably would make sense to land first all PRs that are currently in flight.

I think we should do this. What is in flight that we ought to wait for?

nikomatsakis (May 30 2018 at 15:02, on Zulip):

(cc @Vytautas Astrauskas and @Santiago Pastorino)

nikomatsakis (May 30 2018 at 15:03, on Zulip):

issuing a new version of polonius is easy enough

Santiago Pastorino (May 30 2018 at 15:06, on Zulip):

I think it needs this https://github.com/rust-lang-nursery/polonius/pull/64 ?

Santiago Pastorino (May 30 2018 at 15:06, on Zulip):

can check better a bit later

lqd (May 30 2018 at 15:06, on Zulip):

or maybe just compare-mode ? otherwise #51133 has landed yesterday (maybe that was the only one we do need)

nikomatsakis (May 30 2018 at 15:08, on Zulip):

I think it needs this https://github.com/rust-lang-nursery/polonius/pull/64 ?

I just merged that :)

Vytautas Astrauskas (May 30 2018 at 15:08, on Zulip):

I think we should do this. What is in flight that we ought to wait for?

I can implement this now.

Vytautas Astrauskas (May 30 2018 at 15:46, on Zulip):

I think it needs this https://github.com/rust-lang-nursery/polonius/pull/64 ?

I just merged that :)

It seems that I did something stupid, which caused this pull request to result in a merge error and non-compiling code.

Vytautas Astrauskas (May 30 2018 at 15:46, on Zulip):

My attempt to fix it: https://github.com/rust-lang-nursery/polonius/pull/66.

Vytautas Astrauskas (May 30 2018 at 15:47, on Zulip):

By the way, don't we have CI enabled for the Polonius repository to prevent such errors?

lqd (May 30 2018 at 15:48, on Zulip):

we do :thinking_face: that is weird

nikomatsakis (May 30 2018 at 15:49, on Zulip):

well, it tests pre-merge I think

nikomatsakis (May 30 2018 at 15:49, on Zulip):

rarely makes a difference

nikomatsakis (May 30 2018 at 15:49, on Zulip):

but sometimes it does!

nikomatsakis (May 30 2018 at 15:50, on Zulip):

I've debated about setting up bors-ng or some such thing

nikomatsakis (May 30 2018 at 15:50, on Zulip):

@Vytautas Astrauskas we no longer need the variants fn?

Santiago Pastorino (May 30 2018 at 15:52, on Zulip):

or maybe just compare-mode ? otherwise #51133 has landed yesterday (maybe that was the only one we do need)

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

Vytautas Astrauskas (May 30 2018 at 15:52, on Zulip):

@nikomatsakis We do need the variantsfn, and we do have one: https://github.com/rust-lang-nursery/polonius/pull/66/files#diff-29826c0b0a8116137a1af9348ecacac9R30

Santiago Pastorino (May 30 2018 at 15:52, on Zulip):

if you're referring to that didn't land yet

Vytautas Astrauskas (May 30 2018 at 15:52, on Zulip):

Currently, in the Polonius master we have two of them.

Vytautas Astrauskas (May 30 2018 at 15:54, on Zulip):

Sorry, for the mess.

nikomatsakis (May 30 2018 at 15:54, on Zulip):

no worries, I should have done my due diligence before merging :)

nikomatsakis (May 30 2018 at 15:54, on Zulip):

I merged #66, @Vytautas Astrauskas

Vytautas Astrauskas (May 30 2018 at 15:57, on Zulip):

Cool, I am testing locally my code that allows choosing Polonius algorithm via env variable against Polonius master.

Vytautas Astrauskas (May 30 2018 at 15:58, on Zulip):

Once I am convinced that it works, I will open a pull request.

Santiago Pastorino (May 30 2018 at 17:07, on Zulip):

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

Santiago Pastorino (May 30 2018 at 17:07, on Zulip):

force pushed again, because there were merge conflicts after something was merged on master

Santiago Pastorino (May 30 2018 at 17:08, on Zulip):

it need to be r+ again, right?

nikomatsakis (May 30 2018 at 17:18, on Zulip):

@Santiago Pastorino errors in https://github.com/rust-lang/rust/pull/51138

Santiago Pastorino (May 30 2018 at 17:28, on Zulip):

will check

Santiago Pastorino (May 30 2018 at 17:29, on Zulip):

unsure how this happened

Santiago Pastorino (May 30 2018 at 17:29, on Zulip):

checking

Santiago Pastorino (May 30 2018 at 17:30, on Zulip):

my changes are lost, I have no idea what's going on with git rebase yesterday something similar happened

Santiago Pastorino (May 30 2018 at 17:31, on Zulip):

anyway fixing it

Santiago Pastorino (May 30 2018 at 17:38, on Zulip):

@nikomatsakis it should be fine now

nikomatsakis (May 31 2018 at 16:12, on Zulip):

@Vytautas Astrauskas I guess that https://github.com/rust-lang/rust/pull/51246 needs a new release of polonius?

Santiago Pastorino (May 31 2018 at 16:31, on Zulip):

@nikomatsakis :+1:

nikomatsakis (May 31 2018 at 16:52, on Zulip):

I'll just make it 0.5.0

nikomatsakis (May 31 2018 at 16:52, on Zulip):

I can't be bothered to tell if this is semver compat or what

nikomatsakis (May 31 2018 at 16:53, on Zulip):

presumably not since we are modifying enums etc

Santiago Pastorino (May 31 2018 at 16:58, on Zulip):

below 1.0 doesn't matter :P

Santiago Pastorino (May 31 2018 at 16:58, on Zulip):

@Vytautas Astrauskas would need to change Cargo.toml and Cargo.lock now from that PR

Vytautas Astrauskas (May 31 2018 at 17:01, on Zulip):

I'll just make it 0.5.0

Thank you!

Vytautas Astrauskas (May 31 2018 at 17:01, on Zulip):

@Vytautas Astrauskas would need to change Cargo.toml and Cargo.lock now from that PR

Updating…

nikomatsakis (May 31 2018 at 17:10, on Zulip):

@Vytautas Astrauskas published

Vytautas Astrauskas (May 31 2018 at 17:11, on Zulip):

@Vytautas Astrauskas published

Thanks! I have updated the pull request. However, I am still compiling to test if it works.

Vytautas Astrauskas (May 31 2018 at 17:11, on Zulip):

Should take around 30 minutes.

Vytautas Astrauskas (May 31 2018 at 17:38, on Zulip):

@nikomatsakis Selecting the algorithm seems to work:

POLONIUS_ALGORITHM=compare RUST_LOG=rustc_mir::borrow_check::nll=trace  ./x.py test --stage 1 --compare-mode polonius -- src/test/ui/nll/issue-47680.rs
...
stderr:
------------------------------------------
 INFO 2018-05-31T17:35:31Z: rustc_mir::borrow_check::nll: Using Polonius algorithm: Compare
 INFO 2018-05-31T17:35:31Z: rustc_mir::borrow_check::nll: Using Polonius algorithm: Compare

------------------------------------------
...
Vytautas Astrauskas (May 31 2018 at 17:38, on Zulip):

However, I still have not found a way how to get logs from polonius_engine.

Vytautas Astrauskas (May 31 2018 at 17:41, on Zulip):

(This command POLONIUS_ALGORITHM=compare RUST_LOG=polonius_engine::output=trace ./x.py test --stage 1 --compare-mode polonius -- src/test/ui/nll/issue-47680.rs does not output anything.)

nikomatsakis (May 31 2018 at 17:42, on Zulip):

(This command POLONIUS_ALGORITHM=compare RUST_LOG=polonius_engine::output=trace ./x.py test --stage 1 --compare-mode polonius -- src/test/ui/nll/issue-47680.rs does not output anything.)

huh, that's confusing

nikomatsakis (May 31 2018 at 17:42, on Zulip):

maybe we should ping sfackler

Vytautas Astrauskas (May 31 2018 at 17:46, on Zulip):

maybe we should ping sfackler

@nikomatsakis Just to be clear: by anything I mean that it does not output any logs from polonius_engine.

lqd (Jun 04 2018 at 11:13, on Zulip):

@Vytautas Astrauskas btw https://github.com/rust-lang/rust/pull/51246 is ready for r+ right ?

Vytautas Astrauskas (Jun 04 2018 at 11:35, on Zulip):

@lqd I have addressed the Niko comment about log levels, so yes.

lqd (Jun 04 2018 at 11:36, on Zulip):

sweet :thumbs_up:

lqd (Jun 04 2018 at 14:43, on Zulip):

(btw if people end up using this PR to compare between the different polonius variants, please note that -a compare compares _errors_ today, not borrow_live_at which we know can differ between Naive and Opt, and that Naive doesn't produce errors today)

nikomatsakis (Jun 04 2018 at 14:45, on Zulip):

I thought we fixed that

nikomatsakis (Jun 04 2018 at 14:45, on Zulip):

that is, fixed Naive to produce errors?

lqd (Jun 04 2018 at 14:46, on Zulip):

IIRC it was both Opt and LocInsensitive

lqd (Jun 04 2018 at 14:46, on Zulip):

but ia0 did have a branch when he worked during the impl days IIRC

lqd (Jun 04 2018 at 14:48, on Zulip):

which was here

lqd (Jun 05 2018 at 15:42, on Zulip):

@Santiago Pastorino what was your x.py test command line for when you generated the list of -Zpolonius failures ? .ie. how does one add the -Zpolonius ? :D

Santiago Pastorino (Jun 05 2018 at 15:44, on Zulip):

what I did at that time was to change the code locally :)

lqd (Jun 05 2018 at 15:45, on Zulip):

haha

Santiago Pastorino (Jun 05 2018 at 15:45, on Zulip):

but now we merged my PR and @Vytautas Astrauskas's one I guess we can do --compare-mode polonius

lqd (Jun 05 2018 at 15:46, on Zulip):

nice, I'll try that then, when my build is finished

Last update: Nov 22 2019 at 00:10UTC