about summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--compiler/rustc_borrowck/src/invalidation.rs2
-rw-r--r--compiler/rustc_borrowck/src/lib.rs9
-rw-r--r--compiler/rustc_codegen_ssa/src/back/linker.rs24
-rw-r--r--compiler/rustc_codegen_ssa/src/mir/analyze.rs2
-rw-r--r--compiler/rustc_data_structures/src/graph/dominators/mod.rs95
-rw-r--r--compiler/rustc_middle/src/mir/basic_blocks.rs5
-rw-r--r--compiler/rustc_mir_transform/src/coverage/graph.rs29
-rw-r--r--compiler/rustc_mir_transform/src/coverage/spans.rs2
-rw-r--r--compiler/rustc_mir_transform/src/ctfe_limit.rs2
-rw-r--r--compiler/rustc_mir_transform/src/ssa.rs12
-rw-r--r--compiler/rustc_privacy/src/lib.rs263
11 files changed, 219 insertions, 226 deletions
diff --git a/compiler/rustc_borrowck/src/invalidation.rs b/compiler/rustc_borrowck/src/invalidation.rs
index 863c92acdf4..036391d074d 100644
--- a/compiler/rustc_borrowck/src/invalidation.rs
+++ b/compiler/rustc_borrowck/src/invalidation.rs
@@ -46,7 +46,7 @@ struct InvalidationGenerator<'cx, 'tcx> {
     all_facts: &'cx mut AllFacts,
     location_table: &'cx LocationTable,
     body: &'cx Body<'tcx>,
-    dominators: Dominators<BasicBlock>,
+    dominators: &'cx Dominators<BasicBlock>,
     borrow_set: &'cx BorrowSet<'tcx>,
 }
 
diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs
index eb25d454339..43c4e1a9d6f 100644
--- a/compiler/rustc_borrowck/src/lib.rs
+++ b/compiler/rustc_borrowck/src/lib.rs
@@ -43,7 +43,6 @@ use rustc_target::abi::FieldIdx;
 
 use either::Either;
 use smallvec::SmallVec;
-use std::cell::OnceCell;
 use std::cell::RefCell;
 use std::collections::BTreeMap;
 use std::ops::Deref;
@@ -331,7 +330,6 @@ fn do_mir_borrowck<'tcx>(
                 used_mut: Default::default(),
                 used_mut_upvars: SmallVec::new(),
                 borrow_set: Rc::clone(&borrow_set),
-                dominators: Default::default(),
                 upvars: Vec::new(),
                 local_names: IndexVec::from_elem(None, &promoted_body.local_decls),
                 region_names: RefCell::default(),
@@ -360,7 +358,6 @@ fn do_mir_borrowck<'tcx>(
         used_mut: Default::default(),
         used_mut_upvars: SmallVec::new(),
         borrow_set: Rc::clone(&borrow_set),
-        dominators: Default::default(),
         upvars,
         local_names,
         region_names: RefCell::default(),
@@ -591,9 +588,6 @@ struct MirBorrowckCtxt<'cx, 'tcx> {
     /// The set of borrows extracted from the MIR
     borrow_set: Rc<BorrowSet<'tcx>>,
 
-    /// Dominators for MIR
-    dominators: OnceCell<Dominators<BasicBlock>>,
-
     /// Information about upvars not necessarily preserved in types or MIR
     upvars: Vec<Upvar<'tcx>>,
 
@@ -2269,7 +2263,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
     }
 
     fn dominators(&self) -> &Dominators<BasicBlock> {
-        self.dominators.get_or_init(|| self.body.basic_blocks.dominators())
+        // `BasicBlocks` computes dominators on-demand and caches them.
+        self.body.basic_blocks.dominators()
     }
 }
 
diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs
index 1e57f4248d2..cd56f85cccd 100644
--- a/compiler/rustc_codegen_ssa/src/back/linker.rs
+++ b/compiler/rustc_codegen_ssa/src/back/linker.rs
@@ -144,7 +144,7 @@ pub fn get_linker<'a>(
             cmd,
             sess,
             target_cpu,
-            hinted_static: false,
+            hinted_static: None,
             is_ld: cc == Cc::No,
             is_gnu: flavor.is_gnu(),
         }) as Box<dyn Linker>,
@@ -214,7 +214,7 @@ pub struct GccLinker<'a> {
     cmd: Command,
     sess: &'a Session,
     target_cpu: &'a str,
-    hinted_static: bool, // Keeps track of the current hinting mode.
+    hinted_static: Option<bool>, // Keeps track of the current hinting mode.
     // Link as ld
     is_ld: bool,
     is_gnu: bool,
