From f040210b31987e8fbe799fb1b7feeda3ab3ccc56 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 28 Sep 2023 22:15:51 +0000 Subject: [PATCH 1/9] Remove outdated comment There is no `reset` anymore --- compiler/rustc_middle/src/mir/traversal.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index ec16a8470c412..fd11bd0c1ac4b 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -249,8 +249,7 @@ pub fn postorder<'a, 'tcx>( /// /// Construction of a `ReversePostorder` traversal requires doing a full /// postorder traversal of the graph, therefore this traversal should be -/// constructed as few times as possible. Use the `reset` method to be able -/// to re-use the traversal +/// constructed as few times as possible. #[derive(Clone)] pub struct ReversePostorder<'a, 'tcx> { body: &'a Body<'tcx>, From 82e251be7da92f9f7ddfcb511b11aa8a1c33f98e Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 28 Sep 2023 22:17:13 +0000 Subject: [PATCH 2/9] Remove `ReversePostorder` altogether It was not used anywhere, instead we directly reverse postorder. --- compiler/rustc_middle/src/mir/traversal.rs | 58 ---------------------- 1 file changed, 58 deletions(-) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index fd11bd0c1ac4b..ab7b3f26d5555 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -226,64 +226,6 @@ pub fn postorder<'a, 'tcx>( reverse_postorder(body).rev() } -/// Reverse postorder traversal of a graph -/// -/// Reverse postorder is the reverse order of a postorder traversal. -/// This is different to a preorder traversal and represents a natural -/// linearization of control-flow. -/// -/// ```text -/// -/// A -/// / \ -/// / \ -/// B C -/// \ / -/// \ / -/// D -/// ``` -/// -/// A reverse postorder traversal of this graph is either `A B C D` or `A C B D` -/// Note that for a graph containing no loops (i.e., A DAG), this is equivalent to -/// a topological sort. -/// -/// Construction of a `ReversePostorder` traversal requires doing a full -/// postorder traversal of the graph, therefore this traversal should be -/// constructed as few times as possible. -#[derive(Clone)] -pub struct ReversePostorder<'a, 'tcx> { - body: &'a Body<'tcx>, - blocks: Vec, - idx: usize, -} - -impl<'a, 'tcx> ReversePostorder<'a, 'tcx> { - pub fn new(body: &'a Body<'tcx>, root: BasicBlock) -> ReversePostorder<'a, 'tcx> { - let blocks: Vec<_> = Postorder::new(&body.basic_blocks, root).map(|(bb, _)| bb).collect(); - let len = blocks.len(); - ReversePostorder { body, blocks, idx: len } - } -} - -impl<'a, 'tcx> Iterator for ReversePostorder<'a, 'tcx> { - type Item = (BasicBlock, &'a BasicBlockData<'tcx>); - - fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { - if self.idx == 0 { - return None; - } - self.idx -= 1; - - self.blocks.get(self.idx).map(|&bb| (bb, &self.body[bb])) - } - - fn size_hint(&self) -> (usize, Option) { - (self.idx, Some(self.idx)) - } -} - -impl<'a, 'tcx> ExactSizeIterator for ReversePostorder<'a, 'tcx> {} - /// Returns an iterator over all basic blocks reachable from the `START_BLOCK` in no particular /// order. /// From fb0e58596f9c504466676b70d1c93c910d6ba9ae Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 28 Sep 2023 22:28:09 +0000 Subject: [PATCH 3/9] Simplify `Postorder::next` --- compiler/rustc_middle/src/mir/traversal.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index ab7b3f26d5555..b2d0d83240b95 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -192,12 +192,10 @@ impl<'a, 'tcx> Iterator for Postorder<'a, 'tcx> { type Item = (BasicBlock, &'a BasicBlockData<'tcx>); fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { - let next = self.visit_stack.pop(); - if next.is_some() { - self.traverse_successor(); - } - - next.map(|(bb, _)| (bb, &self.basic_blocks[bb])) + let (bb, _) = self.visit_stack.pop()?; + self.traverse_successor(); + + Some((bb, &self.basic_blocks[bb])) } fn size_hint(&self) -> (usize, Option) { From 0d8a45813c52c33790648b9401428087f0ecb5d2 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 28 Sep 2023 22:30:31 +0000 Subject: [PATCH 4/9] `(&mut iter)` -> `iter.by_ref()` --- compiler/rustc_middle/src/mir/traversal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index b2d0d83240b95..07baaa6f90c96 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -237,7 +237,7 @@ pub fn reachable<'a, 'tcx>( /// Returns a `BitSet` containing all basic blocks reachable from the `START_BLOCK`. pub fn reachable_as_bitset(body: &Body<'_>) -> BitSet { let mut iter = preorder(body); - (&mut iter).for_each(drop); + iter.by_ref().for_each(drop); iter.visited } From a7f3c4e6086c70f1c617bae6c157d5dc5636670f Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 28 Sep 2023 22:39:53 +0000 Subject: [PATCH 5/9] Don't resolve basic block data in `Postorder` The only usage immediately throws out the data, so. --- compiler/rustc_middle/src/mir/basic_blocks.rs | 3 +-- compiler/rustc_middle/src/mir/traversal.rs | 10 +++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_middle/src/mir/basic_blocks.rs b/compiler/rustc_middle/src/mir/basic_blocks.rs index cd770c395e4d3..c0f02c64bbf62 100644 --- a/compiler/rustc_middle/src/mir/basic_blocks.rs +++ b/compiler/rustc_middle/src/mir/basic_blocks.rs @@ -66,8 +66,7 @@ impl<'tcx> BasicBlocks<'tcx> { #[inline] pub fn reverse_postorder(&self) -> &[BasicBlock] { self.cache.reverse_postorder.get_or_init(|| { - let mut rpo: Vec<_> = - Postorder::new(&self.basic_blocks, START_BLOCK).map(|(bb, _)| bb).collect(); + let mut rpo: Vec<_> = Postorder::new(&self.basic_blocks, START_BLOCK).collect(); rpo.reverse(); rpo }) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index 07baaa6f90c96..a89dbf991f0c2 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -188,14 +188,14 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> { } } -impl<'a, 'tcx> Iterator for Postorder<'a, 'tcx> { - type Item = (BasicBlock, &'a BasicBlockData<'tcx>); +impl<'tcx> Iterator for Postorder<'_, 'tcx> { + type Item = BasicBlock; - fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { + fn next(&mut self) -> Option { let (bb, _) = self.visit_stack.pop()?; self.traverse_successor(); - - Some((bb, &self.basic_blocks[bb])) + + Some(bb) } fn size_hint(&self) -> (usize, Option) { From e0abb98e2120903fe3a1b787914e0092150d88da Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 28 Sep 2023 23:11:15 +0000 Subject: [PATCH 6/9] Remove unnecessary `&mut/ref mut` pair --- compiler/rustc_middle/src/mir/traversal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index a89dbf991f0c2..cc189a2cd97d9 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -178,7 +178,7 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> { // When we yield `C` and call `traverse_successor`, we push `B` to the stack, but // since we've already visited `E`, that child isn't added to the stack. The last // two iterations yield `B` and finally `A` for a final traversal of [E, D, C, B, A] - while let Some(&mut (_, ref mut iter)) = self.visit_stack.last_mut() && let Some(bb) = iter.next_back() { + while let Some((_, iter)) = self.visit_stack.last_mut() && let Some(bb) = iter.next_back() { if self.visited.insert(bb) { if let Some(term) = &self.basic_blocks[bb].terminator { self.visit_stack.push((bb, term.successors())); From 0e0dc59acbdc4671c1fdcaf6c06fd9c4d1d8c9e2 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 28 Sep 2023 23:16:48 +0000 Subject: [PATCH 7/9] Use `and_then` instead of while let chain to clarify `iter` scope --- compiler/rustc_middle/src/mir/traversal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index cc189a2cd97d9..28b05f2524f02 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -178,7 +178,7 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> { // When we yield `C` and call `traverse_successor`, we push `B` to the stack, but // since we've already visited `E`, that child isn't added to the stack. The last // two iterations yield `B` and finally `A` for a final traversal of [E, D, C, B, A] - while let Some((_, iter)) = self.visit_stack.last_mut() && let Some(bb) = iter.next_back() { + while let Some(bb) = self.visit_stack.last_mut().and_then(|(_, iter)| iter.next_back()) { if self.visited.insert(bb) { if let Some(term) = &self.basic_blocks[bb].terminator { self.visit_stack.push((bb, term.successors())); From 27242437c7315b8fd456e310fe767a152cb434cd Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 28 Sep 2023 23:50:56 +0000 Subject: [PATCH 8/9] Reverse postorder instead of using reversed postorder --- src/tools/clippy/clippy_utils/src/mir/mod.rs | 32 ++++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/tools/clippy/clippy_utils/src/mir/mod.rs b/src/tools/clippy/clippy_utils/src/mir/mod.rs index f04467dc19d37..9dbb4c68d13f8 100644 --- a/src/tools/clippy/clippy_utils/src/mir/mod.rs +++ b/src/tools/clippy/clippy_utils/src/mir/mod.rs @@ -30,20 +30,26 @@ pub fn visit_local_usage(locals: &[Local], mir: &Body<'_>, location: Location) - locals.len() ]; - traversal::ReversePostorder::new(mir, location.block).try_fold(init, |usage, (tbb, tdata)| { - // Give up on loops - if tdata.terminator().successors().any(|s| s == location.block) { - return None; - } + traversal::Postorder::new(&mir.basic_blocks, location.block) + .collect::>() + .into_iter() + .rev() + .try_fold(init, |usage, tbb| { + let tdata = &mir.basic_blocks[tbb]; + + // Give up on loops + if tdata.terminator().successors().any(|s| s == location.block) { + return None; + } - let mut v = V { - locals, - location, - results: usage, - }; - v.visit_basic_block_data(tbb, tdata); - Some(v.results) - }) + let mut v = V { + locals, + location, + results: usage, + }; + v.visit_basic_block_data(tbb, tdata); + Some(v.results) + }) } struct V<'a> { From 814fbd89b69173a3f2189c739dd32921702c5eca Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 29 Sep 2023 09:58:53 +0000 Subject: [PATCH 9/9] Remove deleted docs + better link together MIR traversing docs --- compiler/rustc_middle/src/mir/basic_blocks.rs | 4 +++ compiler/rustc_middle/src/mir/traversal.rs | 35 +++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/mir/basic_blocks.rs b/compiler/rustc_middle/src/mir/basic_blocks.rs index c0f02c64bbf62..3ecd5b9cd3456 100644 --- a/compiler/rustc_middle/src/mir/basic_blocks.rs +++ b/compiler/rustc_middle/src/mir/basic_blocks.rs @@ -63,6 +63,10 @@ impl<'tcx> BasicBlocks<'tcx> { } /// Returns basic blocks in a reverse postorder. + /// + /// See [`traversal::reverse_postorder`]'s docs to learn what is preorder traversal. + /// + /// [`traversal::reverse_postorder`]: crate::mir::traversal::reverse_postorder #[inline] pub fn reverse_postorder(&self) -> &[BasicBlock] { self.cache.reverse_postorder.get_or_init(|| { diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index 28b05f2524f02..a1ff8410eac4a 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -41,6 +41,12 @@ impl<'a, 'tcx> Preorder<'a, 'tcx> { } } +/// Preorder traversal of a graph. +/// +/// This function creates an iterator over the `Body`'s basic blocks, that +/// returns basic blocks in a preorder. +/// +/// See [`Preorder`]'s docs to learn what is preorder traversal. pub fn preorder<'a, 'tcx>(body: &'a Body<'tcx>) -> Preorder<'a, 'tcx> { Preorder::new(body, START_BLOCK) } @@ -213,10 +219,14 @@ impl<'tcx> Iterator for Postorder<'_, 'tcx> { } } -/// Creates an iterator over the `Body`'s basic blocks, that: +/// Postorder traversal of a graph. +/// +/// This function creates an iterator over the `Body`'s basic blocks, that: /// - returns basic blocks in a postorder, /// - traverses the `BasicBlocks` CFG cache's reverse postorder backwards, and does not cache the /// postorder itself. +/// +/// See [`Postorder`]'s docs to learn what is postorder traversal. pub fn postorder<'a, 'tcx>( body: &'a Body<'tcx>, ) -> impl Iterator)> + ExactSizeIterator + DoubleEndedIterator @@ -241,9 +251,30 @@ pub fn reachable_as_bitset(body: &Body<'_>) -> BitSet { iter.visited } -/// Creates an iterator over the `Body`'s basic blocks, that: +/// Reverse postorder traversal of a graph. +/// +/// This function creates an iterator over the `Body`'s basic blocks, that: /// - returns basic blocks in a reverse postorder, /// - makes use of the `BasicBlocks` CFG cache's reverse postorder. +/// +/// Reverse postorder is the reverse order of a postorder traversal. +/// This is different to a preorder traversal and represents a natural +/// linearization of control-flow. +/// +/// ```text +/// +/// A +/// / \ +/// / \ +/// B C +/// \ / +/// \ / +/// D +/// ``` +/// +/// A reverse postorder traversal of this graph is either `A B C D` or `A C B D` +/// Note that for a graph containing no loops (i.e., A DAG), this is equivalent to +/// a topological sort. pub fn reverse_postorder<'a, 'tcx>( body: &'a Body<'tcx>, ) -> impl Iterator)> + ExactSizeIterator + DoubleEndedIterator