Stream: t-compiler

Topic: Dataflow-based UseDef chains


Dylan MacKenzie (Jun 17 2019 at 21:20, on Zulip):

On a lark, I decided to try implementing a reaching definitions analysis for rustc. The main use cases for this are dataflow-based const-qualification as discussed in https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/dataflow-based.20const.20qualification.20MVP and better const-propagation during MIR optimization. I'll post a WIP PR when I finish adapting the rustc_peek machinery to my use case.

I wanted to let people know that someone is working on this, as well as solicit feedback on implementation strategy and other potential use cases.

The current analysis:
- Is field-insensitive (assignments to projections like x.y = ... will not kill any previous assignments to x, even ones like x.y.z = .... I think this is not too hard to implement correctly, but I need to incorporate code from some other analyses)
- Handles pointer aliasing by killing all definitions of a variable whose address is observed at any point whenever a definition goes through a pointer (e.g. *p = ...) (this was proposed by eddyb on github in an issue I can no longer find)
- Is context-insensitive and very conservative w.r.t. function calls (it treats function calls just like *p = .... This is because a function could take a reference (or a raw pointer) to a local as an argument and mutate it).

I'll post here again when I have a draft PR ready. Also, please be gentle, I've never had a compilers course :).

oli (Jun 18 2019 at 10:32, on Zulip):

cc @WG-mir-opt

Dylan MacKenzie (Jun 18 2019 at 16:43, on Zulip):

@eddyb @pnkfelix Why does state_for_location initialize the gen and kill sets to the entry state of the basic block in question? https://github.com/rust-lang/rust/blob/44fb88d25282d9362774536965f2455f677734f3/src/librustc_mir/dataflow/mod.rs#L361-L363

eddyb (Jun 18 2019 at 16:43, on Zulip):

I don't actually know

eddyb (Jun 18 2019 at 16:44, on Zulip):

that might be the thing @Zoxc added

eddyb (Jun 18 2019 at 16:44, on Zulip):

also, IIRC @nagisa tried to add a "reaching definition" analysis

eddyb (Jun 18 2019 at 16:44, on Zulip):

or something more powerful, I forget

eddyb (Jun 18 2019 at 16:45, on Zulip):

@Dylan MacKenzie you don't need it for const-qualif though?

Dylan MacKenzie (Jun 18 2019 at 16:47, on Zulip):

No, I'm trying to overload rustc_peek to spit out the set of reaching defs for debugging. AFAICT it's doing its own version of state_for_location, but only looking at assignments since that's the only place that will change the initialization state of a move path.

Zoxc (Jun 18 2019 at 16:55, on Zulip):

Not all fields of BlockSets is used in state_for_location, I think. Also you shouldn't be using state_for_locationif you're looking at many or all locations.

Dylan MacKenzie (Jun 18 2019 at 17:13, on Zulip):

@Zoxc, I'm not sure I quite understand your first point. I think someone is working on a cursor to remove the O(n^2) behavior, but that isn't an issue here since rustc_peek is only needs the state at the end of a block.

pnkfelix (Jun 19 2019 at 14:09, on Zulip):

My current inference is that the pattern in question originated here: PR 44480, fn state_for_location

pnkfelix (Jun 19 2019 at 14:09, on Zulip):

Note that in particular, that code is creating the IdxSetBuf (entry), and passing a mutable reference to it via the gen_set field.

pnkfelix (Jun 19 2019 at 14:10, on Zulip):

it also clones it for the on_entry and kill_set fields, but those clones are discarded by this code.

pnkfelix (Jun 19 2019 at 14:11, on Zulip):

the one that is returned is the one referenced by the gen_set field, because that presumably reflects the accumulated intra-basicblock dataflow state up to and including loc

pnkfelix (Jun 19 2019 at 14:12, on Zulip):

