Stream: t-compiler/wg-nll

Topic: Rust-Fest-Impl-Days


Vytautas Astrauskas (May 27 2018 at 06:20, on Zulip):

@lqd Are you staying for Impl Days?

Vytautas Astrauskas (May 27 2018 at 06:23, on Zulip):

(Just thinking that we should make a group of people working on NLL during Impl Days.)

lqd (May 27 2018 at 06:25, on Zulip):

Unfortunately not, leaving tomorrow. I think I remember your name flash on a badge and it seemed familIar d:)

lqd (May 27 2018 at 06:26, on Zulip):

It would have been nice indeed

Vytautas Astrauskas (May 27 2018 at 09:32, on Zulip):

I see. It seems that I managed to get sick, so not sure if I manage to come tomorrow.

Vytautas Astrauskas (May 28 2018 at 08:31, on Zulip):

@nikomatsakis We are 8 beginners at Impl Days keen to work on NLL.

Vytautas Astrauskas (May 28 2018 at 08:34, on Zulip):

Any help would be greatly appreciated. :relaxed:

Vytautas Astrauskas (May 28 2018 at 08:35, on Zulip):

Dropbox paper: https://paper.dropbox.com/doc/Rust-Fest-Impl-Days-NLL-bEyDVSumYzlqpGx51ayY3

emilsp (May 28 2018 at 08:47, on Zulip):

This is one of the unsound cases - https://github.com/rust-lang/rust/issues/51117

pnkfelix (May 28 2018 at 09:50, on Zulip):

@Vytautas Astrauskas its a holiday in the USA today so its at least possible that Niko is spending the day offline with his family

nikomatsakis (May 28 2018 at 10:28, on Zulip):

I'll be around somewhat today.

nikomatsakis (May 28 2018 at 10:29, on Zulip):

@Vytautas Astrauskas what is current state?

Vytautas Astrauskas (May 28 2018 at 10:47, on Zulip):

Currently, everybody is trying to understand what is Polonius, Datalog, etc.

nikomatsakis (May 28 2018 at 10:47, on Zulip):

ok :)

nikomatsakis (May 28 2018 at 10:47, on Zulip):

that's kind of astray from the soundness bugs

nikomatsakis (May 28 2018 at 10:48, on Zulip):

https://github.com/rust-lang/rust/issues/50716 might be an easy-ish one to fix, for example

nikomatsakis (May 28 2018 at 10:48, on Zulip):

I'd be up to walk somebody through that

Vytautas Astrauskas (May 28 2018 at 10:49, on Zulip):

https://github.com/rust-lang/rust/issues/50716 might be an easy-ish one to fix, for example

Trying to understand what it is about.

Frank McSherry (May 28 2018 at 10:52, on Zulip):

I'm online and can help field random datafrog questions. Not super on-point about polonius and what it needs next, though.

nikomatsakis (May 28 2018 at 11:02, on Zulip):

@Vytautas Astrauskas actually that bug is kinda' silly, let me look over the rest first

Vytautas Astrauskas (May 28 2018 at 11:02, on Zulip):

https://github.com/rust-lang/rust/issues/50716 might be an easy-ish one to fix, for example

Trying to understand what it is about.

Ok, I think I understand now where the problem is.

nikomatsakis (May 28 2018 at 11:03, on Zulip):

I guess though that, apart from https://github.com/rust-lang/rust/issues/47184, that is the major open soundness issue. The others seem mostly to have to do with matches, and @pnkfelix has an open PR that (I believe) should fix them

pnkfelix (May 28 2018 at 11:04, on Zulip):

no no, my open PR isn't for this

nikomatsakis (May 28 2018 at 11:04, on Zulip):

@pnkfelix isn't for what?

pnkfelix (May 28 2018 at 11:04, on Zulip):

I am working on a branch that addresses some of this

pnkfelix (May 28 2018 at 11:04, on Zulip):

oh oh oh wait

nikomatsakis (May 28 2018 at 11:04, on Zulip):

you have an open PR for matches

pnkfelix (May 28 2018 at 11:04, on Zulip):

I misread your message

pnkfelix (May 28 2018 at 11:04, on Zulip):

