Stream: t-compiler/wg-nll

Topic: impl-dfs-for-bitdenotation


Santiago Pastorino (Sep 10 2018 at 17:47, on Zulip):

@hey @nikomatsakis

nikomatsakis (Sep 10 2018 at 17:47, on Zulip):

:wave:

Santiago Pastorino (Sep 10 2018 at 17:47, on Zulip):

as a follow up of https://github.com/rust-lang/rust/issues/53394

nikomatsakis (Sep 10 2018 at 17:51, on Zulip):

yes

Santiago Pastorino (Sep 10 2018 at 17:53, on Zulip):

have some questions but got interrupted now for a bit, will ask in a couple of minutes

nikomatsakis (Sep 10 2018 at 17:55, on Zulip):

ok

Santiago Pastorino (Sep 10 2018 at 17:56, on Zulip):

so ... one thing that I was wondering was ...

Santiago Pastorino (Sep 10 2018 at 17:57, on Zulip):

I have self which was MirBorrowckCtxtand was using it's move_data member which is of type MoveData

Santiago Pastorino (Sep 10 2018 at 17:58, on Zulip):

but I need to get use the MoveOutStatements thing

Santiago Pastorino (Sep 10 2018 at 17:58, on Zulip):

which is the thing that implements BitDenotation

Santiago Pastorino (Sep 10 2018 at 17:59, on Zulip):

was wondering how to go from MoveData or from MirBorrowckCtxt to MoveOutStatements

Santiago Pastorino (Sep 10 2018 at 17:59, on Zulip):

one of the first things I have on my todo list is to read https://trello.com/c/rOwLDxgp/4439-review-start-to-document-mir-borrow-check-190

Santiago Pastorino (Sep 10 2018 at 17:59, on Zulip):

maybe I should read it before continuing with the task :)

nikomatsakis (Sep 10 2018 at 18:04, on Zulip):

hmm

nikomatsakis (Sep 10 2018 at 18:06, on Zulip):

the MoveOutStatements is not available, I don't think, from either of those directly

nikomatsakis (Sep 10 2018 at 18:06, on Zulip):

well

nikomatsakis (Sep 10 2018 at 18:06, on Zulip):

yeah so I think we need to create the BitDenotation but...

nikomatsakis (Sep 10 2018 at 18:07, on Zulip):

@Santiago Pastorino which was your PR again :)

nikomatsakis (Sep 10 2018 at 18:07, on Zulip):

well nm

nikomatsakis (Sep 10 2018 at 18:08, on Zulip):

so I think you can just create a new MovingOutStatements using the new function

nikomatsakis (Sep 10 2018 at 18:08, on Zulip):

(that is a pointer to an older version of rustc, when that code still existed)

nikomatsakis (Sep 10 2018 at 18:08, on Zulip):

it doesn't really carry any state itself

Santiago Pastorino (Sep 10 2018 at 18:19, on Zulip):

I see

Santiago Pastorino (Sep 10 2018 at 18:31, on Zulip):
pub struct MoveDataParamEnv<'gcx, 'tcx> {
    pub(crate) move_data: MoveData<'tcx>,
    pub(crate) param_env: ty::ParamEnv<'gcx>,
}
Santiago Pastorino (Sep 10 2018 at 18:32, on Zulip):

@nikomatsakis param_env?

nikomatsakis (Sep 10 2018 at 18:32, on Zulip):

you have that floating around somewhere

nikomatsakis (Sep 10 2018 at 18:32, on Zulip):

it stores the where-clauses and things that are in scope

Santiago Pastorino (Sep 10 2018 at 18:32, on Zulip):

just empty

Santiago Pastorino (Sep 10 2018 at 18:32, on Zulip):

ahh let me checl

nikomatsakis (Sep 10 2018 at 18:32, on Zulip):

e.g. here

nikomatsakis (Sep 10 2018 at 18:32, on Zulip):

https://doc.rust-lang.org/nightly/nightly-rustc/rustc_mir/borrow_check/struct.MirBorrowckCtxt.html#structfield.param_env

Santiago Pastorino (Sep 10 2018 at 18:33, on Zulip):

I had it in my nose :P

Santiago Pastorino (Sep 10 2018 at 18:33, on Zulip):

so close that couldn't find it :P

Santiago Pastorino (Sep 10 2018 at 18:38, on Zulip):

I have some doubts

Santiago Pastorino (Sep 10 2018 at 18:38, on Zulip):

what's exactly BitDenotation?

nikomatsakis (Sep 10 2018 at 18:39, on Zulip):

BitDenotation is a trait that defines a dataflow analysis

nikomatsakis (Sep 10 2018 at 18:39, on Zulip):

it basically says, for any given mir statement/terminator, what the gen/kill bits are

nikomatsakis (Sep 10 2018 at 18:39, on Zulip):

for some particular analysis

