Stream: t-compiler/wg-nll

Topic: friendly-front-end


nikomatsakis (May 14 2018 at 20:26, on Zulip):

(branching off into a separate topic)@lqd

nikomatsakis (May 14 2018 at 20:26, on Zulip):

well, plausibly we could do something like souffle, but that's not especially friendly

nikomatsakis (May 14 2018 at 20:26, on Zulip):

I was thinking more like the input resembles a Rust program

nikomatsakis (May 14 2018 at 20:26, on Zulip):

or a MIR dump

lqd (May 14 2018 at 20:27, on Zulip):

oh I see :) I was just mentioning it since you had already done some work in that direction

lqd (May 14 2018 at 20:28, on Zulip):

but it's true that embedding a rust fragment à la chalk would be natural I think

lqd (May 20 2018 at 15:16, on Zulip):

@nikomatsakis would we need a kills(Loan)kind of fact in statements, in addition to outlives/borrow_region_at/invalidates ?

lqd (May 20 2018 at 16:09, on Zulip):

I was also wondering how this needed to be integrated. I did the parser for the format you described this morning (but I'm not generating AllFacts yet) with lalrpop. So maybe, a subcrate like chalk does?

lqd (May 20 2018 at 22:06, on Zulip):

hum, I'm not sure at which statement point should we emit invalidates facts ?

Reed Koser (May 20 2018 at 22:07, on Zulip):

Mostly the mid point, but the start point for some of the generator resume stuff. That might not matter for the "friendly front-end" though

Reed Koser (May 20 2018 at 22:07, on Zulip):

(generator resume, not mid point. Mid point is important =))

Reed Koser (May 20 2018 at 22:08, on Zulip):

My understanding is that the mid point is "as the statement executes," and since it's the process of executing the statement that invalidates the fact that's when you want to emit the invalidate

lqd (May 20 2018 at 22:08, on Zulip):

ok I'll just emit from the mid point for now, when we get to testing generator-like facts in the friendly front-end, we'll see :)

Reed Koser (May 20 2018 at 22:09, on Zulip):

:+1: That's what the code I added to rustc does, so if it's wrong we're wrong together :smile:

lqd (May 20 2018 at 23:21, on Zulip):

little update: almost done on the frontend parsing and creating AllFacts — mostly missing the previous questions about killand where to put everything. (oh, and I eyeballed it but will add tests when I know where to add them :)

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

@nikomatsakis would we need a kills(Loan)kind of fact in statements, in addition to outlives/borrow_region_at/invalidates ?

yes.

nikomatsakis (May 21 2018 at 12:35, on Zulip):

