Convert typeck constraints in location-sensitive polonius

[lqd]

In this PR, we do a big chunk of the work of localizing regular outlives constraints.

The slightly annoying thing is handling effectful statements: usually the subset graph propagates loans at a single point between regions, and liveness propagates loans between points within a single region, but some statements have effects applied on exit.

This was also a problem before, in datalog polonius terms and Niko's solution at the time, this is about: the mid-point. The idea was to duplicate all MIR locations into two physical points, and orchestrate the effects with that. Somewhat easier to do, but double the CFG.

We've always believed we didn't need midpoints in principle, as we can represent changes on exit as on happening entry to the successor, but there's some difficulty in tracking the position information at sufficient granularity through outlives relation (especially since we also have bidirectional edges and time-traveling now).

Now, that is surely what we should be doing in the future. In the mean time, I infer this from the kind of statement/terminator where an outlives constraint arose. It's not particularly complicated but some explanation will help clarify the code.

Assignments (in their various forms) are the quintessential example of these crossover cases: loans that would flow into the LHS would not be visible on entry to the point but on exit -- so we'll localize these edges to the successor. Let's look at a real-world example, involving invariance for bidirectional edges:

let mut _1: HashMap<i32, &'7 i32>;
let mut _3: &'9 mut HashMap<i32, &'10 i32>;
...
/* at bb1[3]: */ _3 = &'3 mut _1;

Here, typeck expectedly produces 3 outlives constraints today:

1. '3 -> '9
2. '7 -> '10
3. '10 -> '7

And we localize them like so,

1. '3 -> '9 flows into the LHS and becomes: 3_bb1_3 -> 9_bb1_4
2. '7 -> '10 flows into the LHS and becomes: 7_bb1_3 -> 10_bb1_4
3. '10 -> '7 flows from the LHS and becomes: 10_bb1_4 -> 7_bb1_3 (time traveling 👌)

---

r? @jackh726

To keep you entertained during the holidays I also threw in a couple of small changes removing cruft in the borrow checker.

We're actually getting there. The next PR will be the last one needed to get end-to-end tests working.

[jackh726]

I think this is right, but it feels off to me. The loop using for_each_free_region feels like the wrong construct to me.

[lqd]

I'd also prefer if this differentiation was done in MIR typeck instead, if that's what you mean. It wasn't trivial in the NLL days, according to Niko and some comments in the crate, but should still be possible I think?

[jackh726]

I think given the FIXME further up in the file, we don't need to do anything in this PR, just wanted to point out that it sure does feel weird.

[jackh726]

I wonder if readability would be better here if you moved the LocalizedOutlivesConstraint construction deeper (into compute_constraint_direction), instead of propagating up a (PointIndex, PointIndex). It'll then be much more clear the matching of sup/sub to from/to.

[lqd]

I initially has something similar indeed, but the duplication was a bit much and I turned it into this. I can bring it back.

I've pushed a commit to do so, let me know what you think (especially if you didn't mean to also inline the helper function compute_constraint_direction) and I can then squash it in. Inlining the helper makes the link between the sub/sup and from/to clearer with the actual result type we're checking.

[jackh726]

To add to this, are these actually useful? (I think I remember that these get filtered somewhere? Or was that something else?)

If these aren't useful, then you can make this branch empty.

[lqd]

(I think I remember that these get filtered somewhere? Or was that something else?)

I think that may be something else? Only an edge from and to the same node is filtered out, if that's what you remembered: if the regions are different but from/to the same point, they're not filtered out.

To add to this, are these actually useful?

I think handling some statements should be needed in addition to assignments, like maybe fake reads and type ascriptions (but the latter should hold everywhere and not go through this code path here, but the Locations::All path).

I'll try to look into it soon; I don't think we'll have tests that exercise this anyways.

[jackh726]

Okay, I do still even if these are needed that duplicating the LocalizedOutlivesConstraint (instead of propagating up the PointIndex tuple will improve readability.

[jackh726]

r=me with or without moving the LocalizedOutlivesConstraint construction deeper

[jackh726]

@bors r+

[bors]

📌 Commit fc7ee23 has been approved by jackh726

It is now in the queue for this repository.

[lqd]

This is feature gated and off by default, and should be safe to rollup.

@bors rollup