Stream: t-compiler/wg-nll

Topic: polonius


nikomatsakis (Sep 25 2018 at 20:09, on Zulip):

@Matthew Jasper you saw my comments on your PR?

Matthew Jasper (Sep 25 2018 at 20:10, on Zulip):

Yes, I'm trying to track down something so for your first comment

lqd (Sep 27 2018 at 20:02, on Zulip):

I was wondering whether polonius needed the outlives facts where regions outlive themselves (one of the contributing factors to regions being computed as subsets of themselves) ? (there aren't many of these regions outliving themselves, for example: 456 out of the 534 327 outlives facts in clap-rs)

nikomatsakis (Sep 27 2018 at 20:28, on Zulip):

I don't think so

lqd (Sep 27 2018 at 20:29, on Zulip):

they probable have a need in rustc though

nikomatsakis (Sep 27 2018 at 20:29, on Zulip):

no

nikomatsakis (Sep 27 2018 at 20:29, on Zulip):

they just get stripped out

lqd (Sep 27 2018 at 20:29, on Zulip):

oh really

lqd (Sep 27 2018 at 20:30, on Zulip):

interesting, thank you (I think I remember it from back in May when I looked at this)

lqd (Sep 27 2018 at 20:52, on Zulip):

btw did the recent PR switch all invalidates facts to mid points ? (IIRC most of them were emitted on mid-points, but there were also some on start-points) ? (if so, maybe the PR allowing the parser/program to emit effects on start-points might not needed anymore?)

Matthew Jasper (Sep 27 2018 at 20:54, on Zulip):

#54468 moves them to the start

nikomatsakis (Sep 27 2018 at 21:00, on Zulip):

ok, I'm gonna start bringing polonius back in cache now :)

nikomatsakis (Sep 27 2018 at 21:00, on Zulip):

seems like we can merge https://github.com/rust-lang-nursery/polonius/pull/70/, to start (cc @lqd) =)

nikomatsakis (Sep 27 2018 at 21:01, on Zulip):

@lqd didn't we have some known bug that you minimized...?

nikomatsakis (Sep 27 2018 at 21:02, on Zulip):

I guess that https://github.com/rust-lang-nursery/polonius/issues/44 is basically done

lqd (Sep 27 2018 at 21:02, on Zulip):

@Matthew Jasper oh sorry I completely misremembered that from your PR, and thought it was literally the opposite

lqd (Sep 27 2018 at 21:03, on Zulip):

@nikomatsakis the 3 tests in test.rs are the known bug datafrogopt indeed, they are marked as #[should_panic] until we do fix it :)

lqd (Sep 27 2018 at 21:04, on Zulip):

I would like to fix correctly the previous PR hack, and I feel it's going to take a bit of API in datafrog, something like filter_map

nikomatsakis (Sep 27 2018 at 21:05, on Zulip):

yeah...

lqd (Sep 27 2018 at 21:06, on Zulip):

(polonius #70 is indeed just switching these tests from being manual facts to using the parser — tests were one of its original purposes anyways; but they're slightly less minimized this way)

lqd (Sep 27 2018 at 21:07, on Zulip):

btw the biggest win we'd have is from starting using the "treefrog leapjoin" work in https://github.com/rust-lang-nursery/datafrog/pull/11

nikomatsakis (Sep 27 2018 at 21:07, on Zulip):

oh man

nikomatsakis (Sep 27 2018 at 21:08, on Zulip):

I didn't even realize datafrog had pending PRs :)

nikomatsakis (Sep 27 2018 at 21:08, on Zulip):

/me grumbles about GH's workflow

lqd (Sep 27 2018 at 21:08, on Zulip):

frank has merged some already

lqd (Sep 27 2018 at 21:09, on Zulip):

(and I want to close/redo the specialization one)

nikomatsakis (Sep 27 2018 at 21:10, on Zulip):

I was just skimming

nikomatsakis (Sep 27 2018 at 21:10, on Zulip):

I am :+1: on sticking to stable if at all possible

lqd (Sep 27 2018 at 21:10, on Zulip):

I'm trying to comment and cleanup the cfg compression work rn, I'm sure there will be lots of things to improve once I open the PR

lqd (Sep 27 2018 at 21:12, on Zulip):

yeah sticking to stable will just be using the existing :frog: fns, not hard at all

lqd (Sep 27 2018 at 21:12, on Zulip):

I'm sure datafrog could use a bit of API polish as well, sticking to existing conventions, function names, etc

lqd (Sep 27 2018 at 21:14, on Zulip):

(and I feel there's a case with leapfrog where I expected it to work and it didn't, probable no big deal or me being stupid, but would help remove some temporary indexes/variables, I'll minimize and open an issue)

nikomatsakis (Sep 27 2018 at 21:20, on Zulip):

one thing that might be nice:

nikomatsakis (Sep 27 2018 at 21:20, on Zulip):

settling on our terminology a bit

nikomatsakis (Sep 27 2018 at 21:20, on Zulip):

e.g., I'm coming to prefer "loans" over "Borrows", but we're awfully inconsistent

lqd (Sep 27 2018 at 21:25, on Zulip):

from the early beginning I've had in my todo list: switch all instances of borrows/B to Loans/L :)

nikomatsakis (Sep 27 2018 at 21:28, on Zulip):

hehe

nikomatsakis (Sep 27 2018 at 21:28, on Zulip):

I'd like to match up the rustc code too

nikomatsakis (Sep 27 2018 at 21:28, on Zulip):

but I guess we could start by picking the vocabulary we want in polonius

nikomatsakis (Sep 27 2018 at 21:28, on Zulip):

there are some other examples, e.g., I think polonius talks about outlives...

nikomatsakis (Sep 27 2018 at 21:28, on Zulip):

though that is still a handy term

nikomatsakis (Sep 27 2018 at 21:29, on Zulip):

something like "base_subset" feels better tho

lqd (Sep 27 2018 at 21:32, on Zulip):

and base_subset was how they were introduced in the blog post IIRC

nikomatsakis (Sep 27 2018 at 21:36, on Zulip):

yeah I think I made an effort in the blog post to cleanup the terminology

nikomatsakis (Sep 27 2018 at 21:36, on Zulip):

but left the code alone :P

lqd (Sep 27 2018 at 22:32, on Zulip):

:tada: #54468 landed

lqd (Oct 02 2018 at 12:28, on Zulip):

@pnkfelix quick question about Polonius: if 1) one had to mutate the input facts, and then 2) change the computation result based on step 1, where would you expect the code for step 2 to be: a) in the computation themselves (but that would require to change the APIs to take this new input) or b) in the "result" struct itself ? (option A is easy but there is no Context struct or whatever making adding inputs a bit less clean atm;, option B is a bit harder to do efficiently because needing mutation of a "complex" data structure while iterating it)