Santiago Pastorino (Sep 10 2018 at 18:40, on Zulip):

ok

Santiago Pastorino (Sep 10 2018 at 18:40, on Zulip):

you said that I should do ...

Santiago Pastorino (Sep 10 2018 at 18:40, on Zulip):
pub(crate) fn find_values_in_scope<BD: BitDenotation>(
    bit: &BD,
    mir: &Mir<'tcx>,
    location: Location,
    ) -> Vec<BD::Value> {
Santiago Pastorino (Sep 10 2018 at 18:40, on Zulip):

I was wondering if we needed a closure to be specific about what we are collecting

Santiago Pastorino (Sep 10 2018 at 18:40, on Zulip):

but I guess I may have all I need in the trait itself?

Santiago Pastorino (Sep 10 2018 at 18:41, on Zulip):

and unsure what is BD::Value?

Santiago Pastorino (Sep 10 2018 at 18:41, on Zulip):

isn't that Idx?

nikomatsakis (Sep 10 2018 at 18:43, on Zulip):

but I guess I may have all I need in the trait itself?

yes

Santiago Pastorino (Sep 10 2018 at 18:43, on Zulip):

ok

nikomatsakis (Sep 10 2018 at 18:43, on Zulip):

isn't that Idx?

yes, I think I meant Idx

Santiago Pastorino (Sep 10 2018 at 18:43, on Zulip):

will check a bit better

Santiago Pastorino (Sep 10 2018 at 18:54, on Zulip):

I see

Santiago Pastorino (Sep 10 2018 at 18:54, on Zulip):

the thing is I see the code is kind of equivalent

Santiago Pastorino (Sep 10 2018 at 18:54, on Zulip):

but unsure if it's exactly

Santiago Pastorino (Sep 10 2018 at 18:55, on Zulip):

for instance ...

Santiago Pastorino (Sep 10 2018 at 19:33, on Zulip):

https://github.com/rust-lang/rust/blob/b0297f3043e4ed592c63c0bcc11df3655ec8cf46/src/librustc_mir/borrow_check/error_reporting.rs#L545-L581

Santiago Pastorino (Sep 10 2018 at 19:34, on Zulip):

vs

nikomatsakis (Sep 10 2018 at 19:34, on Zulip):

I think if we use the generic version, we wouldn't get the continue part of that comment — is that what you mean?

nikomatsakis (Sep 10 2018 at 19:34, on Zulip):

that said, we can filter the results after the fact if we care

Santiago Pastorino (Sep 10 2018 at 19:35, on Zulip):

https://github.com/rust-lang/rust/blob/d5a448b3f47b22c9cb010198bdcc49d4392b090b/src/librustc_mir/dataflow/impls/mod.rs#L503-L529

Santiago Pastorino (Sep 10 2018 at 19:35, on Zulip):

exactly

Santiago Pastorino (Sep 10 2018 at 19:35, on Zulip):

first the continue part

Santiago Pastorino (Sep 10 2018 at 19:35, on Zulip):

and second if it's exactly equivalent

nikomatsakis (Sep 10 2018 at 19:39, on Zulip):

I think apart from that it should be

nikomatsakis (Sep 10 2018 at 19:39, on Zulip):

well, the current code is not handling the return value of a call correctly

nikomatsakis (Sep 10 2018 at 19:39, on Zulip):

which.. might be observable, if we make the right example

nikomatsakis (Sep 10 2018 at 19:40, on Zulip):

but I'm not sure

nikomatsakis (Sep 10 2018 at 19:40, on Zulip):

the way that MIR construction works, it might not be visible

Santiago Pastorino (Sep 10 2018 at 19:41, on Zulip):

ok

Santiago Pastorino (Sep 10 2018 at 19:41, on Zulip):

gonna just change things and see what happens I guess :)

nikomatsakis (Sep 10 2018 at 19:45, on Zulip):

@Santiago Pastorino ok :) this does seem like a good way to get to learn that system

nikomatsakis (Sep 10 2018 at 19:45, on Zulip):

for now I think you can ignore the "call return value" thing

nikomatsakis (Sep 10 2018 at 19:45, on Zulip):

we can return to that later :P

Santiago Pastorino (Sep 10 2018 at 19:46, on Zulip):

ok

Santiago Pastorino (Sep 10 2018 at 19:46, on Zulip):

another thing I don't see is where do I use the MovePathIndex

Santiago Pastorino (Sep 10 2018 at 19:47, on Zulip):

given the way I build a MovingOutStatements and find_values_in_scope signature I don't need it, but that per se sounds strange

nikomatsakis (Sep 10 2018 at 19:48, on Zulip):

I think the depth-first-search code would probably be agnostic about that

nikomatsakis (Sep 10 2018 at 19:48, on Zulip):

that is, it would just talk about BD::Idxvalues and not care about anything else

Santiago Pastorino (Sep 10 2018 at 19:55, on Zulip):

