Move analysis for MIR borrowck #32156
Conversation
|
⛄ (yes, this is a positive emoji, don't ask me why) |
|
This basically looks good to me. Left a bunch of nits and minor comments. I'll take a look at the next round and think more deeply. |
|
☔ The latest upstream changes (presumably #31710) made this pull request unmergeable. Please resolve the merge conflicts. |
pnkfelix
force-pushed
the
borrowck-on-mir-move-analysis
branch
2 times, most recently
from
c52a22d
to
47cd841
Compare
|
@nikomatsakis BTW one other thing I only thought while rebasing the code (but did not attempt to implement): instead of the newer approach to statics that I used in f244405, I might instead consider trying to handle abstracting all statics (or all L-values that are built off of a static) into a single abstract entity in the lifting step that is currently in abs_domain.rs But I am not 100% sure that actually makes sense. So I left it at something to think about and tackle later. (Also, something I do need/want to do is to put in actual tests, using the same sort of error-message-based instrumentation that we've used elsewhere to make observations about the compiler's internal view of a piece of code... its possible that upkeep for such tests could be a pain, but we'd only need to keep the tests around until the code is exercised by other things.) |
strong 👍 |
| if let Some((ref dest_lval, ref dest_bb)) = *destination { | ||
| // N.B.: This must be done *last*, after all other | ||
| // propagation, as documented in comment above. | ||
| on_return(&self.operator, in_out, dest_lval); |
There was a problem hiding this comment.
Seems...pretty clean.
|
r=me on the revisions |
|
@bors r+ |
|
📌 Commit 47cd841 has been approved by |
|
⌛ Testing commit 47cd841 with merge ff2b38a... |
|
💔 Test failed - auto-win-msvc-64-opt |
| } | ||
|
|
||
| fn process_statement(&mut self, bb: BasicBlock, stmt: &Statement<'tcx>) { | ||
| debug!("MirBorrowckCtxt::preprocess_statement({:?}, {:?}", bb, stmt); |
There was a problem hiding this comment.
Function name is wrong in the debug message, preprocess vs. process
pnkfelix
force-pushed
the
borrowck-on-mir-move-analysis
branch
5 times, most recently
from
c6b8e36
to
dd99f58
Compare
… at them in borrowck.
emit (via debug!) scary message from `fn borrowck_mir` until basic prototype is in place. Gather children of move paths and set their kill bits in dataflow. (Each node has a link to the child that is first among its siblings.) Hooked in libgraphviz based rendering, including of borrowck dataflow state. doing this well required some refactoring of the code, so I cleaned it up more generally (adding comments to explain what its trying to do and how it is doing it). Update: this newer version addresses most review comments (at least the ones that were largely mechanical changes), but I left the more interesting revisions to separate followup commits (in this same PR).
Instead, create a single MovePathIndex that represents all statics. (An alternative here would be to disallow representing statics at all. I am hesitant to do that right now, in part because it could impose a requirement that I thread checks for static data into the calling code, either as pre- or post-invocation of `fn move_path_for`.)
precursor for a number of other simplifying changes (mostly removing uses of `RefCell`). Factor lookup method out of `fn move_path_for`.
need to be initialized. create a MoveOut to represent that deref'ed `*lval` path.
|
@bors r=nikomatsakis |
|
📌 Commit dd99f58 has been approved by |
|
⌛ Testing commit dd99f58 with merge cde53ae... |
|
💔 Test failed - auto-win-gnu-32-opt-rustbuild |
|
Hmm I assume this is some rustbuild dependency issue; will look |
|
@bors r=nikomatsakis |
|
📌 Commit 782c0cf has been approved by |
…matsakis Move analysis for MIR borrowck This PR adds code for doing MIR-based gathering of the moves in a `fn` and the dataflow to determine where uninitialized locations flow to, analogous to how the same thing is done in `borrowck`. It also adds a couple attributes to print out graphviz visualizations of the analyzed MIR that includes the dataflow analysis results. cc @nikomatsakis
This PR adds code for doing MIR-based gathering of the moves in a
fnand the dataflow to determine where uninitialized locations flow to, analogous to how the same thing is done inborrowck.It also adds a couple attributes to print out graphviz visualizations of the analyzed MIR that includes the dataflow analysis results.
cc @nikomatsakis