pnkfelix (Oct 02 2018 at 12:30, on Zulip):

@lqd I don't have enough info in my mental cache to answer that question quickly.

pnkfelix (Oct 02 2018 at 12:31, on Zulip):

@lqd but maybe I can fake it. Can you provide more info on what kind of mutation of the input facts you are doing?

lqd (Oct 02 2018 at 12:31, on Zulip):

@pnkfelix no problem, I'll just do my best and then we can iterate on the PR

pnkfelix (Oct 02 2018 at 12:31, on Zulip):

My intuition is that if you want a Context struct and one is missing, then maybe we should add it

lqd (Oct 02 2018 at 12:31, on Zulip):

I'm compressing the facts, to do the analysis over less data, and then "decompressing" the live borrows

pnkfelix (Oct 02 2018 at 12:31, on Zulip):

because chances are we'll end up adding one eventually anyway

pnkfelix (Oct 02 2018 at 12:32, on Zulip):

Hmm. Interesting.

lqd (Oct 02 2018 at 12:32, on Zulip):

we have a kind of input context (all facts), but then each computation fills results differently, while compression is a crosscutting concern

lqd (Oct 02 2018 at 12:34, on Zulip):

so the whole thing is a mix of 1) adding input data, 2) generating output data, 3) no wanting to duplicate either compression or decompression code in each variant, 4) wanting for now it to be a bit external and toggleable for testing and benchmarking purposes, etc

lqd (Oct 02 2018 at 12:35, on Zulip):

it would probably be easier to iterate on the PR rather than everyone having to use their mind's eye to imagine the different options :)

lqd (Oct 04 2018 at 21:01, on Zulip):

@nikomatsakis btw how would you expect compression to be integrated into Polonius and rustc, toggled, and tested ? new variants ? modifying the existing variants fn signature (to pass possible compression data) ?

lqd (Oct 05 2018 at 08:47, on Zulip):

(right now I'm locally making a temporary (it's not in the PR) variant in order to be able to test from rustc)

lqd (Oct 05 2018 at 11:46, on Zulip):

