Generate "invalidates" facts when -Znll-facts is passed #50798
Conversation
this probably doesn't compile and definitely doesn't work
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
| @@ -38,6 +38,9 @@ crate struct AllFacts { | |||
|
|
|||
| // `region_live_at(R, P)` when the region R appears in a live variable at P | |||
| crate region_live_at: Vec<(RegionVid, LocationIndex)>, | |||
|
|
|||
| // `invalidates(P, B)` when the borrow B is invalidated at point P | |||
There was a problem hiding this comment.
man I want to change from Borrow terminology to Loan. =)
| mir_def_id: DefId, | ||
| borrow_set: &BorrowSet<'tcx>, | ||
| ) { | ||
| if !all_facts.is_some() { |
There was a problem hiding this comment.
Nit: I'd probably write:
if let Some(all_facts) = all_facts {
.. // inner stuff
}then you don't, I think, need the call to take() etc. But whatever.
| } => { | ||
| self.consume_operand(ContextKind::Yield.new(location), value); | ||
|
|
||
| // ** FIXME(bob_twinkles) figure out what the equivalent of this is |
There was a problem hiding this comment.
the equivalent here is invalidating the loans at the point after the yield. Basically we want to invalidate all loans that are borrowing "local" content (something owned by current stack frame) at this point:
let resume_index = self.location_table.start_index(resume.start_location())(That is taken from my branch that integrated with the new approach)
| // } | ||
| } | ||
| TerminatorKind::Resume | TerminatorKind::Return | TerminatorKind::GeneratorDrop => { | ||
| // ** FIXME(bob_twinkles) figure out what the equivalent of this is |
There was a problem hiding this comment.
We want to invalidate all loans that refer to content owned by the current stack frame; I'm actually not sure why this uses with_outgoing_borrows, I think for our purposes the current location is fine (self.location_table.start_index(loc)).
That is what I did in my branch that integrates with dataflow, anyway:
| // unique or mutable borrows are invalidated by writes. | ||
| // Reservations count as writes since we need to check | ||
| // that activating the borrow will be OK | ||
| // TOOD(bob_twinkles) is this actually the right thing to do? |
There was a problem hiding this comment.
seems right to me..why are you worried?
nikomatsakis
added
S-waiting-on-author
sapphire-arches
added
S-waiting-on-review
|
@bors r+ |
|
📌 Commit 965eef9 has been approved by |
bors
added
S-waiting-on-bors
Following up my changes in rust-lang#50798, I wanted to grab at least the low-hanging fruit for code deduplication.
sapphire-arches
force-pushed
the
nll_facts_invalidate
branch
from
e62fa27 to
965eef9
Compare
|
@bors r+ |
|
📌 Commit 965eef9 has been approved by |
Generate "invalidates" facts when -Znll-facts is passed Most of the new code is copied directly from the heart of the MIR borrowchecker. I was expecting more fundamental structural changes, hence the copying. This appears to work as it stands, but I'd like to submit a follow-up PR to reduce code duplication. I figured that could wait though, since this is blocking a large amount of work in the borrow check repository and I'm out of time for tonight =). r? @nikomatsakis
|
☀️ Test successful - status-appveyor, status-travis |
Most of the new code is copied directly from the heart of the MIR borrowchecker. I was expecting more fundamental structural changes, hence the copying. This appears to work as it stands, but I'd like to submit a follow-up PR to reduce code duplication. I figured that could wait though, since this is blocking a large amount of work in the borrow check repository and I'm out of time for tonight =).
r? @nikomatsakis