In other words, I think the point is this: We want to compute the intra-block gen-set. We do it by starting with the state at the start of the control flow of the block, and then compute the effect of each statement (as is reflected in the gen-set state that is imperatively modified by statement_effect

pnkfelix (Jun 19 2019 at 14:13, on Zulip):

The fact that we are also creating valid &mut IdxSetBuf values for on_entry and kill_set is, I assume, to allow statement_effect to maintain or reference state within those bits.

pnkfelix (Jun 19 2019 at 14:14, on Zulip):

@Zoxc does the above summary (of how fn state_for_location works) sound approximately correct to you?

pnkfelix (Jun 19 2019 at 14:18, on Zulip):

(Now, if you ask me, it probably doesn't make sense for the statement_effect method to actually read the current value of the gen_set/kill_set it receives. If my memory is right, it generally just writes to those arrays. And that may be why it does not matter that we are passing in a clone of entry_set as a value for kill_set here; because, I think, no implementation of statement_effect would read those bits anyway.)

Zoxc (Jun 19 2019 at 15:18, on Zulip):

Sounds approximately correct to me =P

Dylan MacKenzie (Jun 19 2019 at 17:03, on Zulip):

So does state_at_location just ignore kills?

pnkfelix (Jun 20 2019 at 09:38, on Zulip):

@Dylan MacKenzie well, I suspect it doesn't ignore all kills. In particular, since its computing the cumulative effect of some prefix of the block (starting from the entry set state of that block), it needs to apply the kills for each statement to the state

pnkfelix (Jun 20 2019 at 09:39, on Zulip):

which is fine; I believe that is indeed part of what happens during the loop

pnkfelix (Jun 20 2019 at 09:39, on Zulip):

what it does ignore is the final value of the cloned kill-set. It throws that away.

pnkfelix (Jun 20 2019 at 09:39, on Zulip):

and I believe that any state written to that cloned kill-set does not matter.

Dylan MacKenzie (Jun 20 2019 at 15:28, on Zulip):

I figured out why this works, but it's kind of subtle.

Dylan MacKenzie (Jun 20 2019 at 15:28, on Zulip):
pub fn state_for_location<'tcx, T: BitDenotation<'tcx>>(loc: Location,
                                                        analysis: &T,
                                                        result: &DataflowResults<'tcx, T>,
                                                        body: &Body<'tcx>)
    -> BitSet<T::Idx> {
    let mut on_entry = result.sets().on_entry_set_for(loc.block.index()).to_owned();
    let mut kill_set = on_entry.to_hybrid();
    let mut gen_set = kill_set.clone();

    {
        let mut sets = BlockSets {
            on_entry: &mut on_entry,
            kill_set: &mut kill_set,
            gen_set: &mut gen_set,
        };

        for stmt in 0..loc.statement_index {
            let mut stmt_loc = loc;
            stmt_loc.statement_index = stmt;
            analysis.before_statement_effect(&mut sets, stmt_loc);
            analysis.statement_effect(&mut sets, stmt_loc);
        }

        // Apply the pre-statement effect of the statement we're evaluating.
        if loc.statement_index == body[loc.block].statements.len() {
            analysis.before_terminator_effect(&mut sets, loc);
        } else {
            analysis.before_statement_effect(&mut sets, loc);
        }
    }

    gen_set.to_dense()
}
Dylan MacKenzie (Jun 20 2019 at 15:30, on Zulip):

It's because the kill and kill_all setters for BlockSets remove bits from gen_set that conflict with the newly added kills.

Dylan MacKenzie (Jun 20 2019 at 15:36, on Zulip):

I'd like for someone (possibly me) to replace this API with a cursor which caches the result of the last call of state_for_location. This way accessing statements sequentially won't be O(n^2) (as mentioned above).

Dylan MacKenzie (Jun 20 2019 at 15:39, on Zulip):

Hence all the questions :)

pnkfelix (Jun 20 2019 at 19:01, on Zulip):

right: computing the cumulative gen-set for the whole block requires combining both the gens and the kills of the statements in the block.

pnkfelix (Jun 20 2019 at 19:01, on Zulip):

(a critical detail I left out above)

Dylan MacKenzie (Jun 20 2019 at 19:38, on Zulip):

There's a couple more issues another issue I have with the implementation.

First, should state_for_location include before_statement_effect for the given location? Currently it does not. Ignore this, I was wrong :)

Dylan MacKenzie (Jun 20 2019 at 19:41, on Zulip):

Second, when we're building the transfer function by visiting each statement, on_entry is empty (or full if bottom_value == true) at the start of every basic block except START_BLOCK (which has start_block_effect applied to it). If accumulates_intrablock_state is true, each statement gets to observe the effect of all previous statements before it in the same block on the initial on_entry when deciding its transfer function.

Dylan MacKenzie (Jun 20 2019 at 19:42, on Zulip):

However, in state_for_location, on_entry is initialized to the fixpoint instead.

Dylan MacKenzie (Jun 20 2019 at 19:42, on Zulip):

@pnkfelix

Dylan MacKenzie (Jun 20 2019 at 19:44, on Zulip):

(assuming dataflow analysis is run before state_for_location is called

Dylan MacKenzie (Jun 20 2019 at 19:48, on Zulip):

I need modify this to support arbitrary transfer functions as described here: https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/dataflow-based.20const.20qualification.20MVP/near/168608696

Dylan MacKenzie (Jun 20 2019 at 19:49, on Zulip):

Whether or not this is a good idea is TBD :)

pnkfelix (Jun 20 2019 at 20:03, on Zulip):

What was the question?

pnkfelix (Jun 20 2019 at 20:03, on Zulip):

in the first case, we are computing the transfer function for the whole block, before we know what the fixed point is

pnkfelix (Jun 20 2019 at 20:04, on Zulip):

in the second case, we are taking the fixed point result (which is only saved at the entry point for each block), and using it to compute the precise state within a block

Dylan MacKenzie (Jun 20 2019 at 20:05, on Zulip):

To do that, we have to recompute the transfer function for each statement, but we're passing different inputs to the function which determines the transfer function in state_for_location than we are when we initially run dataflow.

Dylan MacKenzie (Jun 20 2019 at 20:06, on Zulip):

@pnkfelix which I think is incorrect

pnkfelix (Jun 20 2019 at 20:06, on Zulip):

yeah so that probably prvides evidence that statement_effectshould not be reading from the state of on_entry in its calculation, right?

Dylan MacKenzie (Jun 20 2019 at 20:08, on Zulip):

When I did the refactor in #61787, I initially removed that parameter since I couldn't see when you would want to read from on_entry

pnkfelix (Jun 20 2019 at 20:08, on Zulip):

someone does want to read from it?

Dylan MacKenzie (Jun 20 2019 at 20:08, on Zulip):

but it's currently used in...

pnkfelix (Jun 20 2019 at 20:09, on Zulip):

oh maybe its used for something in functions that use accumulates_intrablock_state, let me look

Dylan MacKenzie (Jun 20 2019 at 20:09, on Zulip):

https://github.com/rust-lang/rust/blob/f0c2bdf52e0c8bce258f1bbb5652c9691b7f3193/src/librustc_mir/dataflow/impls/borrows.rs#L207

Dylan MacKenzie (Jun 20 2019 at 20:10, on Zulip):

this function

pnkfelix (Jun 20 2019 at 20:10, on Zulip):

hmm

pnkfelix (Jun 20 2019 at 20:11, on Zulip):

seems dangerous

Dylan MacKenzie (Jun 20 2019 at 20:12, on Zulip):

Updating on_entry when defining the transfer function is the only reason accumulates_intrablock_state exists right?

pnkfelix (Jun 20 2019 at 20:13, on Zulip):

that was what I was initially thinking, but I think you found a counter example

pnkfelix (Jun 20 2019 at 20:14, on Zulip):

my current guess is that kill_borrows_on_place is reading from on_entry only as a way to avoid iterating over the whole universe

pnkfelix (Jun 20 2019 at 20:15, on Zulip):

in terms of what is "correct" for the API as designed, especially given that we start with empty-sets for on_entry, I would have thought you need to compute the kill-sets there more conservatively

pnkfelix (Jun 20 2019 at 20:15, on Zulip):

cc @davidtwco ^

Dylan MacKenzie (Jun 20 2019 at 20:17, on Zulip):

So right now this "kill"s all borrows that occurred earlier in the same basic block that definitely conflict with the borrow at the current statment.

pnkfelix (Jun 20 2019 at 20:18, on Zulip):

right, and that doesn't sound sufficient to me

davidtwco (Jun 20 2019 at 20:18, on Zulip):

I last looked at this part of the compiler quite a while ago so I’ll need to bring it back into cache to be useful here.

pnkfelix (Jun 20 2019 at 20:22, on Zulip):

(yeah its been a while for me too)

pnkfelix (Jun 20 2019 at 20:23, on Zulip):

one reason why we may not be hitting bugs here as often as you might expect is because we only go into this path for non-Local places.

Dylan MacKenzie (Jun 20 2019 at 20:24, on Zulip):

The issue you're discussing is specific to the borrow check analysis though? It's sometimes valid to look at the accumulated state within a block?

Dylan MacKenzie (Jun 20 2019 at 20:25, on Zulip):

And can starting with the fixpoint instead of the empty set (or start_block_effect) in state_for_location cause errors?

pnkfelix (Jun 20 2019 at 20:26, on Zulip):

oh sure: what I'm currently saying is that I'm worried that kill_borrows_on_place is buggy.

pnkfelix (Jun 20 2019 at 20:27, on Zulip):

In general ... I would need to review the other cases that read from on_entry

Dylan MacKenzie (Jun 20 2019 at 20:27, on Zulip):

You've just reviewed all of them :)

pnkfelix (Jun 20 2019 at 20:27, on Zulip):

namely the ones that motivated the addition of fn accumulates_intrablock_state

Dylan MacKenzie (Jun 20 2019 at 20:27, on Zulip):

AFAICT

pnkfelix (Jun 20 2019 at 20:28, on Zulip):

wait, so does no one override fn accumulates_intrablock_state anymore?

pnkfelix (Jun 20 2019 at 20:29, on Zulip):

And can starting with the fixpoint instead of the empty set (or start_block_effect) in state_for_location cause errors?

I do not believe this would cause errors. In fact, I think starting with the fixed point is necessary for the correctness of state_for_location

Dylan MacKenzie (Jun 20 2019 at 20:30, on Zulip):

I see no one overriding accumulates_intrablock_state (including Borrows, which means the entry set is always empty except in the start block)

Dylan MacKenzie (Jun 20 2019 at 20:33, on Zulip):

I'll try to rephrase, I think a proper implementation of state_for_location should:
1. Initialize a BitSet to the fixpoint at entry to the basic block.
2. Pass an empty bitset to {statement,terminator}_effect (if accumulates_intrablock_state is false) to get the transfer function.
3. Apply the transfer function to the fixpoint

pnkfelix (Jun 20 2019 at 20:34, on Zulip):

that sounds like that would be correct

pnkfelix (Jun 20 2019 at 20:34, on Zulip):

but I think the end effect would be the same as what it does now?

Dylan MacKenzie (Jun 20 2019 at 20:34, on Zulip):

Currently step 2 is pass the fixpoint to {statement,terminator}_effect to get the transfer function

pnkfelix (Jun 20 2019 at 20:35, on Zulip):

Well, it would be the same, depending on whether {statement, terminator}_effect are allowed to change their behavior based on reading from gen-set or kill-set.

pnkfelix (Jun 20 2019 at 20:36, on Zulip):

(i've been assuming they are not allowed to read; only write. And thus, the current implementation uses the fixed-point-computed on_entry, and passes that in as the initial value for gen-set. Then applying the effect of each statement updates gen-set accordingly.)

Dylan MacKenzie (Jun 20 2019 at 20:37, on Zulip):

So the fixpoint is passed to on_entry as well in state_for_location, meaning that reading from on_entry (as is done in Borrows) may also cause a different transfer function to be generated depending on whether we're actually running dataflow or doing state_for_location

pnkfelix (Jun 20 2019 at 20:37, on Zulip):

But the implementation you describe sounds like it could be more robust, in terms of not having to follow rules like "don't ever read from gen-sets/`kill-sets

pnkfelix (Jun 20 2019 at 20:38, on Zulip):

yeah I think the problem here is that I don't think of the gen-set being computed by state_for_location as being the transfer function. It is the result of applying the transfer function to the entry set

pnkfelix (Jun 20 2019 at 20:39, on Zulip):

or, yes, sorry

pnkfelix (Jun 20 2019 at 20:39, on Zulip):

I misread your message

Dylan MacKenzie (Jun 20 2019 at 20:39, on Zulip):

I agree, but the question is how do you get the transfer function.

pnkfelix (Jun 20 2019 at 20:39, on Zulip):

reading from on_entry, as is done in Borrows, does seem like it could cause a different transfer function to be computed in the two cases

pnkfelix (Jun 20 2019 at 20:39, on Zulip):

which is exactly what I'm sitting here worried about

pnkfelix (Jun 20 2019 at 20:40, on Zulip):

especially since the transfer function you get from on_entry=null-set seems like it might miss important entries in the kill-set.

Dylan MacKenzie (Jun 20 2019 at 20:41, on Zulip):

It seems wrong to me too (especially since accumulate_intrablock_state is false), but I'd have to read more to understand the original intent.

pnkfelix (Jun 20 2019 at 20:43, on Zulip):

oh I now see who was using accumulate_intrablock_state

Dylan MacKenzie (Jun 20 2019 at 20:43, on Zulip):

Can I go to a PR from a commit hash?

pnkfelix (Jun 20 2019 at 20:43, on Zulip):

it originated from #46537

pnkfelix (Jun 20 2019 at 20:43, on Zulip):

Can I go to a PR from a commit hash?

I do it via the web interface for github

Dylan MacKenzie (Jun 20 2019 at 20:44, on Zulip):

https://github.com/rust-lang/rust/commit/db635fc5664c0a19cdb4063545ca2d2b297f0212

pnkfelix (Jun 20 2019 at 20:44, on Zulip):

e.g. if you look at this commit, you'll see it says "master (#46537)" beneath the log message for the commit.

Dylan MacKenzie (Jun 20 2019 at 20:44, on Zulip):

Ah, got it

pnkfelix (Jun 20 2019 at 20:45, on Zulip):

okay so first things first

pnkfelix (Jun 20 2019 at 20:45, on Zulip):

lets kill off accumulates_intrablock_state

pnkfelix (Jun 20 2019 at 20:45, on Zulip):

before anyone else has "really bright ideas" like I did in that old buggy two-phase borrows hack.

pnkfelix (Jun 20 2019 at 20:47, on Zulip):

but also

Dylan MacKenzie (Jun 20 2019 at 20:47, on Zulip):

I do have one use case, if we allow transfer functions that copy bits from other locals in the dataflow set (this is one approach to dataflow-based const-qualification, the other is based on reaching definitions). Then we may want to simply gen the qualification bits for _2 in code like:

_1 = {something qualified}
_2 = _1
pnkfelix (Jun 20 2019 at 20:47, on Zulip):

do i dare log an issue saying that I need to investigate whether kill_borrows_on_place is secretly buggy...

Dylan MacKenzie (Jun 20 2019 at 20:48, on Zulip):

instead of copying the qualifications from _1 (which is slow)

Dylan MacKenzie (Jun 20 2019 at 20:49, on Zulip):

Also, this particular code landed in #56649

pnkfelix (Jun 20 2019 at 20:49, on Zulip):

yeah I was talking about where accumulates_intrablock_state came from

pnkfelix (Jun 20 2019 at 20:50, on Zulip):

(because no one is currently overriding it, so I wanted to see what my movitation was for adding it in the first place)

Dylan MacKenzie (Jun 20 2019 at 20:50, on Zulip):

Ah yes

pnkfelix (Jun 20 2019 at 20:50, on Zulip):

I agree that kill_borrows_on_place originated with PR #56649; that's why I cc'ed @davidtwco above.

pnkfelix (Jun 20 2019 at 20:53, on Zulip):

so, like, the motivating example on the description of issue #46589 looks like something that is operating on a local

pnkfelix (Jun 20 2019 at 20:53, on Zulip):

and thus it would not even get into the code here

pnkfelix (Jun 20 2019 at 20:53, on Zulip):

so the challenge for me is to try to construct an example

pnkfelix (Jun 20 2019 at 20:54, on Zulip):

where it is not operating on a local

pnkfelix (Jun 20 2019 at 20:54, on Zulip):

and where the transfer function here

pnkfelix (Jun 20 2019 at 20:54, on Zulip):

computes an insufficient kill-set

pnkfelix (Jun 20 2019 at 20:54, on Zulip):

and thus we end up with incorrect results in our final analysis

pnkfelix (Jun 20 2019 at 20:54, on Zulip):

(but can I accept that challenge at 22:54 at night ...?)

Dylan MacKenzie (Jun 20 2019 at 20:55, on Zulip):

XD

pnkfelix (Jun 20 2019 at 20:57, on Zulip):

OR ... oh

pnkfelix (Jun 20 2019 at 20:57, on Zulip):

does this just cause us to reject code

pnkfelix (Jun 20 2019 at 20:57, on Zulip):

that we would otherwise accept ...?

pnkfelix (Jun 20 2019 at 20:57, on Zulip):

/me thinks

pnkfelix (Jun 20 2019 at 20:57, on Zulip):

that could be a reason why no one has filed a bug about this

Dylan MacKenzie (Jun 20 2019 at 20:57, on Zulip):

I have about two weeks of experience working with MIR so I'm not gonna be very helpful I'm afraid :

pnkfelix (Jun 20 2019 at 20:58, on Zulip):

consider this example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6c1e275769db89f67966969f5bcee308

pnkfelix (Jun 20 2019 at 20:58, on Zulip):

(at this point I'm just talking out loud)

Dylan MacKenzie (Jun 20 2019 at 20:58, on Zulip):

but I'm gonna try to understand!

pnkfelix (Jun 20 2019 at 20:58, on Zulip):

there are two functions there

Dylan MacKenzie (Jun 20 2019 at 20:58, on Zulip):

Since I'm west coast US, and thus currently of sound mind

pnkfelix (Jun 20 2019 at 20:58, on Zulip):

to_refs1 and to_refs2

pnkfelix (Jun 20 2019 at 20:58, on Zulip):

adapted from #46589

pnkfelix (Jun 20 2019 at 20:59, on Zulip):

they are almost identical

pnkfelix (Jun 20 2019 at 20:59, on Zulip):

to_refs2 is just wrapping a unary tuple, (T,), around the given typee

Dylan MacKenzie (Jun 20 2019 at 20:59, on Zulip):

Except you're borrowing a projection instead of a bare local

pnkfelix (Jun 20 2019 at 20:59, on Zulip):

that is a hack to force us to go down the non-local path

pnkfelix (Jun 20 2019 at 20:59, on Zulip):

and the compiler rejects to_refs2

pnkfelix (Jun 20 2019 at 21:00, on Zulip):

because, if we under-estimate the kill-set for borrows

pnkfelix (Jun 20 2019 at 21:01, on Zulip):

then we end up with a over-approximate fixed point, right?

pnkfelix (Jun 20 2019 at 21:01, on Zulip):

as in, we end up with a fixed point that has more borrows in it than is strictly necessary .... I think

pnkfelix (Jun 20 2019 at 21:01, on Zulip):

but I'll definitely need to think more carefully about that

pnkfelix (Jun 20 2019 at 21:01, on Zulip):

(This is definitely getting into the weeds about the particular details of this one specific dataflow analysis)

pnkfelix (Jun 20 2019 at 21:02, on Zulip):

anyway I have enough information now to at least file a bug

pnkfelix (Jun 20 2019 at 21:02, on Zulip):

that I can follow up on in the future

pnkfelix (Jun 20 2019 at 21:02, on Zulip):

I would personally be interested in exploring the refactoring you spoke of earlier

pnkfelix (Jun 20 2019 at 21:02, on Zulip):

where you said you removed the on_entry from the API for statement_effect

pnkfelix (Jun 20 2019 at 21:02, on Zulip):

(or at least I think that's what you said)

pnkfelix (Jun 20 2019 at 21:03, on Zulip):

and if this Borrows code is the only thing stopping you

Dylan MacKenzie (Jun 20 2019 at 21:03, on Zulip):

Yes, I tried that as part of https://github.com/rust-lang/rust/pull/61787

pnkfelix (Jun 20 2019 at 21:03, on Zulip):

right, okay

pnkfelix (Jun 20 2019 at 21:04, on Zulip):

i'd be curious to explore doing it

pnkfelix (Jun 20 2019 at 21:04, on Zulip):

namely, I think an interesting experiment

pnkfelix (Jun 20 2019 at 21:04, on Zulip):

would be to change this Borrows code

pnkfelix (Jun 20 2019 at 21:04, on Zulip):

to stop reading on_entry

pnkfelix (Jun 20 2019 at 21:04, on Zulip):

and just act as if on_entry is always the null-set.

pnkfelix (Jun 20 2019 at 21:04, on Zulip):

I'm speaking in particular about kill_borrows_on_place

Dylan MacKenzie (Jun 20 2019 at 21:05, on Zulip):

But didn't we decide that on_entry will always be the null-set?

pnkfelix (Jun 20 2019 at 21:05, on Zulip):

but I can experiment with that on my own time tomorrow.

pnkfelix (Jun 20 2019 at 21:05, on Zulip):

But didn't we decide that on_entry will always be the null-set?

I haven't proven that to myself yet

Dylan MacKenzie (Jun 20 2019 at 21:05, on Zulip):

Since accumulates_intrablock_state is false

pnkfelix (Jun 20 2019 at 21:05, on Zulip):

I hope its the case

Dylan MacKenzie (Jun 20 2019 at 21:05, on Zulip):

except in state_for_location that is XD

pnkfelix (Jun 20 2019 at 21:06, on Zulip):

I'm going to file a bug about this business, and assign it to myself

pnkfelix (Jun 20 2019 at 21:06, on Zulip):

hopefully I will get a chance to look more into it tomorrow.

Dylan MacKenzie (Jun 20 2019 at 21:06, on Zulip):

But didn't we decide that on_entry will always be the null-set?

That's a royal we I guess

Dylan MacKenzie (Jun 20 2019 at 21:06, on Zulip):

Okay, thanks for having a look

pnkfelix (Jun 20 2019 at 21:24, on Zulip):

okay, filed #62007 as a follow-up issue.

pnkfelix (Jun 20 2019 at 21:47, on Zulip):

I suppose I should have kept talking in here

pnkfelix (Jun 20 2019 at 21:48, on Zulip):

since I suspect we're coming to similar conclusions at the same time

pnkfelix (Jun 20 2019 at 21:48, on Zulip):

I am not at the moment sure exactly what the local_map holds.

pnkfelix (Jun 20 2019 at 21:49, on Zulip):

but a cursory review of the code makes me think that it actually does cover all places that have a local at their base

pnkfelix (Jun 20 2019 at 21:49, on Zulip):

(and therefore the idea has merit)

Dylan MacKenzie (Jun 20 2019 at 21:49, on Zulip):

Yeah, all I saw was that it calls base_local which (I believe) iterates through all field projections until it finds the base

pnkfelix (Jun 20 2019 at 21:50, on Zulip):

I need to hit the sack

Dylan MacKenzie (Jun 20 2019 at 21:50, on Zulip):

(Not sure how derefs are handled)

pnkfelix (Jun 20 2019 at 21:50, on Zulip):

but I'll try to look at this more tomorrow

Dylan MacKenzie (Jun 20 2019 at 21:50, on Zulip):

I'll make the change I have in mind in the meantime

pnkfelix (Jun 20 2019 at 21:51, on Zulip):

(Not sure how derefs are handled)

currently, they're just another Projection

pnkfelix (Jun 20 2019 at 21:51, on Zulip):

so they're handled the same way as place.field or place[index]

Dylan MacKenzie (Jun 20 2019 at 21:53, on Zulip):

Okay, well I'll see what fails and maybe that will let you work a bit faster

Dylan MacKenzie (Jul 09 2019 at 23:40, on Zulip):

I started working on this again and have a functioning prototype in #62547.

Last update: Nov 22 2019 at 04:30UTC