(and this temporary compressed-variant seemingly behaves like the default opt variant in rustc tests (both have the same 37 test failures, and which I believe are the failures @Matthew Jasper saw in their PR, some of them due to ignore-compare-mode-nll and other to differences/lack of diagnostics), and also like the naive variant, if I modify it to compute errors instead of just live loans)

lqd (Oct 05 2018 at 13:28, on Zulip):

(but to remove the biggish joins to actually compute errors instead of live loans, we'll need the treefrog PR merged in :frog: :)

nikomatsakis (Oct 05 2018 at 15:24, on Zulip):

@lqd I don't know, I have to review your PR, I have mixed feelings about doing too much pre-processing etc when it comes to polonius.

lqd (Oct 05 2018 at 15:26, on Zulip):

in a sense it is getting closer to a representation focusing on liveness, IIRC like arielb mentioned a few times before

nikomatsakis (Dec 04 2018 at 11:51, on Zulip):

BTW, in my spare time I've been working on polonius.

I fixed a bug in how we handle two-phase-borrows and fact generation and am currently going through the missing test cases.

Kind of tight for time so no time for a bigger update, just though I'd leave a note.

nikomatsakis (Dec 04 2018 at 13:00, on Zulip):

It seems like, apart from the region bugs I described here, a compare-mode=polonius run passes all tests now -- but some of them get more errors than they used to (duplicate-ish error reports).

nikomatsakis (Dec 04 2018 at 13:01, on Zulip):

I'll open a PR with the various bug fixes though

lqd (Dec 20 2018 at 12:31, on Zulip):

uh-oh

lqd (Dec 20 2018 at 12:35, on Zulip):

so I was trying to implement the illegal subset relations errors

lqd (Dec 20 2018 at 12:36, on Zulip):

I think we're going to hit a tiny snag with :frog:'s way of ensuring stratification

lqd (Dec 20 2018 at 12:37, on Zulip):

that is, the subset errors are to be computed by checking a dynamic Variable (albeit "trivially stratifiable" in theory), while datafrog will in practice only allow antijoins with a static Relation

lqd (Dec 20 2018 at 12:40, on Zulip):

@nikomatsakis my WIP branch is here and unless I'm missing something obvious I'm not sure I can compute the subset errors now ?

lqd (Dec 20 2018 at 12:50, on Zulip):

when we recently talked about a couple features and APIs datafrog could offer, we also mentioned leapjoins over Variables being nice-to-have-someday. Until now I think we would have only used that for optimization purposes, but now may become a requirement to compute these kinds of errors.

nikomatsakis (Dec 20 2018 at 15:41, on Zulip):

that is, the subset errors are to be computed by checking a dynamic Variable (albeit "trivially stratifiable" in theory), while datafrog will in practice only allow antijoins with a static Relation

I mean I think the idea would be to make a Relation by calling complete

nikomatsakis (Dec 20 2018 at 15:41, on Zulip):

however, I plan to change the rules from that post :)

nikomatsakis (Dec 20 2018 at 15:41, on Zulip):

though they will be similar

nikomatsakis (Dec 20 2018 at 15:41, on Zulip):

in case you are implementing them :)

nikomatsakis (Dec 20 2018 at 15:41, on Zulip):

I'll take a look at the branch

lqd (Dec 20 2018 at 15:47, on Zulip):

it's true that we could compute the known_subset TC before doing the analysis

lqd (Dec 20 2018 at 15:47, on Zulip):

I was indeed implementing them

lqd (Dec 20 2018 at 15:49, on Zulip):

I also used to compute the CFG compression table in another pass like this, so it turns out that I did in fact miss something obvious :face_palm:

lqd (Dec 20 2018 at 15:55, on Zulip):

I was later a bit stuck trying to come up with facts for a text program, I think mostly for the liveness ones -- and using tiny rust examples and -Znll-facts only slightly helped (the "outlives hack" creating facts I didn't yet separate into needed/unneeded, to later encode in a test program)

lqd (Dec 20 2018 at 16:04, on Zulip):

(and also it seems the 1) debug_with to use unterned atoms 2) unit testing facts 3) test code deduplication, 4) tidying up the output struct 5) having a way to test inner parts of the computation (the "verbose" data) -- and so on -- ideas are looking more and more interesting by ~the compute~ the minute)

nikomatsakis (Dec 20 2018 at 16:05, on Zulip):

I should maybe make a DebugWith crate -- the impl I was using in Lark is working out "ok", but relies on specialization

lqd (Dec 20 2018 at 16:08, on Zulip):

could be interesting, or someone could take that off your plate :)