right sorry

pnkfelix (May 28 2018 at 11:05, on Zulip):

okay yes, I do have an open PR for matches that hopefully resolves much of our issues there

nikomatsakis (May 28 2018 at 11:05, on Zulip):

ok

Vytautas Astrauskas (May 28 2018 at 11:05, on Zulip):

@Vytautas Astrauskas actually that bug is kinda' silly, let me look over the rest first

OK. By the way, are Polonius issues 12, 19, and 20 available to work right now?

pnkfelix (May 28 2018 at 11:06, on Zulip):

and I am independently working on a branch that addresses one part of rust-lang/rust#47184 ((namely, deconstructive bindings like let (a, b): (T, U);))

nikomatsakis (May 28 2018 at 11:06, on Zulip):

@Vytautas Astrauskas in terms of polonius, I feel like rustc-integration is the most imp't thing, but @qmx and @Santiago Pastorino have that under control; after that, https://github.com/rust-lang-nursery/borrow-check/issues/20 may be the most interesting issue. It's maybe semi-blocked by #24 but not necessarily

Vytautas Astrauskas (May 28 2018 at 11:06, on Zulip):

I think these issues are the ones that I could help others to work on.

pnkfelix (May 28 2018 at 11:06, on Zulip):

but I haven't thought terribly hard about how to further generalize to e.g. foo::<'static>(argh)

nikomatsakis (May 28 2018 at 11:07, on Zulip):

@Vytautas Astrauskas I'm not aware of anyone working on those. That said, I don't know that #19 would be particularly helpful. I think that the DatafrogOpt variant probably solves that problem fairly effectively.

nikomatsakis (May 28 2018 at 11:08, on Zulip):

I should probably close that

Vytautas Astrauskas (May 28 2018 at 11:11, on Zulip):

OK, so we have four issues we could work on.

Julien Cretin (ia0) (May 28 2018 at 11:17, on Zulip):

I'm starting to take a look at https://github.com/rust-lang-nursery/polonius/issues/20

Vytautas Astrauskas (May 28 2018 at 11:18, on Zulip):

Personally, I would be up to work on either #47184 or #50716. However, I would definitely need a walk through.

nikomatsakis (May 28 2018 at 11:22, on Zulip):

@Vytautas Astrauskas well let's fix #50716, or .. at least partially fix it .. (there's some subtleties I think concerning location). But it seems like something you can get a PR up for today whereas the remaining parts of #47184 would be quite hard)

Vytautas Astrauskas (May 28 2018 at 11:30, on Zulip):

@nikomatsakis

@Vytautas Astrauskas well let's fix #50716, or .. at least partially fix it .. (there's some subtleties I think concerning location). But it seems like something you can get a PR up for today whereas the remaining parts of #47184 would be quite hard)

Sounds good.

nikomatsakis (May 28 2018 at 11:31, on Zulip):

ok @Vytautas Astrauskas — I'm writing up some brief notes on #50716

Vytautas Astrauskas (May 28 2018 at 11:32, on Zulip):

OK.

Andrea Lattuada (May 28 2018 at 11:32, on Zulip):

Heyo! Me and @Yati Sagade are taking a look at better debug output with GraphViz.
May have qs for @Frank McSherry

Jaime Valdemoros (May 28 2018 at 11:32, on Zulip):

I'm going to have a look at https://github.com/rust-lang/rust/issues/48417, although I don't think I should self-assign until I think I've made some headway

nikomatsakis (May 28 2018 at 11:34, on Zulip):

@Vytautas Astrauskas here is the comment

nikomatsakis (May 28 2018 at 11:35, on Zulip):

@Jaime Valdemoros sounds fine, you can always leave a comment, but I don't know that you have any competition. I'll try to be around to answer any questions...

Vytautas Astrauskas (May 28 2018 at 11:37, on Zulip):

@Vytautas Astrauskas here is the comment

Thanks, reading it.

Jaime Valdemoros (May 28 2018 at 14:04, on Zulip):

Hi @nikomatsakis, @Christian and I have been looking at whether we could spot any easy speed-ups in the use of datafrog. We're noticed that on the 'bad input' in the repo we have no output from datafrogopts compared to naive - do you know if that's expected?

