remove visit_terminator_kind from MIR visitor #72814
Conversation
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
RalfJung
force-pushed
the
mir-visir-terminator
branch
from
6de019c
to
fef7702
Compare
| @@ -84,6 +88,8 @@ impl<'visit, 'cx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'cx, 'tc | |||
| ); | |||
| self.remove_never_initialized_mut_locals(*into); | |||
| } | |||
|
|
|||
| // FIXME: no super_statement? | |||
There was a problem hiding this comment.
I didn't change anything here, but it seems rather suspicious that this does not call super_statement -- in particular since there can be uses of locals inside statements, and this visitor does use visit_local.
There was a problem hiding this comment.
This looks like a bug
There was a problem hiding this comment.
@matthewjasper I added this now, at the end of this function. I was not sure if beginning or end was appropriate though.
There was a problem hiding this comment.
If this is a bug, wouldn't it make sense to add a test to cover this case?.
There was a problem hiding this comment.
I have no idea what this code is doing or where it is invoked. I can do this as a drive-by fix, but I can't spend further research time on this to figure out enough to be able to write a testcase.
If someone wants to take over and do that, that would be great. :D
There was a problem hiding this comment.
The logic in this area is a bit of a mess and is somewhat redundant. The actual test is being able to do some of that clean up without breaking the existing tests.
There was a problem hiding this comment.
Well, that seems to work.^^
RalfJung
force-pushed
the
mir-visir-terminator
branch
from
5e40fa7
to
76e5fd5
Compare
|
About the missing super calls, I'm not sure if those were accidentally left out or if those methods are intended as a let's overwrite this method. To be honest I'm not sure cc @oli-obk |
This comment has been minimized.
This comment has been minimized.
RalfJung
force-pushed
the
mir-visir-terminator
branch
from
4f01cbb
to
704a063
Compare
This comment has been minimized.
This comment has been minimized.
RalfJung
force-pushed
the
mir-visir-terminator
branch
from
704a063
to
827ccf7
Compare
|
Rebased. @matthewjasper @eddyb given the frequency of conflicts, would be nice to get this reviewed soon-ish. :) Cc @oli-obk maybe they can help. |
|
📌 Commit 827ccf7 has been approved by |
bors
added
S-waiting-on-bors
…i-obk remove visit_terminator_kind from MIR visitor For some reason, we had both `visit_terminator` and `visit_terminator_kind`. In contrast, for `Statement` we just have `visit_statement`. So this cleans things up by removing `visit_terminator_kind` and porting its users to `visit_terminator`.
…i-obk remove visit_terminator_kind from MIR visitor For some reason, we had both `visit_terminator` and `visit_terminator_kind`. In contrast, for `Statement` we just have `visit_statement`. So this cleans things up by removing `visit_terminator_kind` and porting its users to `visit_terminator`.
…i-obk remove visit_terminator_kind from MIR visitor For some reason, we had both `visit_terminator` and `visit_terminator_kind`. In contrast, for `Statement` we just have `visit_statement`. So this cleans things up by removing `visit_terminator_kind` and porting its users to `visit_terminator`.
…arth Rollup of 17 pull requests Successful merges: - rust-lang#70551 (Make all uses of ty::Error delay a span bug) - rust-lang#71338 (Expand "recursive opaque type" diagnostic) - rust-lang#71976 (Improve diagnostics for `let x += 1`) - rust-lang#72279 (add raw_ref macros) - rust-lang#72628 (Add tests for 'impl Default for [T; N]') - rust-lang#72804 (Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position) - rust-lang#72814 (remove visit_terminator_kind from MIR visitor) - rust-lang#72836 (Complete the std::time documentation to warn about the inconsistencies between OS) - rust-lang#72968 (Only highlight doc search results via mouseover if mouse has moved) - rust-lang#73034 (Export `#[inline]` fns with extern indicators) - rust-lang#73315 (Clean up some weird command strings) - rust-lang#73320 (Make new type param suggestion more targetted) - rust-lang#73361 (Tweak "non-primitive cast" error) - rust-lang#73425 (Mention functions pointers in the documentation) - rust-lang#73428 (Fix typo in librustc_ast docs) - rust-lang#73447 (Improve document for `Result::as_deref(_mut)` methods) - rust-lang#73476 (Added tooltip for should_panic code examples) Failed merges: r? @ghost
For some reason, we had both
visit_terminatorandvisit_terminator_kind. In contrast, forStatementwe just havevisit_statement. So this cleans things up by removingvisit_terminator_kindand porting its users tovisit_terminator.