lqd (Dec 20 2018 at 16:11, on Zulip):

(also 6) renaming everything, eg borrow to loan, universal/free regions to placeholder (IIUC) etc)

lqd (Dec 20 2018 at 17:28, on Zulip):

however, I plan to change the rules from that post :)

with any luck we'll also be able to use leapjoins for the new ones :) (the current subset error rule is "currently not well-formed" for leapjoins)

lqd (Dec 20 2018 at 21:47, on Zulip):

maybe they will be location insensitive, rn I feel we'd have errors at "each point" ? (or, say, more than once per missing lifetime relation; I guess points would be used for diagnostics so some will be useful :)

nikomatsakis (Dec 26 2018 at 16:30, on Zulip):

@lqd see also https://github.com/rust-lang-nursery/datafrog/pull/22 :)

lqd (Dec 26 2018 at 16:47, on Zulip):

oh wow :) I'll take a lot in more detail when I'm back in front of a computer but it's looking nice. (Modifying Relation::from to not take an iter + collect into a vec was something I did before as well — but using specialization and before leapjoins, and I wanted to revisit it since closing that PR and leapjoins had been merged, so, yay)

lqd (Dec 27 2018 at 12:05, on Zulip):

the ergonomic changes are looking super nice :thumbs_up:

nikomatsakis (Dec 27 2018 at 12:07, on Zulip):

@lqd I really like this idea of the "tuple per round" dump you were adding -- I'm wondering if we can modify the output to make it more easily analyzed, though. For example, maybe we want to dump into a csv files in the file system, even? Then you could enable it with something like POLONIUS_CSV and analyze rustc

lqd (Dec 27 2018 at 12:09, on Zulip):

indeed it would be cool, I was planning on looking into it (thanks for the review comments btw) — have you seen soufflé's profiler btw ? a CSV seems a good idea so I'll look into that.

lqd (Dec 27 2018 at 12:10, on Zulip):