nikomatsakis (May 28 2018 at 14:08, on Zulip):

I don't (fully) understand the question

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

but in general naive and datafrogopt should produce precisely the same final result

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

and can be compared against one another

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

that is indeed the whole point of keeping naive around :)

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

I think you are saying that we are not really testing as thoroughly as we could be? In which case I'm sure that's true and would welcome PRs to do better :)

Jaime Valdemoros (May 28 2018 at 14:28, on Zulip):

No, I don't think I'm saying you should do anything differently :) I think I was just messing up the flags - with -v both print the same thing, but without -v (which is what I was doing) the naive one was printing out a load of results and datafrogopts wasn't which made me think it was coming to a different conclusion to datafrogopts.

nikomatsakis (May 28 2018 at 14:35, on Zulip):

@Jaime Valdemoros oh, hmm, well — that is probably a sorta bug too :)

nikomatsakis (May 28 2018 at 14:35, on Zulip):

I think that (without -v) both should just print the errors and that's it

nikomatsakis (May 28 2018 at 14:35, on Zulip):

I'm not sure why it is acting differently actually...

nikomatsakis (May 28 2018 at 14:35, on Zulip):

probably something simple though

Jaime Valdemoros (May 28 2018 at 14:37, on Zulip):

Okay, thanks, that's what we were wondering :simple_smile: might be a good thing for us to possibly contribute with

nikomatsakis (May 28 2018 at 14:37, on Zulip):

yeah, seems like a simple place to start and get familiar:)

nikomatsakis (May 28 2018 at 14:40, on Zulip):

(if anybody is still looking for things to do, we could merge https://github.com/rust-lang-nursery/polonius/pull/48 then I think there is follow-up work to do there)

lqd (May 29 2018 at 07:42, on Zulip):

@nikomatsakis here are a couple things I thought could be interesting for the people at impl days, what do you think ?
- implement def into polonius-parser, and produce the correct region_live_at with liveness, as described in the 2 github PRs
- add crude statistics to datafrog, behind a feature, to both have the number of tuples, and the time it took to merge and produce all of them, per Relation. There are IIRC examples of this inside datafrog itself rn (look for commented out Drop impls)
- remove the subset symmetries, a lot of tuples in clap are the same region on both sides of the relation
- if anybody wants to land leapfrog, note that the gists we showed still produce borrow_live_at while the repo ones are more looking to produce errors; while we could lean leapfrog producing borrows and joining to invalidates as right now; however in this direction, borrow_live_at is a temporary that does not need to produce tuples (except in -v) + Invalidates (in my mind) will cut into the relation quite a bit since those should be a nice subset of all points. I have already done this work but was waiting for FLTJ to land in master, without more tests I'm not sure on a specific API point, but I can find the gist when I get to the office if need be
- try to see if we need to share more of the location-insensitive and -sensitive work, by filtering the latter's work with the former's output
- find out what is slow in compiling webrender and cargo when compiled with NLL, as shown on perf.rlo — use the code versions in the rustc-perf repo; extract benchmark use cases for polonius out of those slow cases, if needed (or maybe rustc issues)

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

@lqd these all seem good, whether at impl days or not. We should open corresponding issues on the polonius repository

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

(would you be up for doing that?)

nikomatsakis (May 29 2018 at 08:17, on Zulip):

I think outside of polonius, we should be focusing probably on diagnostics — I am kind of unclear on the status there, I know that @pnkfelix did a bunch of awesome work categorizing things, but I'm not clear on how many are fixed (it looked like there was good progress by @Matthew Jasper on at least some things)

lqd (May 29 2018 at 08:18, on Zulip):

(sure I can create issues at lunch break, which is in a couple hours or so)

lqd (May 29 2018 at 08:31, on Zulip):

another datafrog thing people might find interesting: implement multi variable TFLJ, to see if removing even more of the temporary Variables has a an impact; more info on how to do that from Frank here

nikomatsakis (May 29 2018 at 08:43, on Zulip):

@pnkfelix you know what's annoying? that -Znll doesn't mean the same thing as #![feature(nll)] (right?).

nikomatsakis (May 29 2018 at 08:43, on Zulip):

I wish that we had -Zdisable-two-phase-borrows instead

qmx (May 29 2018 at 10:41, on Zulip):

isn't this annoyance worth to be raised as an issue?

pnkfelix (May 29 2018 at 10:41, on Zulip):

we certainly can make that change, especially at this point

pnkfelix (May 29 2018 at 10:42, on Zulip):

though we don't even have -Z nll anymore

pnkfelix (May 29 2018 at 10:43, on Zulip):

(-Z borrowck=mir certainly could imply -Z two-phase-borrows, which is probably what niko meant. Its a little goofy, but at this point -Z borrowck=mir implies a lot of things beyond the switch the mir-borrowck...)

nikomatsakis (May 29 2018 at 10:52, on Zulip):

I don't know what I meant:) mostly that I want something as easy as #![feature(nll)] — whether it be -Znll or -Zborrowck=mir I don't care :)

