-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MIR-borrowck] Two phase borrows #46537
Commits on Dec 13, 2017
-
Revised graphviz rendering API to avoid requiring borrowed state.
Made `do_dataflow` and related API `pub(crate)`.
Configuration menu - View commit details
-
Copy full SHA for 93c4ffe - Browse repository at this point
Copy the full SHA 93c4ffeView commit details -
Expanded HIR
--unpretty hir,identified
to include HIR local id.Having the HIR local id is useful for cases like understanding the ReScope identifiers, which are now derived from the HIR local id, and thus one can map an ReScope back to the HIR node, once one knows what those local ids are.
Configuration menu - View commit details
-
Copy full SHA for 171c2ae - Browse repository at this point
Copy the full SHA 171c2aeView commit details -
Refactoring: pull bitvector initialization out from other parts of da…
…taflow. This is meant to ease development of multi-stage dataflow analyses where the output from one analysis is used to initialize the state for the next; in such a context, you cannot start with `bottom_value` for all the bits.
Configuration menu - View commit details
-
Copy full SHA for d4add5d - Browse repository at this point
Copy the full SHA d4add5dView commit details -
Refactoring: Allow
BlockSets.on_entry
to denote locally accumulated…… intrablock state. (Still musing about whether it could make sense to revise the design here to make these constraints on usage explicit.)
Configuration menu - View commit details
-
Copy full SHA for e123117 - Browse repository at this point
Copy the full SHA e123117View commit details -
Configuration menu - View commit details
-
Copy full SHA for 39e126c - Browse repository at this point
Copy the full SHA 39e126cView commit details -
Configuration menu - View commit details
-
Copy full SHA for e437e49 - Browse repository at this point
Copy the full SHA e437e49View commit details -
Configuration menu - View commit details
-
Copy full SHA for ef64ace - Browse repository at this point
Copy the full SHA ef64aceView commit details -
New
ActiveBorrows
dataflow for two-phase&mut
; not yet borrowed-c……hecked. High-level picture: The old `Borrows` analysis is now called `Reservations` (implemented as a newtype wrapper around `Borrows`); this continues to compute whether a `Rvalue::Ref` can reach a statement without an intervening `EndRegion`. In addition, we also track what `Place` each such `Rvalue::Ref` was immediately assigned to in a given borrow (yay for MIR-structural properties!). The new `ActiveBorrows` analysis then tracks the initial use of any of those assigned `Places` for a given borrow. I.e. a borrow becomes "active" immediately after it starts being "used" in some way. (This is conservative in the sense that we will treat a copy `x = y;` as a use of `y`; in principle one might further delay activation in such cases.) The new `ActiveBorrows` analysis needs to take the `Reservations` results as an initial input, because the reservation state influences the gen/kill sets for `ActiveBorrows`. In particular, a use of `a` activates a borrow `a = &b` if and only if there exists a path (in the control flow graph) from the borrow to that use. So we need to know if the borrow reaches a given use to know if it really gets a gen-bit or not. * Incorporating the output from one dataflow analysis into the input of another required more changes to the infrastructure than I had expected, and even after those changes, the resulting code is still a bit subtle. * In particular, Since we need to know the intrablock reservation state, we need to dynamically update a bitvector for the reservations as we are also trying to compute the gen/kills bitvector for the active borrows. * The way I ended up deciding to do this (after also toying with at least two other designs) is to put both the reservation state and the active borrow state into a single bitvector. That is why we now have separate (but related) `BorrowIndex` and `ReserveOrActivateIndex`: each borrow index maps to a pair of neighboring reservation and activation indexes. As noted above, these changes are solely adding the active borrows dataflow analysis (and updating the existing code to cope with the switch from `Borrows` to `Reservations`). The code to process the bitvector in the borrow checker currently just skips over all of the active borrow bits. But atop this commit, one *can* observe the analysis results by looking at the graphviz output, e.g. via ```rust #[rustc_mir(borrowck_graphviz_preflow="pre_two_phase.dot", borrowck_graphviz_postflow="post_two_phase.dot")] ``` Includes doc for `FindPlaceUses`, as well as `Reservations` and `ActiveBorrows` structs, which are wrappers are the `Borrows` struct that dictate which flow analysis should be performed.
Configuration menu - View commit details
-
Copy full SHA for ced5a70 - Browse repository at this point
Copy the full SHA ced5a70View commit details -
Configuration menu - View commit details
-
Copy full SHA for 658ed79 - Browse repository at this point
Copy the full SHA 658ed79View commit details -
Configuration menu - View commit details
-
Copy full SHA for 1334638 - Browse repository at this point
Copy the full SHA 1334638View commit details -
Incorporate active-borrows dataflow into MIR borrow check, yielding
two-phase `&mut`-borrow support. This (new) support sits under `-Z two-phase-borrows` debugflag. (Still needs tests. That's coming next.)
Configuration menu - View commit details
-
Copy full SHA for 3621645 - Browse repository at this point
Copy the full SHA 3621645View commit details -
the minimal test for two-phase borrows: the core example from niko's …
…blog post on it.
Configuration menu - View commit details
-
Copy full SHA for 5f759a9 - Browse repository at this point
Copy the full SHA 5f759a9View commit details -
Configuration menu - View commit details
-
Copy full SHA for dbbec4d - Browse repository at this point
Copy the full SHA dbbec4dView commit details -
Configuration menu - View commit details
-
Copy full SHA for db5420b - Browse repository at this point
Copy the full SHA db5420bView commit details -
two-phase-reservation-sharing-interference.rs variant that is perhaps…
… more surprising.
Configuration menu - View commit details
-
Copy full SHA for 9cb92ac - Browse repository at this point
Copy the full SHA 9cb92acView commit details -
Configuration menu - View commit details
-
Copy full SHA for 18aedf6 - Browse repository at this point
Copy the full SHA 18aedf6View commit details -
Check activation points as the place where mutable borrows become rel…
…evant. Since we are now checking activation points, I removed one of the checks at the reservation point. (You can see the effect this had on two-phase-reservation-sharing-interference-2.rs) Also, since we now have checks at both the reservation point and the activation point, we sometimes would observe duplicate errors (since either one independently interferes with another mutable borrow). To deal with this, I used a similar strategy to one used as discussed on issue rust-lang#45360: keep a set of errors reported (in this case for reservations), and then avoid doing the checks for the corresponding activations. (This does mean that some errors could get masked, namely for conflicting borrows that start after the reservation but still conflict with the activation, which is unchecked when there was an error for the reservation. But this seems like a reasonable price to pay.)
Configuration menu - View commit details
-
Copy full SHA for 5cae7a0 - Browse repository at this point
Copy the full SHA 5cae7a0View commit details
Commits on Dec 14, 2017
-
Address review comment: use
.get
instead of indexing to cope w/ ter……minators. (Same net effect as code from before; just cleaner way to get there.)
Configuration menu - View commit details
-
Copy full SHA for 3c7d9ff - Browse repository at this point
Copy the full SHA 3c7d9ffView commit details -
After discussion with ariel, replacing a guard within kill_loans_out_…
…of_scope_at_location. Instead we are "just" careful to invoke it (which sets up a bunch of kill bits) before we go into the code that sets up the gen bits. That way, when the gen bits are set up, they will override any previously set kill-bits for those reservations or activations.
Configuration menu - View commit details
-
Copy full SHA for f96777c - Browse repository at this point
Copy the full SHA f96777cView commit details -
Address review note:
AccessErrorsReported
meant to track whether er……ror reported at *any* point in past.
Configuration menu - View commit details
-
Copy full SHA for b75248e - Browse repository at this point
Copy the full SHA b75248eView commit details -
Configuration menu - View commit details
-
Copy full SHA for b0421fa - Browse repository at this point
Copy the full SHA b0421faView commit details -
Review feedback: Added test with control flow merge of two borrows "b…
…efore activation" In reality the currently generated MIR has at least one of the activations in a copy that occurs before the merge. But still, good to have a test, in anticipation of that potentially changing...
Configuration menu - View commit details
-
Copy full SHA for d654cd3 - Browse repository at this point
Copy the full SHA d654cd3View commit details -
Address review feedback: don't treat "first" activation special.
Instead, filter out (non-)conflicts of activiations with themselves in the same manner that we filter out non-conflict between an activation and its reservation.
Configuration menu - View commit details
-
Copy full SHA for 159037e - Browse repository at this point
Copy the full SHA 159037eView commit details