(and fix the things the other comments mention — esp I was wondering about the feature flag, whether it's partially useful, or not at all)

lqd (Dec 27 2018 at 12:13, on Zulip):

@nikomatsakis also I was wondering here if the new rules you mentioned for the "invalid subset errors" were related to "location sensitivity" (that is, if I could rework the rules to remove the Points or if you had other ideas in mind)

nikomatsakis (Dec 27 2018 at 12:16, on Zulip):

(and fix the things the other comments mention — esp I was wondering about the feature flag, whether it's partially useful, or not at all)

if possible, I would prefer if we can just ensure it has negligible runtime cost when not enabled, vs a feature flag

nikomatsakis (Dec 27 2018 at 12:16, on Zulip):

it's always nice when you can just use stuff from nightly builds etc

nikomatsakis (Dec 27 2018 at 12:19, on Zulip):

@nikomatsakis also I was wondering here if the new rules you mentioned for the "invalid subset errors" were related to "location sensitivity" (that is, if I could rework the rules to remove the Points or if you had other ideas in mind)

Ah, no, not really. I might try to finish writing up the modifications I had in mind today -- for the case I described, the rules are more-or-less equivalent, but when you get to higher-ranked relations it is helpful to formulate things a bit differently.

In particular the idea is to introduce a "placeholder loan" for each placeholder region. So instead of

placeholder_region(R)

we have

placeholder_region(R, L)

and then we can compute the requires relation more or less as normal (though I think we should rename that to contains, element, or member; it is basically the equivalent of L ∈ R), but instead detect errors when we have a loan member we shouldn't have.

nikomatsakis (Dec 27 2018 at 12:21, on Zulip):

The reason this is useful is that sometimes you might have a r1 <= r2 relation where r1 is (say) in U3 and r2 in U1. This is basically equivalent to a normal <= relation except that the placeholder loans for U2 and U3 cannot appear in r2 -- so if such a loan appears in r1 but we can't "promote" them to some other loan that is nameable from U1, it's an error.

nikomatsakis (Dec 27 2018 at 12:21, on Zulip):

That said, I think there is no need for "additional location sensitivity", do you?

nikomatsakis (Dec 27 2018 at 12:22, on Zulip):

That is, the way I setup the rules, I think it was an error if -- at any point -- there was an invalid subset relation between two placeholder regions, but it was still using the subset relations found at some point (versus the location insensitive rules, which combine all the subsets from all points)

nikomatsakis (Dec 27 2018 at 12:22, on Zulip):

or are you just saying it would be nice to have a location insensitive variant to use for "screening" purposes?

lqd (Dec 27 2018 at 12:22, on Zulip):

oh interesting — understood, it will important that I add more example of lifetime relations errors

lqd (Dec 27 2018 at 12:25, on Zulip):

That said, I think there is no need for "additional location sensitivity", do you?

I did not per se but wasn't fully aware of the diagnostics requirements we might have in rustc — I was wondering about the opposite, less location sensitivity.
yeah "screening purposes" was more like it, not sure if it'd buy anything like the invalid accesses location insensitivity buy us (albeit our hand was forced) or just less points indicating the "same subset error" for diagnostics

nikomatsakis (Dec 27 2018 at 12:26, on Zulip):

I see, well, I hadn't really thought much about diagnostics, I admit

nikomatsakis (Dec 27 2018 at 12:26, on Zulip):

I think it might be useful to remember a location where the error occurred (which we currently discard)

nikomatsakis (Dec 27 2018 at 12:26, on Zulip):

however

nikomatsakis (Dec 27 2018 at 12:26, on Zulip):

the current diagnostics do not have or use that information

nikomatsakis (Dec 27 2018 at 12:26, on Zulip):

we only track the invalid subset relation

nikomatsakis (Dec 27 2018 at 12:27, on Zulip):

that is, given some r1 <= r2 that is not known to be true, we then analyze the subset relations directly

nikomatsakis (Dec 27 2018 at 12:27, on Zulip):

to figure out the output

nikomatsakis (Dec 27 2018 at 12:27, on Zulip):

so actually I think the code should mostly "just work"

nikomatsakis (Dec 27 2018 at 12:27, on Zulip):

at some point once all this stuff settles down, I would like to revisit our diganostic code,

nikomatsakis (Dec 27 2018 at 12:27, on Zulip):

I feel like in principle we should be able to use the same horn-clauses that drive the analysis to also figure out our diagnostics

nikomatsakis (Dec 27 2018 at 12:28, on Zulip):

but we'd need to either do causal analysis or enable a top-down evaluation

nikomatsakis (Dec 27 2018 at 12:28, on Zulip):

(perhaps of the naive rules)

nikomatsakis (Dec 27 2018 at 12:28, on Zulip):

it'd be nice if we could simplify down to a core set of rules and make things more DRY, particularly if we are able to move more things into those rules...

nikomatsakis (Dec 27 2018 at 12:29, on Zulip):

@lqd shall I merge https://github.com/rust-lang-nursery/datafrog/pull/22 then, you think?

lqd (Dec 27 2018 at 12:29, on Zulip):

agreed

nikomatsakis (Dec 27 2018 at 12:29, on Zulip):

afaik it is correct :P

lqd (Dec 27 2018 at 12:29, on Zulip):

yeah I think so :)

nikomatsakis (Dec 27 2018 at 12:30, on Zulip):

although I should probably build a modified version of rustc since the polonius self tests don't really cover much

nikomatsakis (Dec 27 2018 at 12:30, on Zulip):

speaking of which, did my polonius branch land yet...

nikomatsakis (Dec 27 2018 at 12:30, on Zulip):

argh, no

nikomatsakis (Dec 27 2018 at 12:30, on Zulip):

wtf

lqd (Dec 27 2018 at 12:32, on Zulip):

true — I'll try to improve the polonius self tests over time, building a custom rustc for changes in it or datafrog to make sure they're correct is a bit unfortunate rn

lqd (Dec 27 2018 at 12:32, on Zulip):

dang cargo.lock conflicts

nikomatsakis (Dec 27 2018 at 12:32, on Zulip):

yeah -- we could perhaps use proptest

nikomatsakis (Dec 27 2018 at 12:33, on Zulip):

it's hard to tell though how much coverage that's getting

nikomatsakis (Dec 27 2018 at 12:33, on Zulip):

probably better to just move more representative tests over

nikomatsakis (Dec 27 2018 at 12:33, on Zulip):

I forget if I did that or not? I think I did some patterns

lqd (Dec 27 2018 at 12:33, on Zulip):

yeah, you added some tricky ones recently

nikomatsakis (Dec 27 2018 at 12:33, on Zulip):

I'd like to make a few more datafrog tests, the proptest setup seemed to work ok

nikomatsakis (Dec 27 2018 at 12:34, on Zulip):

but anyway

nikomatsakis (Dec 27 2018 at 12:34, on Zulip):

e.g. a test of leapjoin's with more than one combinator etc

lqd (Dec 27 2018 at 12:34, on Zulip):

it seems tractable for datafrog, maybe a bit less so for polonius but would be nice/important

lqd (Dec 27 2018 at 12:36, on Zulip):

(that is, the piece of code to generate a random set of valid facts seemed not immediately obvious but maybe it's not that hard)

nikomatsakis (Dec 27 2018 at 12:42, on Zulip):

it seems tractable for datafrog, maybe a bit less so for polonius but would be nice/important

yes -- I mean we can generate random facts, but it's not clear that those facts are "realistic" in any particularly useful way

lqd (Dec 27 2018 at 12:44, on Zulip):

right

nikomatsakis (Dec 27 2018 at 15:03, on Zulip):

hmm @lqd so I did that run with the new datafrog -- interestingly, it came up with fewer "compare-mode failures" than before. There used to be 22, now there were 11. But each of those 11 were previously present. (I went over all the failures at some point and found that they were all legit) I'm not sure why it's behaving differently, might be owing to the rebase.

nikomatsakis (Dec 27 2018 at 15:04, on Zulip):

I'm now re-running the rebased version of my branch but without the datafrog 2.0 changes

nikomatsakis (Dec 27 2018 at 15:04, on Zulip):

it was mostly stuff like we used to report duplicate errors with polonius enabled but don't know

nikomatsakis (Dec 27 2018 at 15:04, on Zulip):

doesn't seem bad but I don't know the cause

lqd (Dec 27 2018 at 15:04, on Zulip):

@nikomatsakis for the Option<Tuple> I was thinking of something like

fn from_filterable_join<K: Ord, V1: Ord, V2: Ord>(
        &self,
        input1: &Variable<(K, V1)>,
        input2: &Variable<(K, V2)>,
        logic: impl FnMut(&K, &V1, &V2) -> Option<Tuple>,
    ) {
        join::filterable_join_into(input1, input2, self, logic)
}

where filterable_join_into would only add the tuple to the results, if logic would return a Some.

in my mind it would allow to filter the cases where we would see symmetries in Polonius but I could be very mistaken :/

nikomatsakis (Dec 27 2018 at 15:04, on Zulip):

ah, yeah, I wondered about that

nikomatsakis (Dec 27 2018 at 15:05, on Zulip):

that would be less efficient than the PrefixFilter

nikomatsakis (Dec 27 2018 at 15:05, on Zulip):

but roughly equivalent to the ValueFilter, I think

nikomatsakis (Dec 27 2018 at 15:05, on Zulip):

still, I think it's not worth it because it seems to impose a kind of ergonomic hit on all other uses

lqd (Dec 27 2018 at 15:06, on Zulip):

in any case it was mostly a thought about these symmetries and the other PRs (while we were talking about API changes), nothing to prevent the PR from landing or anything

nikomatsakis (Dec 27 2018 at 15:06, on Zulip):

(i.e., we have to add Some everywhere)

nikomatsakis (Dec 27 2018 at 15:06, on Zulip):

it does feel though like there is some kind of "gap" opening up between the leapjoin and join stuff

lqd (Dec 27 2018 at 15:07, on Zulip):

was the run with your new unlanded Polonius fixes btw ?

nikomatsakis (Dec 27 2018 at 15:07, on Zulip):

yes

lqd (Dec 27 2018 at 15:07, on Zulip):

reason I ask is:

nikomatsakis (Dec 27 2018 at 15:07, on Zulip):

it does feel though like there is some kind of "gap" opening up between the leapjoin and join stuff

makes me wonder if there is some unified API waiting to be discovered

lqd (Dec 27 2018 at 15:08, on Zulip):

IIRC it was using polonius engine 0.6, and we fixed the Naive variant producing errors in 0.6.1 so I wondered if compare-mode wouldn't produce more failures than there was for real

lqd (Dec 27 2018 at 15:09, on Zulip):

it does feel though like there is some kind of "gap" opening up between the leapjoin and join stuff

interesting thought :thinking: — in the meantime, the new convenience APIs will at least allow to pick and choose for all the different cases we might need to support

nikomatsakis (Dec 27 2018 at 15:09, on Zulip):

yeah

nikomatsakis (Dec 27 2018 at 15:10, on Zulip):

it's ok, we can do 3.0 :)

nikomatsakis (Dec 27 2018 at 15:10, on Zulip):

major versions are free, I hear

lqd (Dec 27 2018 at 15:12, on Zulip):

:smile:

nikomatsakis (Dec 27 2018 at 15:13, on Zulip):

mostly it feels like there is some sort of "combinator"

nikomatsakis (Dec 27 2018 at 15:13, on Zulip):

that we haven't quite figured out

nikomatsakis (Dec 27 2018 at 15:14, on Zulip):

maybe it's just the differential API :)

lqd (Dec 27 2018 at 15:16, on Zulip):

probably :)

lqd (Dec 27 2018 at 15:44, on Zulip):

did you want another rustc test run after rebasing over the Prefix/Value filters ? (or maybe using polonius-engine 0.6.1 if that was a source of additional or missed errors in compare-mode), in any case merge at your leisure :)