pnkfelix (May 29 2018 at 11:00, on Zulip):

in some ways the easiest thing would be to re-add -Z nll and just have it imply the same set of flags that #![feature(nll)] does

pnkfelix (May 29 2018 at 11:00, on Zulip):

Though perhaps some would find that confusing...

pnkfelix (May 29 2018 at 11:01, on Zulip):

(by "easiest" I sort of mean for end-users. It probably isn't the easiest to implement, though I suspect it wouldn't be that hard either...)

lqd (May 29 2018 at 11:35, on Zulip):

I created https://github.com/rust-lang-nursery/polonius/issues/61 https://github.com/rust-lang-nursery/polonius/issues/62 and https://github.com/rust-lang-nursery/datafrog/issues/6 to track some of the ideas mentioned this morning (sharing work between location-insensitive and sensitive analyses is already in an issue somewhere IIRC, and investigation webrender/cargo NLL slowness didn't feel like it should be in the polonius repo)

lqd (May 29 2018 at 12:29, on Zulip):

ah and https://github.com/rust-lang-nursery/datafrog/issues/5 as well

Christian (May 29 2018 at 12:45, on Zulip):

Double checking, nobody is working on speed improvements in polonius/datafrog right now? Me and @solidninja intend to work on this for today.

lqd (May 29 2018 at 12:49, on Zulip):

Reed had made some tests here https://rust-lang.zulipchat.com/#narrow/stream/122657-wg-nll/subject/optimizing-transitive-closure/near/127141172 recently

Christian (May 29 2018 at 12:55, on Zulip):

On the topic of datafrog / polonius speed improvements: I'd want proper tests that check that all algorithms return the correct results for the test inputs (can't find any doing this in the current code base?), and also listing the timings for these so it will be easy to compare various versions of the code. Has anyone worked on this or should I do this now?

lqd (May 29 2018 at 12:58, on Zulip):

there are 2-3 tests, including comparing the Naive vs Opt variant on one of the datasets, the rest is at the moment done manually comparing the expected outputs from the in-repo datasets; until we have more tests with the frontend, and finalized rustc integration

nikomatsakis (May 29 2018 at 12:58, on Zulip):

we have some tests like that with cargo test, but it is true that we need more. The plan was to land the rustc-integration work (currently in bors queue), find places where we are failing soundness tests, extract the facts, and make tests from those (presuming the flaw is in polonius, perhaps, as opposed to rustc). We don't have to necessarily wait though for rustc-integration to land before beginning that process -- you can build it locally from @Santiago Pastorino's branch

Reed Koser (May 29 2018 at 15:25, on Zulip):

On the topic of datafrog / polonius speed improvements: I'd want proper tests that check that all algorithms return the correct results for the test inputs (can't find any doing this in the current code base?), and also listing the timings for these so it will be easy to compare various versions of the code. Has anyone worked on this or should I do this now?

I've taken some stabs at doing optimizations within Datafrog. I think there's much larger wins to be hard by doing algorithmic optimizations on Polonius, but didn't get a chance to sit down and think about it this weekend

Last update: Nov 21 2019 at 13:05UTC