@@ -275,9 +275,9 @@ impl<'a> GccLinker<'a> {
         if !self.takes_hints() {
             return;
         }
-        if !self.hinted_static {
+        if self.hinted_static != Some(true) {
             self.linker_arg("-Bstatic");
-            self.hinted_static = true;
+            self.hinted_static = Some(true);
         }
     }
 
@@ -285,9 +285,9 @@ impl<'a> GccLinker<'a> {
         if !self.takes_hints() {
             return;
         }
-        if self.hinted_static {
+        if self.hinted_static != Some(false) {
             self.linker_arg("-Bdynamic");
-            self.hinted_static = false;
+            self.hinted_static = Some(false);
         }
     }
 
@@ -1484,25 +1484,25 @@ impl<'a> L4Bender<'a> {
 pub struct AixLinker<'a> {
     cmd: Command,
     sess: &'a Session,
-    hinted_static: bool,
+    hinted_static: Option<bool>,
 }
 
 impl<'a> AixLinker<'a> {
     pub fn new(cmd: Command, sess: &'a Session) -> AixLinker<'a> {
-        AixLinker { cmd: cmd, sess: sess, hinted_static: false }
+        AixLinker { cmd: cmd, sess: sess, hinted_static: None }
     }
 
     fn hint_static(&mut self) {
-        if !self.hinted_static {
+        if self.hinted_static != Some(true) {
             self.cmd.arg("-bstatic");
-            self.hinted_static = true;
+            self.hinted_static = Some(true);
         }
     }
 
     fn hint_dynamic(&mut self) {
-        if self.hinted_static {
+        if self.hinted_static != Some(false) {
             self.cmd.arg("-bdynamic");
-            self.hinted_static = false;
+            self.hinted_static = Some(false);
         }
     }
 
diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs
index 569599faa36..835074806e9 100644
--- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs
@@ -84,7 +84,7 @@ impl DefLocation {
 
 struct LocalAnalyzer<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
     fx: &'mir FunctionCx<'a, 'tcx, Bx>,
-    dominators: Dominators<mir::BasicBlock>,
+    dominators: &'mir Dominators<mir::BasicBlock>,
     locals: IndexVec<mir::Local, LocalKind>,
 }
 
diff --git a/compiler/rustc_data_structures/src/graph/dominators/mod.rs b/compiler/rustc_data_structures/src/graph/dominators/mod.rs
index 594ed1ad2e7..a5db14d9102 100644
--- a/compiler/rustc_data_structures/src/graph/dominators/mod.rs
+++ b/compiler/rustc_data_structures/src/graph/dominators/mod.rs
@@ -26,7 +26,7 @@ rustc_index::newtype_index! {
     struct PreorderIndex {}
 }
 
-pub fn dominators<G: ControlFlowGraph>(graph: G) -> Dominators<G::Node> {
+pub fn dominators<G: ControlFlowGraph>(graph: &G) -> Dominators<G::Node> {
     // compute the post order index (rank) for each node
     let mut post_order_rank = IndexVec::from_elem_n(0, graph.num_nodes());
 
@@ -244,7 +244,10 @@ pub fn dominators<G: ControlFlowGraph>(graph: G) -> Dominators<G::Node> {
 
     let start_node = graph.start_node();
     immediate_dominators[start_node] = None;
-    Dominators { start_node, post_order_rank, immediate_dominators }
+
+    let time = compute_access_time(start_node, &immediate_dominators);
+
+    Dominators { start_node, post_order_rank, immediate_dominators, time }
 }
 
 /// Evaluate the link-eval virtual forest, providing the currently minimum semi
@@ -316,6 +319,7 @@ pub struct Dominators<N: Idx> {
     // possible to get its full list of dominators by looking up the dominator
     // of each dominator. (See the `impl Iterator for Iter` definition).
     immediate_dominators: IndexVec<N, Option<N>>,
+    time: IndexVec<N, Time>,
 }
 
 impl<Node: Idx> Dominators<Node> {
@@ -333,12 +337,7 @@ impl<Node: Idx> Dominators<Node> {
     /// See the `impl Iterator for Iter` definition to understand how this works.
     pub fn dominators(&self, node: Node) -> Iter<'_, Node> {
         assert!(self.is_reachable(node), "node {node:?} is not reachable");
-        Iter { dominators: self, node: Some(node) }
-    }
-
-    pub fn dominates(&self, dom: Node, node: Node) -> bool {
-        // FIXME -- could be optimized by using post-order-rank
-        self.dominators(node).any(|n| n == dom)
+        Iter { dom_tree: self, node: Some(node) }
     }
 
     /// Provide deterministic ordering of nodes such that, if any two nodes have a dominator
@@ -348,10 +347,22 @@ impl<Node: Idx> Dominators<Node> {
     pub fn rank_partial_cmp(&self, lhs: Node, rhs: Node) -> Option<Ordering> {
         self.post_order_rank[rhs].partial_cmp(&self.post_order_rank[lhs])
     }
+
+    /// Returns true if `a` dominates `b`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `b` is unreachable.
+    pub fn dominates(&self, a: Node, b: Node) -> bool {
+        let a = self.time[a];
+        let b = self.time[b];
+        assert!(b.start != 0, "node {b:?} is not reachable");
+        a.start <= b.start && b.finish <= a.finish
+    }
 }
 
 pub struct Iter<'dom, Node: Idx> {
-    dominators: &'dom Dominators<Node>,
+    dom_tree: &'dom Dominators<Node>,
     node: Option<Node>,
 }
 
@@ -360,10 +371,74 @@ impl<'dom, Node: Idx> Iterator for Iter<'dom, Node> {
 
     fn next(&mut self) -> Option<Self::Item> {
         if let Some(node) = self.node {
-            self.node = self.dominators.immediate_dominator(node);
+            self.node = self.dom_tree.immediate_dominator(node);
             Some(node)
         } else {
             None
         }
     }
 }
+
+/// Describes the number of vertices discovered at the time when processing of a particular vertex
+/// started and when it finished. Both values are zero for unreachable vertices.
+#[derive(Copy, Clone, Default, Debug)]
+struct Time {
+    start: u32,
+    finish: u32,
+}
+
+fn compute_access_time<N: Idx>(
+    start_node: N,
+    immediate_dominators: &IndexSlice<N, Option<N>>,
+) -> IndexVec<N, Time> {
+    // Transpose the dominator tree edges, so that child nodes of vertex v are stored in
+    // node[edges[v].start..edges[v].end].
+    let mut edges: IndexVec<N, std::ops::Range<u32>> =
+        IndexVec::from_elem(0..0, immediate_dominators);
+    for &idom in immediate_dominators.iter() {
+        if let Some(idom) = idom {
+            edges[idom].end += 1;
+        }
+    }
+    let mut m = 0;
+    for e in edges.iter_mut() {
+        m += e.end;
+        e.start = m;
+        e.end = m;
+    }
+    let mut node = IndexVec::from_elem_n(Idx::new(0), m.try_into().unwrap());
+    for (i, &idom) in immediate_dominators.iter_enumerated() {
+        if let Some(idom) = idom {
+            edges[idom].start -= 1;
+            node[edges[idom].start] = i;
+        }
+    }
+
+    // Perform a depth-first search of the dominator tree. Record the number of vertices discovered
+    // when vertex v is discovered first as time[v].start, and when its processing is finished as
+    // time[v].finish.
+    let mut time: IndexVec<N, Time> = IndexVec::from_elem(Time::default(), immediate_dominators);
+    let mut stack = Vec::new();
+
+    let mut discovered = 1;
+    stack.push(start_node);
+    time[start_node].start = discovered;
+
+    while let Some(&i) = stack.last() {
+        let e = &mut edges[i];
+        if e.start == e.end {
+            // Finish processing vertex i.
+            time[i].finish = discovered;
+            stack.pop();
+        } else {
+            let j = node[e.start];
+            e.start += 1;
+            // Start processing vertex j.
+            discovered += 1;
+            time[j].start = discovered;
+            stack.push(j);
+        }
+    }
+
+    time
+}
diff --git a/compiler/rustc_middle/src/mir/basic_blocks.rs b/compiler/rustc_middle/src/mir/basic_blocks.rs
index 1319ddbb877..9d70dbfa072 100644
--- a/compiler/rustc_middle/src/mir/basic_blocks.rs
+++ b/compiler/rustc_middle/src/mir/basic_blocks.rs
@@ -27,6 +27,7 @@ struct Cache {
     switch_sources: OnceCell<SwitchSources>,
     is_cyclic: OnceCell<bool>,
     postorder: OnceCell<Vec<BasicBlock>>,
+    dominators: OnceCell<Dominators<BasicBlock>>,
 }
 
 impl<'tcx> BasicBlocks<'tcx> {
@@ -41,8 +42,8 @@ impl<'tcx> BasicBlocks<'tcx> {
         *self.cache.is_cyclic.get_or_init(|| graph::is_cyclic(self))
     }
 
-    pub fn dominators(&self) -> Dominators<BasicBlock> {
-        dominators(&self)
+    pub fn dominators(&self) -> &Dominators<BasicBlock> {
+        self.cache.dominators.get_or_init(|| dominators(self))
     }
 
     /// Returns predecessors for each basic block.
diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs
index 986d2fd190d..ea1223fbca6 100644
--- a/compiler/rustc_mir_transform/src/coverage/graph.rs
+++ b/compiler/rustc_mir_transform/src/coverage/graph.rs
@@ -9,6 +9,7 @@ use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::mir::coverage::*;
 use rustc_middle::mir::{self, BasicBlock, BasicBlockData, Terminator, TerminatorKind};
 
+use std::cmp::Ordering;
 use std::ops::{Index, IndexMut};
 
 const ID_SEPARATOR: &str = ",";
@@ -212,8 +213,12 @@ impl CoverageGraph {
     }
 
     #[inline(always)]
-    pub fn dominators(&self) -> &Dominators<BasicCoverageBlock> {
-        self.dominators.as_ref().unwrap()
+    pub fn rank_partial_cmp(
+        &self,
+        a: BasicCoverageBlock,
+        b: BasicCoverageBlock,
+    ) -> Option<Ordering> {
+        self.dominators.as_ref().unwrap().rank_partial_cmp(a, b)
     }
 }
 
@@ -650,26 +655,6 @@ pub(super) fn find_loop_backedges(
     let mut backedges = IndexVec::from_elem_n(Vec::<BasicCoverageBlock>::new(), num_bcbs);
 
     // Identify loops by their backedges.
-    //
-    // The computational complexity is bounded by: n(s) x d where `n` is the number of
-    // `BasicCoverageBlock` nodes (the simplified/reduced representation of the CFG derived from the
-    // MIR); `s` is the average number of successors per node (which is most likely less than 2, and
-    // independent of the size of the function, so it can be treated as a constant);
-    // and `d` is the average number of dominators per node.
-    //
-    // The average number of dominators depends on the size and complexity of the function, and
-    // nodes near the start of the function's control flow graph typically have less dominators
-    // than nodes near the end of the CFG. Without doing a detailed mathematical analysis, I
-    // think the resulting complexity has the characteristics of O(n log n).
-    //
-    // The overall complexity appears to be comparable to many other MIR transform algorithms, and I
-    // don't expect that this function is creating a performance hot spot, but if this becomes an
-    // issue, there may be ways to optimize the `dominates` algorithm (as indicated by an
-    // existing `FIXME` comment in that code), or possibly ways to optimize it's usage here, perhaps
-    // by keeping track of results for visited `BasicCoverageBlock`s if they can be used to short
-    // circuit downstream `dominates` checks.
-    //
-    // For now, that kind of optimization seems unnecessarily complicated.
     for (bcb, _) in basic_coverage_blocks.iter_enumerated() {
         for &successor in &basic_coverage_blocks.successors[bcb] {
             if basic_coverage_blocks.dominates(successor, bcb) {
diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs
index 14937912cc5..d27200419e2 100644
--- a/compiler/rustc_mir_transform/src/coverage/spans.rs
+++ b/compiler/rustc_mir_transform/src/coverage/spans.rs
@@ -344,7 +344,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
                         // before the dominated equal spans). When later comparing two spans in
                         // order, the first will either dominate the second, or they will have no
                         // dominator relationship.
-                        self.basic_coverage_blocks.dominators().rank_partial_cmp(a.bcb, b.bcb)
+                        self.basic_coverage_blocks.rank_partial_cmp(a.bcb, b.bcb)
                     }
                 } else {
                     // Sort hi() in reverse order so shorter spans are attempted after longer spans.
diff --git a/compiler/rustc_mir_transform/src/ctfe_limit.rs b/compiler/rustc_mir_transform/src/ctfe_limit.rs
index 1b3ac78fbc6..bf5722b3d00 100644
--- a/compiler/rustc_mir_transform/src/ctfe_limit.rs
+++ b/compiler/rustc_mir_transform/src/ctfe_limit.rs
@@ -47,7 +47,7 @@ fn has_back_edge(
         return false;
     }
     // Check if any of the dominators of the node are also the node's successor.
-    doms.dominators(node).any(|dom| node_data.terminator().successors().any(|succ| succ == dom))
+    node_data.terminator().successors().any(|succ| doms.dominates(succ, node))
 }
 
 fn insert_counter(basic_block_data: &mut BasicBlockData<'_>) {
diff --git a/compiler/rustc_mir_transform/src/ssa.rs b/compiler/rustc_mir_transform/src/ssa.rs
index 2b404efccc7..e8e4246b797 100644
--- a/compiler/rustc_mir_transform/src/ssa.rs
+++ b/compiler/rustc_mir_transform/src/ssa.rs
@@ -31,11 +31,11 @@ pub struct SsaLocals {
 /// We often encounter MIR bodies with 1 or 2 basic blocks. In those cases, it's unnecessary to
 /// actually compute dominators, we can just compare block indices because bb0 is always the first
 /// block, and in any body all other blocks are always dominated by bb0.
-struct SmallDominators {
-    inner: Option<Dominators<BasicBlock>>,
+struct SmallDominators<'a> {
+    inner: Option<&'a Dominators<BasicBlock>>,
 }
 
-impl SmallDominators {
+impl SmallDominators<'_> {
     fn dominates(&self, first: Location, second: Location) -> bool {
         if first.block == second.block {
             first.statement_index <= second.statement_index
@@ -198,14 +198,14 @@ enum LocationExtended {
     Arg,
 }
 
-struct SsaVisitor {
-    dominators: SmallDominators,
+struct SsaVisitor<'a> {
+    dominators: SmallDominators<'a>,
     assignments: IndexVec<Local, Set1<LocationExtended>>,
     assignment_order: Vec<Local>,
     direct_uses: IndexVec<Local, u32>,
 }
 
-impl<'tcx> Visitor<'tcx> for SsaVisitor {
+impl<'tcx> Visitor<'tcx> for SsaVisitor<'_> {
     fn visit_local(&mut self, local: Local, ctxt: PlaceContext, loc: Location) {
         match ctxt {
             PlaceContext::MutatingUse(MutatingUseContext::Projection)
diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs
index 7b39cb0a068..65dfdf31e54 100644
--- a/compiler/rustc_privacy/src/lib.rs
+++ b/compiler/rustc_privacy/src/lib.rs
@@ -454,12 +454,14 @@ struct EmbargoVisitor<'tcx> {
     ///     n::p::f()
     /// }
     macro_reachable: FxHashSet<(LocalDefId, LocalDefId)>,
+    /// Preliminary pass for marking all underlying types of `impl Trait`s as reachable.
+    impl_trait_pass: bool,
     /// Has something changed in the level map?
     changed: bool,
 }
 
 struct ReachEverythingInTheInterfaceVisitor<'a, 'tcx> {
-    effective_vis: Option<EffectiveVisibility>,
+    effective_vis: EffectiveVisibility,
     item_def_id: LocalDefId,
     ev: &'a mut EmbargoVisitor<'tcx>,
     level: Level,
@@ -474,7 +476,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
     fn update(
         &mut self,
         def_id: LocalDefId,
-        inherited_effective_vis: Option<EffectiveVisibility>,
+        inherited_effective_vis: EffectiveVisibility,
         level: Level,
     ) {
         let nominal_vis = self.tcx.local_visibility(def_id);
@@ -484,30 +486,27 @@ impl<'tcx> EmbargoVisitor<'tcx> {
     fn update_eff_vis(
         &mut self,
         def_id: LocalDefId,
-        inherited_effective_vis: Option<EffectiveVisibility>,
+        inherited_effective_vis: EffectiveVisibility,
         nominal_vis: Option<ty::Visibility>,
         level: Level,
     ) {
-        if let Some(inherited_effective_vis) = inherited_effective_vis {
-            let private_vis =
-                ty::Visibility::Restricted(self.tcx.parent_module_from_def_id(def_id));
-            if Some(private_vis) != nominal_vis {
-                self.changed |= self.effective_visibilities.update(
-                    def_id,
-                    nominal_vis,
-                    || private_vis,
-                    inherited_effective_vis,
-                    level,
-                    self.tcx,
-                );
-            }
+        let private_vis = ty::Visibility::Restricted(self.tcx.parent_module_from_def_id(def_id));
+        if Some(private_vis) != nominal_vis {
+            self.changed |= self.effective_visibilities.update(
+                def_id,
+                nominal_vis,
+                || private_vis,
+                inherited_effective_vis,
+                level,
+                self.tcx,
+            );
         }
     }
 
     fn reach(
         &mut self,
         def_id: LocalDefId,
-        effective_vis: Option<EffectiveVisibility>,
+        effective_vis: EffectiveVisibility,
     ) -> ReachEverythingInTheInterfaceVisitor<'_, 'tcx> {
         ReachEverythingInTheInterfaceVisitor {
             effective_vis,
@@ -520,7 +519,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
     fn reach_through_impl_trait(
         &mut self,
         def_id: LocalDefId,
-        effective_vis: Option<EffectiveVisibility>,
+        effective_vis: EffectiveVisibility,
     ) -> ReachEverythingInTheInterfaceVisitor<'_, 'tcx> {
         ReachEverythingInTheInterfaceVisitor {
             effective_vis,
@@ -532,9 +531,13 @@ impl<'tcx> EmbargoVisitor<'tcx> {
 
     // We have to make sure that the items that macros might reference
     // are reachable, since they might be exported transitively.
-    fn update_reachability_from_macro(&mut self, local_def_id: LocalDefId, md: &MacroDef) {
+    fn update_reachability_from_macro(
+        &mut self,
+        local_def_id: LocalDefId,
+        md: &MacroDef,
+        macro_ev: EffectiveVisibility,
+    ) {
         // Non-opaque macros cannot make other items more accessible than they already are.
-
         let hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id);
         let attrs = self.tcx.hir().attrs(hir_id);
         if attr::find_transparency(attrs, md.macro_rules).0 != Transparency::Opaque {
@@ -554,8 +557,6 @@ impl<'tcx> EmbargoVisitor<'tcx> {
         // Since we are starting from an externally visible module,
         // all the parents in the loop below are also guaranteed to be modules.
         let mut module_def_id = macro_module_def_id;
-        let macro_ev = self.get(local_def_id);
-        assert!(macro_ev.is_some());
         loop {
             let changed_reachability =
                 self.update_macro_reachable(module_def_id, macro_module_def_id, macro_ev);
@@ -572,7 +573,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
         &mut self,
         module_def_id: LocalDefId,
         defining_mod: LocalDefId,
-        macro_ev: Option<EffectiveVisibility>,
+        macro_ev: EffectiveVisibility,
     ) -> bool {
         if self.macro_reachable.insert((module_def_id, defining_mod)) {
             self.update_macro_reachable_mod(module_def_id, defining_mod, macro_ev);
@@ -586,7 +587,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
         &mut self,
         module_def_id: LocalDefId,
         defining_mod: LocalDefId,
-        macro_ev: Option<EffectiveVisibility>,
+        macro_ev: EffectiveVisibility,
     ) {
         let module = self.tcx.hir().get_module(module_def_id).0;
         for item_id in module.item_ids {
@@ -618,7 +619,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
         def_kind: DefKind,
         vis: ty::Visibility,
         module: LocalDefId,
-        macro_ev: Option<EffectiveVisibility>,
+        macro_ev: EffectiveVisibility,
     ) {
         self.update(def_id, macro_ev, Level::Reachable);
         match def_kind {
@@ -700,128 +701,53 @@ impl<'tcx> EmbargoVisitor<'tcx> {
 }
 
 impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
-    type NestedFilter = nested_filter::All;
-
-    /// We want to visit items in the context of their containing
-    /// module and so forth, so supply a crate for doing a deep walk.
-    fn nested_visit_map(&mut self) -> Self::Map {
-        self.tcx.hir()
-    }
-
     fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
-        let item_ev = match item.kind {
-            hir::ItemKind::Impl { .. } => {
-                let impl_ev = Option::<EffectiveVisibility>::of_impl(
-                    item.owner_id.def_id,
-                    self.tcx,
-                    &self.effective_visibilities,
-                );
-
-                self.update_eff_vis(item.owner_id.def_id, impl_ev, None, Level::Direct);
-                impl_ev
-            }
-            _ => self.get(item.owner_id.def_id),
-        };
-
-        // Update levels of nested things.
-        match item.kind {
-            hir::ItemKind::Enum(ref def, _) => {
-                for variant in def.variants {
-                    self.update(variant.def_id, item_ev, Level::Reachable);
-                    let variant_ev = self.get(variant.def_id);
-                    if let Some(ctor_def_id) = variant.data.ctor_def_id() {
-                        self.update(ctor_def_id, variant_ev, Level::Reachable);
-                    }
-                    for field in variant.data.fields() {
-                        self.update(field.def_id, variant_ev, Level::Reachable);
-                    }
-                }
-            }
-            hir::ItemKind::Impl(ref impl_) => {
-                for impl_item_ref in impl_.items {
-                    let def_id = impl_item_ref.id.owner_id.def_id;
-                    let nominal_vis =
-                        impl_.of_trait.is_none().then(|| self.tcx.local_visibility(def_id));
-                    self.update_eff_vis(def_id, item_ev, nominal_vis, Level::Direct);
-                }
-            }
-            hir::ItemKind::Trait(.., trait_item_refs) => {
-                for trait_item_ref in trait_item_refs {
-                    self.update(trait_item_ref.id.owner_id.def_id, item_ev, Level::Reachable);
-                }
-            }
-            hir::ItemKind::Struct(ref def, _) | hir::ItemKind::Union(ref def, _) => {
-                if let Some(ctor_def_id) = def.ctor_def_id() {
-                    self.update(ctor_def_id, item_ev, Level::Reachable);
-                }
-                for field in def.fields() {
-                    self.update(field.def_id, item_ev, Level::Reachable);
-                }
-            }
-            hir::ItemKind::Macro(ref macro_def, _) => {
-                self.update_reachability_from_macro(item.owner_id.def_id, macro_def);
-            }
-            hir::ItemKind::ForeignMod { items, .. } => {
-                for foreign_item in items {
-                    self.update(foreign_item.id.owner_id.def_id, item_ev, Level::Reachable);
-                }
-            }
-
-            hir::ItemKind::OpaqueTy(..)
-            | hir::ItemKind::Use(..)
-            | hir::ItemKind::Static(..)
-            | hir::ItemKind::Const(..)
-            | hir::ItemKind::GlobalAsm(..)
-            | hir::ItemKind::TyAlias(..)
-            | hir::ItemKind::Mod(..)
-            | hir::ItemKind::TraitAlias(..)
-            | hir::ItemKind::Fn(..)
-            | hir::ItemKind::ExternCrate(..) => {}
+        if self.impl_trait_pass
+            && let hir::ItemKind::OpaqueTy(ref opaque) = item.kind
+            && !opaque.in_trait {
+            // FIXME: This is some serious pessimization intended to workaround deficiencies
+            // in the reachability pass (`middle/reachable.rs`). Types are marked as link-time
+            // reachable if they are returned via `impl Trait`, even from private functions.
+            let pub_ev = EffectiveVisibility::from_vis(ty::Visibility::Public);
+            self.reach_through_impl_trait(item.owner_id.def_id, pub_ev)
+                .generics()
+                .predicates()
+                .ty();
+            return;
         }
 
-        // Mark all items in interfaces of reachable items as reachable.
+        // Update levels of nested things and mark all items
+        // in interfaces of reachable items as reachable.
+        let item_ev = self.get(item.owner_id.def_id);
         match item.kind {
-            // The interface is empty.
-            hir::ItemKind::Macro(..) | hir::ItemKind::ExternCrate(..) => {}
-            // All nested items are checked by `visit_item`.
-            hir::ItemKind::Mod(..) => {}
-            // Handled in `rustc_resolve`.
-            hir::ItemKind::Use(..) => {}
-            // The interface is empty.
-            hir::ItemKind::GlobalAsm(..) => {}
-            hir::ItemKind::OpaqueTy(ref opaque) => {
-                // HACK(jynelson): trying to infer the type of `impl trait` breaks `async-std` (and `pub async fn` in general)
-                // Since rustdoc never needs to do codegen and doesn't care about link-time reachability,
-                // mark this as unreachable.
-                // See https://github.com/rust-lang/rust/issues/75100
-                if !opaque.in_trait && !self.tcx.sess.opts.actually_rustdoc {
-                    // FIXME: This is some serious pessimization intended to workaround deficiencies
-                    // in the reachability pass (`middle/reachable.rs`). Types are marked as link-time
-                    // reachable if they are returned via `impl Trait`, even from private functions.
-                    let exist_ev = Some(EffectiveVisibility::from_vis(ty::Visibility::Public));
-                    self.reach_through_impl_trait(item.owner_id.def_id, exist_ev)
-                        .generics()
-                        .predicates()
-                        .ty();
+            // The interface is empty, and no nested items.
+            hir::ItemKind::Use(..)
+            | hir::ItemKind::ExternCrate(..)
+            | hir::ItemKind::GlobalAsm(..) => {}
+            // The interface is empty, and all nested items are processed by `visit_item`.
+            hir::ItemKind::Mod(..) | hir::ItemKind::OpaqueTy(..) => {}
+            hir::ItemKind::Macro(ref macro_def, _) => {
+                if let Some(item_ev) = item_ev {
+                    self.update_reachability_from_macro(item.owner_id.def_id, macro_def, item_ev);
                 }
             }
-            // Visit everything.
             hir::ItemKind::Const(..)
             | hir::ItemKind::Static(..)
             | hir::ItemKind::Fn(..)
             | hir::ItemKind::TyAlias(..) => {
-                if item_ev.is_some() {
+                if let Some(item_ev) = item_ev {
                     self.reach(item.owner_id.def_id, item_ev).generics().predicates().ty();
                 }
             }
             hir::ItemKind::Trait(.., trait_item_refs) => {
-                if item_ev.is_some() {
+                if let Some(item_ev) = item_ev {
                     self.reach(item.owner_id.def_id, item_ev).generics().predicates();
 
                     for trait_item_ref in trait_item_refs {
+                        self.update(trait_item_ref.id.owner_id.def_id, item_ev, Level::Reachable);
+
                         let tcx = self.tcx;
                         let mut reach = self.reach(trait_item_ref.id.owner_id.def_id, item_ev);
-
                         reach.generics().predicates();
 
                         if trait_item_ref.kind == AssocItemKind::Type
@@ -835,13 +761,18 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
                 }
             }
             hir::ItemKind::TraitAlias(..) => {
-                if item_ev.is_some() {
+                if let Some(item_ev) = item_ev {
                     self.reach(item.owner_id.def_id, item_ev).generics().predicates();
                 }
             }
-            // Visit everything except for private impl items.
             hir::ItemKind::Impl(ref impl_) => {
-                if item_ev.is_some() {
+                if let Some(item_ev) = Option::<EffectiveVisibility>::of_impl(
+                    item.owner_id.def_id,
+                    self.tcx,
+                    &self.effective_visibilities,
+                ) {
+                    self.update_eff_vis(item.owner_id.def_id, item_ev, None, Level::Direct);
+
                     self.reach(item.owner_id.def_id, item_ev)
                         .generics()
                         .predicates()
@@ -849,27 +780,32 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
                         .trait_ref();
 
                     for impl_item_ref in impl_.items {
-                        let impl_item_ev = self.get(impl_item_ref.id.owner_id.def_id);
+                        let def_id = impl_item_ref.id.owner_id.def_id;
+                        let nominal_vis =
+                            impl_.of_trait.is_none().then(|| self.tcx.local_visibility(def_id));
+                        self.update_eff_vis(def_id, item_ev, nominal_vis, Level::Direct);
 
-                        if impl_item_ev.is_some() {
-                            self.reach(impl_item_ref.id.owner_id.def_id, impl_item_ev)
-                                .generics()
-                                .predicates()
-                                .ty();
+                        if let Some(impl_item_ev) = self.get(def_id) {
+                            self.reach(def_id, impl_item_ev).generics().predicates().ty();
                         }
                     }
                 }
             }
-
-            // Visit everything, but enum variants have their own levels.
             hir::ItemKind::Enum(ref def, _) => {
-                if item_ev.is_some() {
+                if let Some(item_ev) = item_ev {
                     self.reach(item.owner_id.def_id, item_ev).generics().predicates();
                 }
                 for variant in def.variants {
-                    let variant_ev = self.get(variant.def_id);
-                    if variant_ev.is_some() {
+                    if let Some(item_ev) = item_ev {
+                        self.update(variant.def_id, item_ev, Level::Reachable);
+                    }
+
+                    if let Some(variant_ev) = self.get(variant.def_id) {
+                        if let Some(ctor_def_id) = variant.data.ctor_def_id() {
+                            self.update(ctor_def_id, variant_ev, Level::Reachable);
+                        }
                         for field in variant.data.fields() {
+                            self.update(field.def_id, variant_ev, Level::Reachable);
                             self.reach(field.def_id, variant_ev).ty();
                         }
                         // Corner case: if the variant is reachable, but its
@@ -877,18 +813,15 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
                         self.reach(item.owner_id.def_id, variant_ev).ty();
                     }
                     if let Some(ctor_def_id) = variant.data.ctor_def_id() {
-                        let ctor_ev = self.get(ctor_def_id);
-                        if ctor_ev.is_some() {
+                        if let Some(ctor_ev) = self.get(ctor_def_id) {
                             self.reach(item.owner_id.def_id, ctor_ev).ty();
                         }
                     }
                 }
             }
-            // Visit everything, but foreign items have their own levels.
             hir::ItemKind::ForeignMod { items, .. } => {
                 for foreign_item in items {
-                    let foreign_item_ev = self.get(foreign_item.id.owner_id.def_id);
-                    if foreign_item_ev.is_some() {
+                    if let Some(foreign_item_ev) = self.get(foreign_item.id.owner_id.def_id) {
                         self.reach(foreign_item.id.owner_id.def_id, foreign_item_ev)
                             .generics()
                             .predicates()
@@ -896,34 +829,26 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
                     }
                 }
             }
-            // Visit everything except for private fields.
             hir::ItemKind::Struct(ref struct_def, _) | hir::ItemKind::Union(ref struct_def, _) => {
-                if item_ev.is_some() {
+                if let Some(item_ev) = item_ev {
                     self.reach(item.owner_id.def_id, item_ev).generics().predicates();
                     for field in struct_def.fields() {
-                        let field_ev = self.get(field.def_id);
-                        if field_ev.is_some() {
+                        self.update(field.def_id, item_ev, Level::Reachable);
+                        if let Some(field_ev) = self.get(field.def_id) {
                             self.reach(field.def_id, field_ev).ty();
                         }
                     }
                 }
                 if let Some(ctor_def_id) = struct_def.ctor_def_id() {
-                    let ctor_ev = self.get(ctor_def_id);
-                    if ctor_ev.is_some() {
+                    if let Some(item_ev) = item_ev {
+                        self.update(ctor_def_id, item_ev, Level::Reachable);
+                    }
+                    if let Some(ctor_ev) = self.get(ctor_def_id) {
                         self.reach(item.owner_id.def_id, ctor_ev).ty();
                     }
                 }
             }
         }
-
-        intravisit::walk_item(self, item);
-    }
-
-    fn visit_block(&mut self, b: &'tcx hir::Block<'tcx>) {
-        // Blocks can have public items, for example impls, but they always
-        // start as completely private regardless of publicity of a function,
-        // constant, type, field, etc., in which this block resides.
-        intravisit::walk_block(self, b);
     }
 }
 
@@ -2205,12 +2130,24 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {
         tcx,
         effective_visibilities: tcx.resolutions(()).effective_visibilities.clone(),
         macro_reachable: Default::default(),
+        // HACK(jynelson): trying to infer the type of `impl Trait` breaks `async-std` (and
+        // `pub async fn` in general). Since rustdoc never needs to do codegen and doesn't
+        // care about link-time reachability, keep them unreachable (issue #75100).
+        impl_trait_pass: !tcx.sess.opts.actually_rustdoc,
         changed: false,
     };
 
     visitor.effective_visibilities.check_invariants(tcx, true);
+    if visitor.impl_trait_pass {
+        // Underlying types of `impl Trait`s are marked as reachable unconditionally,
+        // so this pass doesn't need to be a part of the fixed point iteration below.
+        tcx.hir().visit_all_item_likes_in_crate(&mut visitor);
+        visitor.impl_trait_pass = false;
+        visitor.changed = false;
+    }
+
     loop {
-        tcx.hir().walk_toplevel_module(&mut visitor);
+        tcx.hir().visit_all_item_likes_in_crate(&mut visitor);
         if visitor.changed {
             visitor.changed = false;
         } else {