nikomatsakis (Dec 27 2018 at 15:58, on Zulip):

nah, I don't see how that could introduce bugs

nikomatsakis (Dec 27 2018 at 15:59, on Zulip):

btw @lqd I updated the subregion blog post to the newer formulation, though I've yet to write the sequel, so I may yet tweak it a bit more :)

lqd (Dec 27 2018 at 15:59, on Zulip):

:ship: it (maybe accept the "reuslting" typo fix)

nikomatsakis (Dec 27 2018 at 16:00, on Zulip):

:)

nikomatsakis (Dec 27 2018 at 16:00, on Zulip):

yep

nikomatsakis (Dec 27 2018 at 16:00, on Zulip):

nah, I don't see how that could introduce bugs

famous last words if ever I heard them but wev :)

lqd (Dec 27 2018 at 16:02, on Zulip):

I updated the subregion blog post

awesome, I'll update the WIP branch soon, and if we need to tweak it a bit later, we'll tweak it later :)

lqd (Dec 27 2018 at 16:02, on Zulip):

(and also, thanks)

nikomatsakis (Dec 27 2018 at 16:12, on Zulip):

https://github.com/rust-lang-nursery/datafrog/pull/23

nikomatsakis (Dec 27 2018 at 16:19, on Zulip):

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

lqd (Dec 27 2018 at 16:24, on Zulip):