I was also wondering how this needed to be integrated. I did the parser for the format you described this morning (but I'm not generating AllFacts yet) with lalrpop. So maybe, a subcrate like chalk does?

The reason that I separated chalk into two crates was because LALRPOP compile times were very painful back in the day. This is less true now with the table-based parser and incremental compilation. Still, it's fine to break the parser into its own crate if we want. Ultimately, I would expect this structure in the end, which is basically the same as chalk:

nikomatsakis (May 21 2018 at 12:36, on Zulip):

Mostly the mid point, but the start point for some of the generator resume stuff. That might not matter for the "friendly front-end" though

that's a good point, I forgot that some events occur in the start point of statements. I was intending everything to come at the mid point. Maybe we should change the format of a statement to have an optional "start" — or maybe just a basic block — something like:

Statement := Effects | Effects "/" Effects
Effects := ...

so you can write invalidated(L0) / ... and have the `invalidated appear in the start point, but otherwise you are just specifying things in the mid-point.

lqd (May 21 2018 at 13:04, on Zulip):

yeah from what I could tell (but haven't checked in rustc, only in the polonius inputs; and also I might have misunderstood) use (to emit region_live_at) and outlives occur in both start and mid points ?

nikomatsakis (May 21 2018 at 13:05, on Zulip):

yeah from what I could tell (but haven't checked in rustc, only in the polonius inputs; and also I might have misunderstood) use (to emit region_live_at) and outlives occur in both start and mid points ?

I think that use (which is not a fact) would be at mid points, but that will cause region_live_at at start points, yes

nikomatsakis (May 21 2018 at 13:05, on Zulip):

I don't think we emit outlives at start points?

lqd (May 21 2018 at 13:08, on Zulip):

there are quite a number in clap IIRC

nikomatsakis (May 21 2018 at 13:09, on Zulip):

hmm. curious.

nikomatsakis (May 21 2018 at 13:09, on Zulip):

could be, not sure if it should be

nikomatsakis (May 21 2018 at 13:09, on Zulip):

oh

nikomatsakis (May 21 2018 at 13:09, on Zulip):

I think I know what those are

nikomatsakis (May 21 2018 at 13:09, on Zulip):

that's something else

nikomatsakis (May 21 2018 at 13:09, on Zulip):

@lqd that is due to https://github.com/rust-lang-nursery/polonius/issues/24

nikomatsakis (May 21 2018 at 13:09, on Zulip):

I have a fix for that

nikomatsakis (May 21 2018 at 13:09, on Zulip):

I just haven't opened the PR

nikomatsakis (May 21 2018 at 13:09, on Zulip):

because I've been too busy

nikomatsakis (May 21 2018 at 13:10, on Zulip):

or at least a tentative fix

nikomatsakis (May 21 2018 at 13:10, on Zulip):

should be another small win on clap actually

nikomatsakis (May 21 2018 at 13:10, on Zulip):

I think it reduces the number of outlives facts by 2

lqd (May 21 2018 at 13:10, on Zulip):

oh alright awesome

nikomatsakis (May 21 2018 at 13:11, on Zulip):

I should at least open a [WIP] PR on rustc

nikomatsakis (May 21 2018 at 13:11, on Zulip):

it degrades a few error msgs

nikomatsakis (May 21 2018 at 13:11, on Zulip):

that was the main problem?

nikomatsakis (May 21 2018 at 13:11, on Zulip):

maybe there was an ICE in one case, I can't recall

nikomatsakis (May 21 2018 at 13:12, on Zulip):

/me will rebase and find out

nikomatsakis (May 21 2018 at 13:12, on Zulip):

there are some interesting language questions that get raised

nikomatsakis (May 21 2018 at 13:12, on Zulip):

that have to do with the basic premise of using liveness though

nikomatsakis (May 21 2018 at 13:12, on Zulip):

but I think that's orthogonal ... I have to write that up too ...

lqd (May 21 2018 at 13:12, on Zulip):

so use only emits a region_live_aton start and that's it, the rest can be manually specified (once I remove my outlives derivation on Starts) with the mentioned "/" separator

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

@lqd I opened https://github.com/rust-lang/rust/pull/50938

lqd (May 23 2018 at 19:04, on Zulip):

@nikomatsakis here it is https://github.com/rust-lang-nursery/polonius/pull/45 — as I'm not very familiar with LALRPOP, there might be a lot of things to improve

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

as for the question whether to hack on the PR or iterate in-tree, whichever you prefer ?

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

usually I like hacking in tree better, so others can "join in" and you don't have to do all the rebase pain

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

I'm not entirely sure what I think about the use vs def and how rustc should interact, but probably for now having a separate computation (that rustc maybe uses later) is probably the way to go — in particular if we compressed edges, then having datafrog do it might be better

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

I'm fine with hacking in tree as well :) do you want me at least to nest the Effects enum first ?

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

and this liveness computation could be indeed for https://github.com/rust-lang/rust/issues/51003 - I can do that, or Keith also, as they were looking to work with datafrog :D

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

@lqd I think having someone else to do liveness is even better :) (grow knowledge base)

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

understood, let's go @Keith Yeung :)

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

until we have the computed liveness, we could also emit region_live_atmanually at Start points

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

@lqd conceivably we could remove use and just have people write region_live_at manually to start .. awfully tedious though to get the start/end right .. so I guess you're saying we could special-case that ..? seems reasonable

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

I'm about to push an updated version with the nested Fact enum + comma-separated regions in use

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

manual region_live_at it could be fine for smaller tests, and those would be of little interest

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

so I can guess we can land as is but still have to wait for the liveness computation to have our awesome tests ?

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

(I was really wishing to have some tests before landing leapfrog :)

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

so I can guess we can land as is but still have to wait for the liveness computation to have our awesome tests ?

yeah, sounds good

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

(I was really wishing to have some tests before landing leapfrog :)

might make sense to wait until we get rustc integration going, or else keep leapfrog as a separate algorithm

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

yup, separate algorithm was also proposed by Frank

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

the API is still on a branch in his repo, and maybe the exact API is not nailed down perfectly yet anyway

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

ofc the rebase didn't work, I'll reopen the PR later :sob:

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

@nikomatsakis :sob: sorry GH didn't allow me to reopen so https://github.com/rust-lang-nursery/polonius/pull/48 -- I'll open issues for the specific points we talked about wrt use/def, liveness, etc afterwards

Keith Yeung (May 24 2018 at 13:03, on Zulip):

@lqd it would've worked if you didn't close the PR btw

lqd (May 24 2018 at 13:04, on Zulip):

right

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

@nikomatsakis :sob: sorry GH didn't allow me to reopen so https://github.com/rust-lang-nursery/polonius/pull/48 -- I'll open issues for the specific points we talked about wrt use/def, liveness, etc afterwards

...I maybe can re-open it?

lqd (May 24 2018 at 13:08, on Zulip):

I don't think so, it's because of the force push

lqd (May 24 2018 at 13:08, on Zulip):

_while being closed_

lqd (May 24 2018 at 13:08, on Zulip):

otherwise it's fine ofc

Keith Yeung (May 24 2018 at 13:10, on Zulip):

the force is strong with this one

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

@lqd er I forgot about your PR for a while now :)

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

I see it's been pending

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

looks like it is still mergable though

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

er, not quite

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

just needs a rebase with the Cargo.toml etc

lqd (May 28 2018 at 14:47, on Zulip):

Oh cool, if you think we can land as is, I’ll rebase either at the airport or tomorrow :)

lqd (May 28 2018 at 18:39, on Zulip):

@nikomatsakis here you go, my daily rebase of https://github.com/rust-lang-nursery/polonius/pull/48 — straight from the airport :airplane: :) definitely need to create issues about def + liveness computation (maybe someone at the impl days tomorrow will want to do it) but until I create it, I updated the PR's message to mention it still needs those, and link to the relevant comment of yours.

nikomatsakis (May 28 2018 at 18:41, on Zulip):

@lqd merged =)

lqd (May 28 2018 at 18:41, on Zulip):

awesome thank you :D

lqd (Jun 06 2018 at 08:46, on Zulip):

@nikomatsakis here's the PR I promised last night — https://github.com/rust-lang-nursery/polonius/pull/70

Last update: Nov 21 2019 at 13:25UTC