is there any good example of usage of the BitDenotation API that you remember?

nikomatsakis (Sep 10 2018 at 20:02, on Zulip):

uh

nikomatsakis (Sep 10 2018 at 20:02, on Zulip):

well, there's the dataflow code :)

nikomatsakis (Sep 10 2018 at 20:02, on Zulip):

it's not the most well documented API in the planet I imagine

nikomatsakis (Sep 10 2018 at 20:03, on Zulip):

Hmm, so i'm looking over the details

nikomatsakis (Sep 10 2018 at 20:03, on Zulip):

the real question, @Santiago Pastorino, is how important this exercise is — I was feeling like maybe we want to avoid repeating the same logic over and over again

nikomatsakis (Sep 10 2018 at 20:03, on Zulip):

and this is a way to do that

nikomatsakis (Sep 10 2018 at 20:03, on Zulip):

but otoh are there any other places we do this sort of thing?

nikomatsakis (Sep 10 2018 at 20:03, on Zulip):

I'm having minor misgivings

nikomatsakis (Sep 10 2018 at 20:03, on Zulip):

particualrly now that you landed your first PR

nikomatsakis (Sep 10 2018 at 20:03, on Zulip):

starting to wonder if it makes more sense to focus on something else

nikomatsakis (Sep 10 2018 at 20:04, on Zulip):

though I'd have to find that thing :)

nikomatsakis (Sep 10 2018 at 20:04, on Zulip):

of course, it seems good to have spent some time reading into the code, and maybe it's worth it to push a bit more

nikomatsakis (Sep 10 2018 at 20:04, on Zulip):

I can sort of send you various bits of code that use the API, but let's talk a sec about this at a higher level

nikomatsakis (Sep 10 2018 at 20:05, on Zulip):

one thing in particular...

nikomatsakis (Sep 10 2018 at 20:05, on Zulip):

ah right

nikomatsakis (Sep 10 2018 at 20:05, on Zulip):

so we are not interested in all move outs

nikomatsakis (Sep 10 2018 at 20:05, on Zulip):

but only move outs from certain paths, right?

Santiago Pastorino (Sep 10 2018 at 20:09, on Zulip):

yes

Santiago Pastorino (Sep 10 2018 at 20:10, on Zulip):

about if continuing or not don't worry if stopping is the thing you consider the best

Santiago Pastorino (Sep 10 2018 at 20:10, on Zulip):

because I have some stuff to read anyway

Santiago Pastorino (Sep 10 2018 at 20:10, on Zulip):

in particular, wanted to go over this https://github.com/rust-lang-nursery/rustc-guide/pull/190

Santiago Pastorino (Sep 10 2018 at 20:10, on Zulip):

but I could also continue

Santiago Pastorino (Sep 10 2018 at 20:10, on Zulip):

whatever makes more sense to you :)

Santiago Pastorino (Sep 10 2018 at 20:11, on Zulip):

is not a thing that I have been working on this for years, I've pasted the old code some days ago and I'm kind of starting with it

nikomatsakis (Sep 10 2018 at 20:18, on Zulip):

it seems like it can't be the highest priority task; the code works now and we don't have another consumer now for this more generic approach. I'm also not entirely convinced that the BitDenotation trait setup is the most clear :)

Santiago Pastorino (Sep 10 2018 at 20:18, on Zulip):

done :)

Santiago Pastorino (Sep 10 2018 at 20:18, on Zulip):

gonna read all the beautiful stuff you have written :)

nikomatsakis (Sep 10 2018 at 20:19, on Zulip):

heh, it's not that much

nikomatsakis (Sep 10 2018 at 20:19, on Zulip):

but please do read

nikomatsakis (Sep 10 2018 at 20:19, on Zulip):

I would like to write more...

Santiago Pastorino (Sep 10 2018 at 20:19, on Zulip):

if there's any other task where I can help let me know

nikomatsakis (Sep 10 2018 at 20:19, on Zulip):

actually, I wanted to document some of the BitDenotation stuff

Santiago Pastorino (Sep 10 2018 at 20:19, on Zulip):

yeah, anyway I have the whole guide to read :)

nikomatsakis (Sep 10 2018 at 20:20, on Zulip):

one option might be to look at https://github.com/rust-lang/rust/issues/53114; this would involve poking at MIR construction

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

but I think we have to narrow down on the precise steps we want to take there

nikomatsakis (Sep 10 2018 at 20:23, on Zulip):

actually, probably the work that @Matthew Jasper is doing on _ patterns should land first

nikomatsakis (Sep 10 2018 at 20:23, on Zulip):

so yeah I guess that they are on this, basically

nikomatsakis (Sep 10 2018 at 20:23, on Zulip):

but not the unsafe fix part

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

that is, https://github.com/rust-lang/rust/issues/54003

Last update: Nov 22 2019 at 01:00UTC