bors is stubbornly absent :)

lqd (Dec 27 2018 at 16:34, on Zulip):

(r+ed)

Matthew Jasper (Dec 27 2018 at 17:43, on Zulip):

@nikomatsakis are you going to add this to #56507, or can it be r+'d again now?

lqd (Dec 27 2018 at 18:03, on Zulip):

(datafrog's changes were mostly ergonomic, + the odd possible optimization, so maybe unlikely to be integrated into that PR ?)

lqd (Dec 27 2018 at 18:05, on Zulip):

(hmm, then again niko seems to be close to publishing polonius 0.6.2 so maybe :p — whichever of this and #56384 lands first will probably require the other to rebase anyway)

lqd (Dec 27 2018 at 23:04, on Zulip):

oh wow, higher-ranked relations require contains(R, L, P) :- placeholder_region(R, L), cfg_node(P). mind explodes — I thought these new errors could be more or less easily added to the existing variants but, alas, it seems I was wrong :)

nikomatsakis (Dec 28 2018 at 13:52, on Zulip):

I'm actually not entirely sure what higher-ranked regions will require. I have to think it over more. I thought I saw a "path" but I realized a few potential complications yesterday.

nikomatsakis (Dec 28 2018 at 13:52, on Zulip):

are you going to add this to #56507, or can it be r+'d again now?

added, rebased, r+'d, looks like it may even land for once =)

lqd (Dec 28 2018 at 14:11, on Zulip):

after seeing the effects of compression, anything joining to the full CFG is a tad scary at times :)

nikomatsakis (Dec 28 2018 at 15:17, on Zulip):

yeah, I'd like to eliminate that join

nikomatsakis (Dec 28 2018 at 15:17, on Zulip):

note that we do it today in rust code though

nikomatsakis (Dec 28 2018 at 15:18, on Zulip):

I think it can probably be eliminated, but it might complicate things

nikomatsakis (Dec 28 2018 at 15:20, on Zulip):

btw @lqd I extended https://github.com/rust-lang-nursery/polonius/pull/96

lqd (Dec 28 2018 at 15:21, on Zulip):

awesome!

lqd (Dec 28 2018 at 15:22, on Zulip):

thankfully it didn't uncover bugs :)

lqd (Dec 28 2018 at 15:24, on Zulip):

I guess the comment could mention that it properly checks subsets now and not just the lack of errors but that would be nitpicking

lqd (Dec 28 2018 at 15:38, on Zulip):

@nikomatsakis oh you did change it ;) sorry I noticed after you merged: in the first panic, aren't the first loan and point arguments reversed: "for {point} at {loan}" ?

nikomatsakis (Dec 28 2018 at 15:38, on Zulip):

I changed it after I saw your comment :)

nikomatsakis (Dec 28 2018 at 15:39, on Zulip):

regarding the debug message, probably...

lqd (Dec 28 2018 at 15:39, on Zulip):

no big deal as it won't trigger don't worry about it I'll open a PR :)

nikomatsakis (Dec 28 2018 at 15:39, on Zulip):

ok :)

nikomatsakis (Dec 28 2018 at 15:39, on Zulip):

ty

lqd (Dec 28 2018 at 15:39, on Zulip):

np

lqd (Dec 28 2018 at 15:56, on Zulip):

(done)

lqd (Dec 28 2018 at 17:41, on Zulip):

looks like it may even land for once =)

[narrator] it didn't :(

lqd (Dec 28 2018 at 17:42, on Zulip):

another panic trying to document rustc_codegen_llvm, no backtrace, and the option it complains about seems to exist

lqd (Dec 28 2018 at 17:46, on Zulip):

I'll ask infra in case they've seen it elsewhere, if it's spurious or anything

lqd (Dec 28 2018 at 18:46, on Zulip):

I think I found the vague origin of the failure, and am currently investigating to pinpoint it a bit more

lqd (Dec 28 2018 at 19:43, on Zulip):

(so it's a rustdoc ICE trying to document datafrog, the great simulacrum bisected it to commits close to the leapjoin API additions; I'm trying to minimize these changes so that the rustdoc team can look at it more easily)

lqd (Dec 28 2018 at 20:37, on Zulip):

(update: I've minimized it and will open a rust issue, I think I have a workaround/fix in the meantime)

lqd (Dec 28 2018 at 22:55, on Zulip):

(another parenthesis and another update: @nikomatsakis #56507 is hitting a "rustdoc issue" which I've minimized to #57180 and worked around/fixed in datafrog — once merged, and another :frog: version is published into another published version of polonius, and this PR using the latter THEN your PR will land famous last words :)

nikomatsakis (Jan 02 2019 at 19:34, on Zulip):

wow, awesome @lqd, thanks!

nikomatsakis (Jan 02 2019 at 19:36, on Zulip):

ps @lqd I think I will close https://github.com/rust-lang-nursery/datafrog/pull/19, given that it didn't seem to make things any faster, and it makes the datafrog code that much more complex and less uniform.

lqd (Jan 02 2019 at 19:38, on Zulip):

oh, was it about speed ? I didn't look at it myself so I was expecting it to be about "less copying"

nikomatsakis (Jan 02 2019 at 19:38, on Zulip):

well the idea was to pass around u32 values and not &u32 values

nikomatsakis (Jan 02 2019 at 19:38, on Zulip):

given that the former is smaller

nikomatsakis (Jan 02 2019 at 19:39, on Zulip):

idk, maybe i'll leave it open and re-explore in a bit. it would be nice to be a bit less pointer-centric

lqd (Jan 02 2019 at 19:39, on Zulip):

(and/or possible Variable in leapjoins)

nikomatsakis (Jan 02 2019 at 19:39, on Zulip):

but basically yes it's all a perf optimization

nikomatsakis (Jan 02 2019 at 19:39, on Zulip):

in the end

lqd (Jan 02 2019 at 19:40, on Zulip):

oh ok, it would have been cool !

nikomatsakis (Jan 02 2019 at 19:42, on Zulip):

datafrog v2.0.1 published

nikomatsakis (Jan 02 2019 at 19:43, on Zulip):

I am debating now -- I can presumably update my PR to just add a datafrog = 2.0.1 dependency into rustc

nikomatsakis (Jan 02 2019 at 19:43, on Zulip):

seems like a lot of trouble to re-issue a new version of polonius-engine

nikomatsakis (Jan 02 2019 at 19:44, on Zulip):

I could also just update the cargo lock file I guess but that feels..sketchy to me..?

nikomatsakis (Jan 02 2019 at 19:44, on Zulip):

not sure why

nikomatsakis (Jan 02 2019 at 19:44, on Zulip):

ah, well, just rebasing already did the job

nikomatsakis (Jan 02 2019 at 19:45, on Zulip):

i.e., regenerating the cargo lock gave me datafrog 2.0.1

nikomatsakis (Jan 02 2019 at 19:45, on Zulip):

I'll leave it at that :)

lqd (Jan 02 2019 at 19:45, on Zulip):

haha nice

Last update: Nov 21 2019 at 23:25UTC