Auto merge of #87234 - cjgillot:lower-mono, r=petrochenkov

Lower only one HIR owner at a time

Based on https://github.com/rust-lang/rust/pull/83723
Additional diff is here: https://github.com/cjgillot/rust/compare/ownernode...lower-mono

Lowering is very tangled and has a tendency to intertwine the transformation of different items. This PR aims at simplifying the logic by:
- moving global analyses to the resolver (item_generics_num_lifetimes, proc_macros, trait_impls);
- removing a few special cases (non-exported macros and use statements);
- restricting the amount of available information at any one time;
- avoiding back-and-forth between different owners: an item must now be lowered all at once, and its parent cannot refer to its nodes.

I also removed the sorting of bodies by span.  The diagnostic ordering changes marginally, since definitions are pretty much sorted already according to the AST. This uncovered a subtlety in thir-unsafeck.

(While these items could logically be in different PRs, the dependency between commits and the amount of conflicts force a monolithic PR.)

19 files changed, 163 insertions, 258 deletions

diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs
index ac7145bed78..a6ea4aa8923 100644
--- a/compiler/rustc_ast_lowering/src/expr.rs
+++ b/compiler/rustc_ast_lowering/src/expr.rs
@@ -55,7 +55,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
                         0,
                         ParenthesizedGenericArgs::Err,
                         ImplTraitContext::disallowed(),
-                        None,
                     ));
                     let args = self.lower_exprs(args);
                     hir::ExprKind::MethodCall(
@@ -328,7 +327,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let mut generic_args = vec![];
         for (idx, arg) in args.into_iter().enumerate() {
             if legacy_args_idx.contains(&idx) {
-                let parent_def_id = self.current_hir_id_owner.0;
+                let parent_def_id = self.current_hir_id_owner;
                 let node_id = self.resolver.next_node_id();
 
                 // Add a definition for the in-band const def.
diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
index 980036c662a..25e6fed68b5 100644
--- a/compiler/rustc_ast_lowering/src/item.rs
+++ b/compiler/rustc_ast_lowering/src/item.rs
@@ -40,12 +40,9 @@ impl ItemLowerer<'_, '_, '_> {
 
 impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
     fn visit_item(&mut self, item: &'a Item) {
-        self.lctx.allocate_hir_id_counter(item.id);
         let hir_id = self.lctx.with_hir_id_owner(item.id, |lctx| {
-            lctx.without_in_scope_lifetime_defs(|lctx| {
-                let hir_item = lctx.lower_item(item);
-                lctx.insert_item(hir_item)
-            })
+            let node = lctx.without_in_scope_lifetime_defs(|lctx| lctx.lower_item(item));
+            hir::OwnerNode::Item(node)
         });
 
         self.lctx.with_parent_item_lifetime_defs(hir_id, |this| {
@@ -72,26 +69,17 @@ impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
     }
 
     fn visit_assoc_item(&mut self, item: &'a AssocItem, ctxt: AssocCtxt) {
-        self.lctx.allocate_hir_id_counter(item.id);
         self.lctx.with_hir_id_owner(item.id, |lctx| match ctxt {
-            AssocCtxt::Trait => {
-                let hir_item = lctx.lower_trait_item(item);
-                lctx.insert_trait_item(hir_item);
-            }
-            AssocCtxt::Impl => {
-                let hir_item = lctx.lower_impl_item(item);
-                lctx.insert_impl_item(hir_item);
-            }
+            AssocCtxt::Trait => hir::OwnerNode::TraitItem(lctx.lower_trait_item(item)),
+            AssocCtxt::Impl => hir::OwnerNode::ImplItem(lctx.lower_impl_item(item)),
         });
 
         visit::walk_assoc_item(self, item, ctxt);
     }
 
     fn visit_foreign_item(&mut self, item: &'a ForeignItem) {
-        self.lctx.allocate_hir_id_counter(item.id);
         self.lctx.with_hir_id_owner(item.id, |lctx| {
-            let hir_item = lctx.lower_foreign_item(item);
-            lctx.insert_foreign_item(hir_item);
+            hir::OwnerNode::ForeignItem(lctx.lower_foreign_item(item))
         });
 
         visit::walk_foreign_item(self, item);
@@ -106,12 +94,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
     // only used when lowering a child item of a trait or impl.
     fn with_parent_item_lifetime_defs<T>(
         &mut self,
-        parent_hir_id: hir::ItemId,
+        parent_hir_id: LocalDefId,
         f: impl FnOnce(&mut Self) -> T,
     ) -> T {
         let old_len = self.in_scope_lifetimes.len();
 
-        let parent_generics = match self.owners[parent_hir_id.def_id].unwrap().expect_item().kind {
+        let parent_generics = match self.owners[parent_hir_id].unwrap().expect_item().kind {
             hir::ItemKind::Impl(hir::Impl { ref generics, .. })
             | hir::ItemKind::Trait(_, _, ref generics, ..) => generics.params,
             _ => &[],
@@ -186,19 +174,20 @@ impl<'hir> LoweringContext<'_, 'hir> {
         }
     }
 
-    pub fn lower_item(&mut self, i: &Item) -> hir::Item<'hir> {
+    fn lower_item(&mut self, i: &Item) -> &'hir hir::Item<'hir> {
         let mut ident = i.ident;
-        let mut vis = self.lower_visibility(&i.vis, None);
+        let mut vis = self.lower_visibility(&i.vis);
         let hir_id = self.lower_node_id(i.id);
         let attrs = self.lower_attrs(hir_id, &i.attrs);
         let kind = self.lower_item_kind(i.span, i.id, hir_id, &mut ident, attrs, &mut vis, &i.kind);
-        hir::Item {
+        let item = hir::Item {
             def_id: hir_id.expect_owner(),
             ident: self.lower_ident(ident),
             kind,
             vis,
             span: self.lower_span(i.span),
-        }
+        };
+        self.arena.alloc(item)
     }
 
     fn lower_item_kind(
@@ -480,10 +469,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 // Essentially a single `use` which imports two names is desugared into
                 // two imports.
                 for new_node_id in [id1, id2] {
-                    // Associate an HirId to both ids even if there is no resolution.
-                    let new_id = self.allocate_hir_id_counter(new_node_id);
-
-                    let res = if let Some(res) = resolutions.next() { res } else { continue };
+                    let new_id = self.resolver.local_def_id(new_node_id);
+                    let res = if let Some(res) = resolutions.next() {
+                        res
+                    } else {
+                        // Associate an HirId to both ids even if there is no resolution.
+                        self.node_id_to_hir_id.ensure_contains_elem(new_node_id, || None);
+                        debug_assert!(self.node_id_to_hir_id[new_node_id].is_none());
+                        self.node_id_to_hir_id[new_node_id] = Some(hir::HirId::make_owner(new_id));
+                        continue;
+                    };
                     let ident = *ident;
                     let mut path = path.clone();
                     for seg in &mut path.segments {
@@ -493,24 +488,25 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                     self.with_hir_id_owner(new_node_id, |this| {
                         let res = this.lower_res(res);
-                        let path = this.lower_path_extra(res, &path, ParamMode::Explicit, None);
+                        let path = this.lower_path_extra(res, &path, ParamMode::Explicit);
                         let kind = hir::ItemKind::Use(path, hir::UseKind::Single);
                         let vis = this.rebuild_vis(&vis);
                         if let Some(attrs) = attrs {
                             this.attrs.insert(hir::HirId::make_owner(new_id), attrs);
                         }
 
-                        this.insert_item(hir::Item {
+                        let item = hir::Item {
                             def_id: new_id,
                             ident: this.lower_ident(ident),
                             kind,
                             vis,
                             span: this.lower_span(span),
-                        });
+                        };
+                        hir::OwnerNode::Item(this.arena.alloc(item))
                     });
                 }
 
-                let path = self.lower_path_extra(ret_res, &path, ParamMode::Explicit, None);
+                let path = self.lower_path_extra(ret_res, &path, ParamMode::Explicit);
                 hir::ItemKind::Use(path, hir::UseKind::Single)
             }
             UseTreeKind::Glob => {
@@ -550,7 +546,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                 // Add all the nested `PathListItem`s to the HIR.
                 for &(ref use_tree, id) in trees {
-                    let new_hir_id = self.allocate_hir_id_counter(id);
+                    let new_hir_id = self.resolver.local_def_id(id);
 
                     let mut prefix = prefix.clone();
 
@@ -574,13 +570,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
                             this.attrs.insert(hir::HirId::make_owner(new_hir_id), attrs);
                         }
 
-                        this.insert_item(hir::Item {
+                        let item = hir::Item {
                             def_id: new_hir_id,
                             ident: this.lower_ident(ident),
                             kind,
                             vis,
                             span: this.lower_span(use_tree.span),
-                        });
+                        };
+                        hir::OwnerNode::Item(this.arena.alloc(item))
                     });
                 }
 
@@ -610,7 +607,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                 let res = self.expect_full_res_from_use(id).next().unwrap_or(Res::Err);
                 let res = self.lower_res(res);
-                let path = self.lower_path_extra(res, &prefix, ParamMode::Explicit, None);
+                let path = self.lower_path_extra(res, &prefix, ParamMode::Explicit);
                 hir::ItemKind::Use(path, hir::UseKind::ListStem)
             }
         }
@@ -647,11 +644,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
         respan(self.lower_span(vis.span), vis_kind)
     }
 
-    fn lower_foreign_item(&mut self, i: &ForeignItem) -> hir::ForeignItem<'hir> {
+    fn lower_foreign_item(&mut self, i: &ForeignItem) -> &'hir hir::ForeignItem<'hir> {
         let hir_id = self.lower_node_id(i.id);
         let def_id = hir_id.expect_owner();
         self.lower_attrs(hir_id, &i.attrs);
-        hir::ForeignItem {
+        let item = hir::ForeignItem {
             def_id,
             ident: self.lower_ident(i.ident),
             kind: match i.kind {
@@ -679,17 +676,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 ForeignItemKind::TyAlias(..) => hir::ForeignItemKind::Type,
                 ForeignItemKind::MacCall(_) => panic!("macro shouldn't exist here"),
             },
-            vis: self.lower_visibility(&i.vis, None),
+            vis: self.lower_visibility(&i.vis),
             span: self.lower_span(i.span),
-        }
+        };
+        self.arena.alloc(item)
     }
 
-    fn lower_foreign_item_ref(&mut self, i: &ForeignItem) -> hir::ForeignItemRef<'hir> {
+    fn lower_foreign_item_ref(&mut self, i: &ForeignItem) -> hir::ForeignItemRef {
         hir::ForeignItemRef {
-            id: hir::ForeignItemId { def_id: self.allocate_hir_id_counter(i.id) },
+            id: hir::ForeignItemId { def_id: self.resolver.local_def_id(i.id) },
             ident: self.lower_ident(i.ident),
             span: self.lower_span(i.span),
-            vis: self.lower_visibility(&i.vis, Some(i.id)),
         }
     }
 
@@ -757,12 +754,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 // FIXME(jseyfried): positional field hygiene.
                 None => Ident::new(sym::integer(index), self.lower_span(f.span)),
             },
-            vis: self.lower_visibility(&f.vis, None),
+            vis: self.lower_visibility(&f.vis),
             ty,
         }
     }
 
-    fn lower_trait_item(&mut self, i: &AssocItem) -> hir::TraitItem<'hir> {
+    fn lower_trait_item(&mut self, i: &AssocItem) -> &'hir hir::TraitItem<'hir> {
         let hir_id = self.lower_node_id(i.id);
         let trait_item_def_id = hir_id.expect_owner();
 
@@ -805,13 +802,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
         };
 
         self.lower_attrs(hir_id, &i.attrs);
-        hir::TraitItem {
+        let item = hir::TraitItem {
             def_id: trait_item_def_id,
             ident: self.lower_ident(i.ident),
             generics,
             kind,
             span: self.lower_span(i.span),
-        }
+        };
+        self.arena.alloc(item)
     }
 
     fn lower_trait_item_ref(&mut self, i: &AssocItem) -> hir::TraitItemRef {
@@ -841,7 +839,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         self.expr(span, hir::ExprKind::Err, AttrVec::new())
     }
 
-    fn lower_impl_item(&mut self, i: &AssocItem) -> hir::ImplItem<'hir> {
+    fn lower_impl_item(&mut self, i: &AssocItem) -> &'hir hir::ImplItem<'hir> {
         let impl_item_def_id = self.resolver.local_def_id(i.id);
 
         let (generics, kind) = match &i.kind {
@@ -895,26 +893,26 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
         let hir_id = self.lower_node_id(i.id);
         self.lower_attrs(hir_id, &i.attrs);
-        hir::ImplItem {
+        let item = hir::ImplItem {
             def_id: hir_id.expect_owner(),
             ident: self.lower_ident(i.ident),
             generics,
-            vis: self.lower_visibility(&i.vis, None),
+            vis: self.lower_visibility(&i.vis),
             defaultness,
             kind,
             span: self.lower_span(i.span),
-        }
+        };
+        self.arena.alloc(item)
     }
 
-    fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef<'hir> {
+    fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef {
         // Since `default impl` is not yet implemented, this is always true in impls.
         let has_value = true;
         let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
         hir::ImplItemRef {
-            id: hir::ImplItemId { def_id: self.allocate_hir_id_counter(i.id) },
+            id: hir::ImplItemId { def_id: self.resolver.local_def_id(i.id) },
             ident: self.lower_ident(i.ident),
             span: self.lower_span(i.span),
-            vis: self.lower_visibility(&i.vis, Some(i.id)),
             defaultness,
             kind: match &i.kind {
                 AssocItemKind::Const(..) => hir::AssocItemKind::Const,
@@ -932,25 +930,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
     /// lowered. This can happen during `lower_impl_item_ref()` where we need to
     /// lower a `Visibility` value although we haven't lowered the owning
     /// `ImplItem` in question yet.
-    fn lower_visibility(
-        &mut self,
-        v: &Visibility,
-        explicit_owner: Option<NodeId>,
-    ) -> hir::Visibility<'hir> {
+    fn lower_visibility(&mut self, v: &Visibility) -> hir::Visibility<'hir> {
         let node = match v.kind {
             VisibilityKind::Public => hir::VisibilityKind::Public,
             VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
             VisibilityKind::Restricted { ref path, id } => {
                 debug!("lower_visibility: restricted path id = {:?}", id);
-                let lowered_id = if let Some(owner) = explicit_owner {
-                    self.lower_node_id_with_owner(id, owner)
-                } else {
-                    self.lower_node_id(id)
-                };
-                let res = self.expect_full_res(id);
-                let res = self.lower_res(res);
+                let lowered_id = self.lower_node_id(id);
                 hir::VisibilityKind::Restricted {
-                    path: self.lower_path_extra(res, path, ParamMode::Explicit, explicit_owner),
+                    path: self.lower_path(id, path, ParamMode::Explicit),
                     hir_id: lowered_id,
                 }
             }
diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
index 66bcd698e6a..5ec060f6540 100644
--- a/compiler/rustc_ast_lowering/src/lib.rs
+++ b/compiler/rustc_ast_lowering/src/lib.rs
@@ -80,8 +80,6 @@ mod item;
 mod pat;
 mod path;
 
-const HIR_ID_COUNTER_LOCKED: u32 = 0xFFFFFFFF;
-
 rustc_hir::arena_types!(rustc_arena::declare_arena, 'tcx);
 
 struct LoweringContext<'a, 'hir: 'a> {
@@ -150,8 +148,8 @@ struct LoweringContext<'a, 'hir: 'a> {
     /// vector.
     in_scope_lifetimes: Vec<ParamName>,
 
-    current_hir_id_owner: (LocalDefId, u32),
-    item_local_id_counters: NodeMap<u32>,
+    current_hir_id_owner: LocalDefId,
+    item_local_id_counter: hir::ItemLocalId,
     node_id_to_hir_id: IndexVec<NodeId, Option<hir::HirId>>,
 
     allow_try_trait: Option<Lrc<[Symbol]>>,
@@ -330,8 +328,8 @@ pub fn lower_crate<'a, 'hir>(
         is_in_trait_impl: false,
         is_in_dyn_type: false,
         anonymous_lifetime_mode: AnonymousLifetimeMode::PassThrough,
-        current_hir_id_owner: (CRATE_DEF_ID, 0),
-        item_local_id_counters: Default::default(),
+        current_hir_id_owner: CRATE_DEF_ID,
+        item_local_id_counter: hir::ItemLocalId::new(0),
         node_id_to_hir_id: IndexVec::new(),
         generator_kind: None,
         task_context: None,
@@ -412,15 +410,15 @@ enum AnonymousLifetimeMode {
 
 impl<'a, 'hir> LoweringContext<'a, 'hir> {
     fn lower_crate(mut self, c: &Crate) -> &'hir hir::Crate<'hir> {
-        self.lower_node_id(CRATE_NODE_ID);
-        debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == Some(hir::CRATE_HIR_ID));
+        debug_assert_eq!(self.resolver.local_def_id(CRATE_NODE_ID), CRATE_DEF_ID);
 
         visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c);
 
-        let module = self.arena.alloc(self.lower_mod(&c.items, c.span));
-        self.lower_attrs(hir::CRATE_HIR_ID, &c.attrs);
-        self.owners.ensure_contains_elem(CRATE_DEF_ID, || None);
-        self.owners[CRATE_DEF_ID] = Some(hir::OwnerNode::Crate(module));
+        self.with_hir_id_owner(CRATE_NODE_ID, |lctx| {
+            let module = lctx.lower_mod(&c.items, c.span);
+            lctx.lower_attrs(hir::CRATE_HIR_ID, &c.attrs);
+            hir::OwnerNode::Crate(lctx.arena.alloc(module))
+        });
 
         let mut trait_map: FxHashMap<_, FxHashMap<_, _>> = FxHashMap::default();
         for (k, v) in self.resolver.take_trait_map().into_iter() {
@@ -456,47 +454,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         self.arena.alloc(krate)
     }
 
-    fn insert_item(&mut self, item: hir::Item<'hir>) -> hir::ItemId {
-        let id = item.item_id();
-        let item = self.arena.alloc(item);
-        self.owners.ensure_contains_elem(id.def_id, || None);
-        self.owners[id.def_id] = Some(hir::OwnerNode::Item(item));
-        id
-    }
-
-    fn insert_foreign_item(&mut self, item: hir::ForeignItem<'hir>) -> hir::ForeignItemId {
-        let id = item.foreign_item_id();
-        let item = self.arena.alloc(item);
-        self.owners.ensure_contains_elem(id.def_id, || None);
-        self.owners[id.def_id] = Some(hir::OwnerNode::ForeignItem(item));
-        id
-    }
-
-    fn insert_impl_item(&mut self, item: hir::ImplItem<'hir>) -> hir::ImplItemId {
-        let id = item.impl_item_id();
-        let item = self.arena.alloc(item);
-        self.owners.ensure_contains_elem(id.def_id, || None);
-        self.owners[id.def_id] = Some(hir::OwnerNode::ImplItem(item));
-        id
-    }
-
-    fn insert_trait_item(&mut self, item: hir::TraitItem<'hir>) -> hir::TraitItemId {
-        let id = item.trait_item_id();
-        let item = self.arena.alloc(item);
-        self.owners.ensure_contains_elem(id.def_id, || None);
-        self.owners[id.def_id] = Some(hir::OwnerNode::TraitItem(item));
-        id
-    }
-
-    fn allocate_hir_id_counter(&mut self, owner: NodeId) -> LocalDefId {
-        // Set up the counter if needed.
-        self.item_local_id_counters.entry(owner).or_insert(0);
-        // Always allocate the first `HirId` for the owner itself.
-        let lowered = self.lower_node_id_with_owner(owner, owner);
-        debug_assert_eq!(lowered.local_id.as_u32(), 0);
-        lowered.owner
-    }
-
     fn create_stable_hashing_context(&self) -> LoweringHasher<'_> {
         LoweringHasher {
             source_map: CachingSourceMapView::new(self.sess.source_map()),
@@ -504,47 +461,33 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
     }
 
-    fn lower_node_id_generic(
+    fn with_hir_id_owner(
         &mut self,
-        ast_node_id: NodeId,
-        alloc_hir_id: impl FnOnce(&mut Self) -> hir::HirId,
-    ) -> hir::HirId {
-        assert_ne!(ast_node_id, DUMMY_NODE_ID);
-
-        let min_size = ast_node_id.as_usize() + 1;
+        owner: NodeId,
+        f: impl FnOnce(&mut Self) -> hir::OwnerNode<'hir>,
+    ) -> LocalDefId {
+        let def_id = self.resolver.local_def_id(owner);
 
-        if min_size > self.node_id_to_hir_id.len() {
-            self.node_id_to_hir_id.resize(min_size, None);
+        // Always allocate the first `HirId` for the owner itself.
+        self.node_id_to_hir_id.ensure_contains_elem(owner, || None);
+        if let Some(_lowered) = self.node_id_to_hir_id[owner] {
+            panic!("with_hir_id_owner must not be called multiple times on owner {:?}", def_id);
         }
+        self.node_id_to_hir_id[owner] = Some(hir::HirId::make_owner(def_id));
 
-        if let Some(existing_hir_id) = self.node_id_to_hir_id[ast_node_id] {
-            existing_hir_id
-        } else {
-            // Generate a new `HirId`.
-            let hir_id = alloc_hir_id(self);
-            self.node_id_to_hir_id[ast_node_id] = Some(hir_id);
+        let current_owner = std::mem::replace(&mut self.current_hir_id_owner, def_id);
+        let current_local_counter =
+            std::mem::replace(&mut self.item_local_id_counter, hir::ItemLocalId::new(1));
 
-            hir_id
-        }
-    }
+        let item = f(self);
 
-    fn with_hir_id_owner<T>(&mut self, owner: NodeId, f: impl FnOnce(&mut Self) -> T) -> T {
-        let counter = self
-            .item_local_id_counters
-            .insert(owner, HIR_ID_COUNTER_LOCKED)
-            .unwrap_or_else(|| panic!("no `item_local_id_counters` entry for {:?}", owner));
-        let def_id = self.resolver.local_def_id(owner);
-        let old_owner = std::mem::replace(&mut self.current_hir_id_owner, (def_id, counter));
-        let ret = f(self);
-        let (new_def_id, new_counter) =
-            std::mem::replace(&mut self.current_hir_id_owner, old_owner);
+        self.current_hir_id_owner = current_owner;
+        self.item_local_id_counter = current_local_counter;
 
-        debug_assert!(def_id == new_def_id);
-        debug_assert!(new_counter >= counter);
+        self.owners.ensure_contains_elem(def_id, || None);
+        self.owners[def_id] = Some(item);
 
-        let prev = self.item_local_id_counters.insert(owner, new_counter).unwrap();
-        debug_assert!(prev == HIR_ID_COUNTER_LOCKED);
-        ret
+        def_id
     }
 
     /// This method allocates a new `HirId` for the given `NodeId` and stores it in
@@ -554,35 +497,20 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     /// `HirIdValidator` later on, which makes sure that all `NodeId`s got mapped
     /// properly. Calling the method twice with the same `NodeId` is fine though.
     fn lower_node_id(&mut self, ast_node_id: NodeId) -> hir::HirId {
-        self.lower_node_id_generic(ast_node_id, |this| {
-            let &mut (owner, ref mut local_id_counter) = &mut this.current_hir_id_owner;
-            let local_id = *local_id_counter;
-            *local_id_counter += 1;
-            hir::HirId { owner, local_id: hir::ItemLocalId::from_u32(local_id) }
-        })
-    }
-
-    fn lower_node_id_with_owner(&mut self, ast_node_id: NodeId, owner: NodeId) -> hir::HirId {
-        self.lower_node_id_generic(ast_node_id, |this| {
-            let local_id_counter = this
-                .item_local_id_counters
-                .get_mut(&owner)
-                .expect("called `lower_node_id_with_owner` before `allocate_hir_id_counter`");
-            let local_id = *local_id_counter;
-
-            // We want to be sure not to modify the counter in the map while it
-            // is also on the stack. Otherwise we'll get lost updates when writing
-            // back from the stack to the map.
-            debug_assert!(local_id != HIR_ID_COUNTER_LOCKED);
-
-            *local_id_counter += 1;
-            let owner = this.resolver.opt_local_def_id(owner).expect(
-                "you forgot to call `create_def` or are lowering node-IDs \
-                 that do not belong to the current owner",
-            );
+        assert_ne!(ast_node_id, DUMMY_NODE_ID);
 
-            hir::HirId { owner, local_id: hir::ItemLocalId::from_u32(local_id) }
-        })
+        self.node_id_to_hir_id.ensure_contains_elem(ast_node_id, || None);
+        if let Some(existing_hir_id) = self.node_id_to_hir_id[ast_node_id] {
+            existing_hir_id
+        } else {
+            // Generate a new `HirId`.
+            let owner = self.current_hir_id_owner;
+            let local_id = self.item_local_id_counter;
+            self.item_local_id_counter.increment_by(1);
+            let hir_id = hir::HirId { owner, local_id };
+            self.node_id_to_hir_id[ast_node_id] = Some(hir_id);
+            hir_id
+        }
     }
 
     fn next_id(&mut self) -> hir::HirId {
@@ -592,7 +520,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
     fn lower_res(&mut self, res: Res<NodeId>) -> Res {
         res.map_id(|id| {
-            self.lower_node_id_generic(id, |_| {
+            self.node_id_to_hir_id.get(id).copied().flatten().unwrap_or_else(|| {
                 panic!("expected `NodeId` to be lowered already for res {:#?}", res);
             })
         })
@@ -655,7 +583,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     /// Mark a span as relative to the current owning item.
     fn lower_span(&self, span: Span) -> Span {
         if self.sess.opts.debugging_opts.incremental_relative_spans {
-            span.with_parent(Some(self.current_hir_id_owner.0))
+            span.with_parent(Some(self.current_hir_id_owner))
         } else {
             // Do not make spans relative when not using incremental compilation.
             span
@@ -828,7 +756,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     // wouldn't have been added yet.
                     let generics = this.lower_generics_mut(
                         generics,
-                        ImplTraitContext::Universal(&mut params, this.current_hir_id_owner.0),
+                        ImplTraitContext::Universal(&mut params, this.current_hir_id_owner),
                     );
                     let res = f(this, &mut params);
                     (params, (generics, res))
@@ -1034,7 +962,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             }
             AssocTyConstraintKind::Bound { ref bounds } => {
                 let mut capturable_lifetimes;
-                let mut parent_def_id = self.current_hir_id_owner.0;
+                let mut parent_def_id = self.current_hir_id_owner;
                 // Piggy-back on the `impl Trait` context to figure out the correct behavior.
                 let (desugar_to_impl_trait, itctx) = match itctx {
                     // We are in the return position:
@@ -1162,7 +1090,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
                                 // Construct an AnonConst where the expr is the "ty"'s path.
 
-                                let parent_def_id = self.current_hir_id_owner.0;
+                                let parent_def_id = self.current_hir_id_owner;
                                 let node_id = self.resolver.next_node_id();
 
                                 // Add a definition for the in-band const def.
@@ -1428,12 +1356,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         // frequently opened issues show.
         let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::OpaqueTy, span, None);
 
-        let opaque_ty_def_id = self.allocate_hir_id_counter(opaque_ty_node_id);
+        let opaque_ty_def_id = self.resolver.local_def_id(opaque_ty_node_id);
 
-        let collected_lifetimes = self.with_hir_id_owner(opaque_ty_node_id, move |lctx| {
+        let mut collected_lifetimes = Vec::new();
+        self.with_hir_id_owner(opaque_ty_node_id, |lctx| {
             let hir_bounds = lower_bounds(lctx);
 
-            let collected_lifetimes = lifetimes_from_impl_trait_bounds(
+            collected_lifetimes = lifetimes_from_impl_trait_bounds(
                 opaque_ty_node_id,
                 &hir_bounds,
                 capturable_lifetimes,
@@ -1486,9 +1415,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             };
 
             trace!("lower_opaque_impl_trait: {:#?}", opaque_ty_def_id);
-            lctx.generate_opaque_type(opaque_ty_def_id, opaque_ty_item, span, opaque_ty_span);
-
-            collected_lifetimes
+            lctx.generate_opaque_type(opaque_ty_def_id, opaque_ty_item, span, opaque_ty_span)
         });
 
         let lifetimes =
@@ -1510,7 +1437,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         opaque_ty_item: hir::OpaqueTy<'hir>,
         span: Span,
         opaque_ty_span: Span,
-    ) {
+    ) -> hir::OwnerNode<'hir> {
         let opaque_ty_item_kind = hir::ItemKind::OpaqueTy(opaque_ty_item);
         // Generate an `type Foo = impl Trait;` declaration.
         trace!("registering opaque type with id {:#?}", opaque_ty_id);
@@ -1521,11 +1448,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             vis: respan(self.lower_span(span.shrink_to_lo()), hir::VisibilityKind::Inherited),
             span: self.lower_span(opaque_ty_span),
         };
-
-        // Insert the item into the global item list. This usually happens
-        // automatically for all AST items. But this opaque type item
-        // does not actually exist in the AST.
-        self.insert_item(opaque_ty_item);
+        hir::OwnerNode::Item(self.arena.alloc(opaque_ty_item))
     }
 
     fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> &'hir [Ident] {
@@ -1594,7 +1517,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 if let Some((_, ibty)) = &mut in_band_ty_params {
                     this.lower_ty_direct(
                         &param.ty,
-                        ImplTraitContext::Universal(ibty, this.current_hir_id_owner.0),
+                        ImplTraitContext::Universal(ibty, this.current_hir_id_owner),
                     )
                 } else {
                     this.lower_ty_direct(&param.ty, ImplTraitContext::disallowed())
@@ -1685,7 +1608,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
         let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::Async, span, None);
 
-        let opaque_ty_def_id = self.allocate_hir_id_counter(opaque_ty_node_id);
+        let opaque_ty_def_id = self.resolver.local_def_id(opaque_ty_node_id);
 
         // When we create the opaque type for this async fn, it is going to have
         // to capture all the lifetimes involved in the signature (including in the
@@ -1735,7 +1658,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         // grow.
         let input_lifetimes_count = self.in_scope_lifetimes.len() + self.lifetimes_to_define.len();
 
-        let lifetime_params = self.with_hir_id_owner(opaque_ty_node_id, |this| {
+        let mut lifetime_params = Vec::new();
+        self.with_hir_id_owner(opaque_ty_node_id, |this| {
             // We have to be careful to get elision right here. The
             // idea is that we create a lifetime parameter for each
             // lifetime in the return type.  So, given a return type
@@ -1757,7 +1681,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             //
             // Note: this must be done after lowering the output type,
             // as the output type may introduce new in-band lifetimes.
-            let lifetime_params: Vec<(Span, ParamName)> = this
+            lifetime_params = this
                 .in_scope_lifetimes
                 .iter()
                 .cloned()
@@ -1786,9 +1710,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             };
 
             trace!("exist ty from async fn def id: {:#?}", opaque_ty_def_id);
-            this.generate_opaque_type(opaque_ty_def_id, opaque_ty_item, span, opaque_ty_span);
-
-            lifetime_params
+            this.generate_opaque_type(opaque_ty_def_id, opaque_ty_item, span, opaque_ty_span)
         });
 
         // As documented above on the variable
diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs
index 90a22b5c209..929f427484d 100644
--- a/compiler/rustc_ast_lowering/src/path.rs
+++ b/compiler/rustc_ast_lowering/src/path.rs
@@ -99,7 +99,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         num_lifetimes,
                         parenthesized_generic_args,
                         itctx.reborrow(),
-                        None,
                     )
                 },
             )),
@@ -147,7 +146,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 0,
                 ParenthesizedGenericArgs::Err,
                 itctx.reborrow(),
-                None,
             ));
             let qpath = hir::QPath::TypeRelative(ty, hir_segment);
 
@@ -178,7 +176,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         res: Res,
         p: &Path,
         param_mode: ParamMode,
-        explicit_owner: Option<NodeId>,
     ) -> &'hir hir::Path<'hir> {
         self.arena.alloc(hir::Path {
             res,
@@ -190,7 +187,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     0,
                     ParenthesizedGenericArgs::Err,
                     ImplTraitContext::disallowed(),
-                    explicit_owner,
                 )
             })),
             span: self.lower_span(p.span),
@@ -205,7 +201,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     ) -> &'hir hir::Path<'hir> {
         let res = self.expect_full_res(id);
         let res = self.lower_res(res);
-        self.lower_path_extra(res, p, param_mode, None)
+        self.lower_path_extra(res, p, param_mode)
     }
 
     crate fn lower_path_segment(
@@ -216,7 +212,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         expected_lifetimes: usize,
         parenthesized_generic_args: ParenthesizedGenericArgs,
         itctx: ImplTraitContext<'_, 'hir>,
-        explicit_owner: Option<NodeId>,
     ) -> hir::PathSegment<'hir> {
         debug!(
             "path_span: {:?}, lower_path_segment(segment: {:?}, expected_lifetimes: {:?})",
@@ -354,11 +349,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
 
         let res = self.expect_full_res(segment.id);
-        let id = if let Some(owner) = explicit_owner {
-            self.lower_node_id_with_owner(segment.id, owner)
-        } else {
-            self.lower_node_id(segment.id)
-        };
+        let id = self.lower_node_id(segment.id);
         debug!(
             "lower_path_segment: ident={:?} original-id={:?} new-id={:?}",
             segment.ident, segment.id, id,
diff --git a/compiler/rustc_hir/src/arena.rs b/compiler/rustc_hir/src/arena.rs
index 3e8b98e9f54..f07e52e04da 100644
--- a/compiler/rustc_hir/src/arena.rs
+++ b/compiler/rustc_hir/src/arena.rs
@@ -28,9 +28,9 @@ macro_rules! arena_types {
             [] pat_field: rustc_hir::PatField<$tcx>,
             [] fn_decl: rustc_hir::FnDecl<$tcx>,
             [] foreign_item: rustc_hir::ForeignItem<$tcx>,
-            [few] foreign_item_ref: rustc_hir::ForeignItemRef<$tcx>,
+            [few] foreign_item_ref: rustc_hir::ForeignItemRef,
             [] impl_item: rustc_hir::ImplItem<$tcx>,
-            [] impl_item_ref: rustc_hir::ImplItemRef<$tcx>,
+            [] impl_item_ref: rustc_hir::ImplItemRef,
             [] item: rustc_hir::Item<$tcx>,
             [few] inline_asm: rustc_hir::InlineAsm<$tcx>,
             [few] llvm_inline_asm: rustc_hir::LlvmInlineAsm<$tcx>,
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
index f5fc693ce25..05b652fd5af 100644
--- a/compiler/rustc_hir/src/hir.rs
+++ b/compiler/rustc_hir/src/hir.rs
@@ -2745,7 +2745,7 @@ pub enum ItemKind<'hir> {
     /// A module.
     Mod(Mod<'hir>),
     /// An external module, e.g. `extern { .. }`.
-    ForeignMod { abi: Abi, items: &'hir [ForeignItemRef<'hir>] },
+    ForeignMod { abi: Abi, items: &'hir [ForeignItemRef] },
     /// Module-level inline assembly (from `global_asm!`).
     GlobalAsm(&'hir InlineAsm<'hir>),
     /// A type alias, e.g., `type Foo = Bar<u8>`.
@@ -2782,7 +2782,7 @@ pub struct Impl<'hir> {
     pub of_trait: Option<TraitRef<'hir>>,
 
     pub self_ty: &'hir Ty<'hir>,
-    pub items: &'hir [ImplItemRef<'hir>],
+    pub items: &'hir [ImplItemRef],
 }
 
 impl ItemKind<'_> {
@@ -2846,13 +2846,12 @@ pub struct TraitItemRef {
 /// passes to find the impl they want without loading the ID (which
 /// means fewer edges in the incremental compilation graph).
 #[derive(Debug, HashStable_Generic)]
-pub struct ImplItemRef<'hir> {
+pub struct ImplItemRef {
     pub id: ImplItemId,
     #[stable_hasher(project(name))]
     pub ident: Ident,
     pub kind: AssocItemKind,
     pub span: Span,
-    pub vis: Visibility<'hir>,
     pub defaultness: Defaultness,
 }
 
@@ -2886,12 +2885,11 @@ impl ForeignItemId {
 /// passes to find the impl they want without loading the ID (which
 /// means fewer edges in the incremental compilation graph).
 #[derive(Debug, HashStable_Generic)]
-pub struct ForeignItemRef<'hir> {
+pub struct ForeignItemRef {
     pub id: ForeignItemId,
     #[stable_hasher(project(name))]
     pub ident: Ident,
     pub span: Span,
-    pub vis: Visibility<'hir>,
 }
 
 #[derive(Debug)]
diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs
index 137782a6dc7..1ac2625dd47 100644
--- a/compiler/rustc_hir/src/intravisit.rs
+++ b/compiler/rustc_hir/src/intravisit.rs
@@ -392,10 +392,10 @@ pub trait Visitor<'v>: Sized {
     fn visit_impl_item(&mut self, ii: &'v ImplItem<'v>) {
         walk_impl_item(self, ii)
     }
-    fn visit_foreign_item_ref(&mut self, ii: &'v ForeignItemRef<'v>) {
+    fn visit_foreign_item_ref(&mut self, ii: &'v ForeignItemRef) {
         walk_foreign_item_ref(self, ii)
     }
-    fn visit_impl_item_ref(&mut self, ii: &'v ImplItemRef<'v>) {
+    fn visit_impl_item_ref(&mut self, ii: &'v ImplItemRef) {
         walk_impl_item_ref(self, ii)
     }
     fn visit_trait_ref(&mut self, t: &'v TraitRef<'v>) {
@@ -1042,22 +1042,20 @@ pub fn walk_impl_item<'v, V: Visitor<'v>>(visitor: &mut V, impl_item: &'v ImplIt
 
 pub fn walk_foreign_item_ref<'v, V: Visitor<'v>>(
     visitor: &mut V,
-    foreign_item_ref: &'v ForeignItemRef<'v>,
+    foreign_item_ref: &'v ForeignItemRef,
 ) {
     // N.B., deliberately force a compilation error if/when new fields are added.
-    let ForeignItemRef { id, ident, span: _, ref vis } = *foreign_item_ref;
+    let ForeignItemRef { id, ident, span: _ } = *foreign_item_ref;
     visitor.visit_nested_foreign_item(id);
     visitor.visit_ident(ident);
-    visitor.visit_vis(vis);
 }
 
-pub fn walk_impl_item_ref<'v, V: Visitor<'v>>(visitor: &mut V, impl_item_ref: &'v ImplItemRef<'v>) {
+pub fn walk_impl_item_ref<'v, V: Visitor<'v>>(visitor: &mut V, impl_item_ref: &'v ImplItemRef) {
     // N.B., deliberately force a compilation error if/when new fields are added.
-    let ImplItemRef { id, ident, ref kind, span: _, ref vis, ref defaultness } = *impl_item_ref;
+    let ImplItemRef { id, ident, ref kind, span: _, ref defaultness } = *impl_item_ref;
     visitor.visit_nested_impl_item(id);
     visitor.visit_ident(ident);
     visitor.visit_associated_item_kind(kind);
-    visitor.visit_vis(vis);
     visitor.visit_defaultness(defaultness);
 }
 
diff --git a/compiler/rustc_metadata/src/native_libs.rs b/compiler/rustc_metadata/src/native_libs.rs
index 5f0d8c46f20..5ad55dbf5c8 100644
--- a/compiler/rustc_metadata/src/native_libs.rs
+++ b/compiler/rustc_metadata/src/native_libs.rs
@@ -382,7 +382,7 @@ impl Collector<'tcx> {
         }
     }
 
-    fn i686_arg_list_size(&self, item: &hir::ForeignItemRef<'_>) -> usize {
+    fn i686_arg_list_size(&self, item: &hir::ForeignItemRef) -> usize {
         let argument_types: &List<Ty<'_>> = self.tcx.erase_late_bound_regions(
             self.tcx
                 .type_of(item.id.def_id)
@@ -406,7 +406,7 @@ impl Collector<'tcx> {
             .sum()
     }
 
-    fn build_dll_import(&self, abi: Abi, item: &hir::ForeignItemRef<'_>) -> DllImport {
+    fn build_dll_import(&self, abi: Abi, item: &hir::ForeignItemRef) -> DllImport {
         let calling_convention = if self.tcx.sess.target.arch == "x86" {
             match abi {
                 Abi::C { .. } | Abi::Cdecl => DllCallingConvention::C,
diff --git a/compiler/rustc_middle/src/hir/map/collector.rs b/compiler/rustc_middle/src/hir/map/collector.rs
index 082948eba41..5ecb1c9b0ff 100644
--- a/compiler/rustc_middle/src/hir/map/collector.rs
+++ b/compiler/rustc_middle/src/hir/map/collector.rs
@@ -413,18 +413,18 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         self.visit_nested_trait_item(id);
     }
 
-    fn visit_impl_item_ref(&mut self, ii: &'hir ImplItemRef<'hir>) {
+    fn visit_impl_item_ref(&mut self, ii: &'hir ImplItemRef) {
         // Do not visit the duplicate information in ImplItemRef. We want to
         // map the actual nodes, not the duplicate ones in the *Ref.
-        let ImplItemRef { id, ident: _, kind: _, span: _, vis: _, defaultness: _ } = *ii;
+        let ImplItemRef { id, ident: _, kind: _, span: _, defaultness: _ } = *ii;
 
         self.visit_nested_impl_item(id);
     }
 
-    fn visit_foreign_item_ref(&mut self, fi: &'hir ForeignItemRef<'hir>) {
+    fn visit_foreign_item_ref(&mut self, fi: &'hir ForeignItemRef) {
         // Do not visit the duplicate information in ForeignItemRef. We want to
         // map the actual nodes, not the duplicate ones in the *Ref.
-        let ForeignItemRef { id, ident: _, span: _, vis: _ } = *fi;
+        let ForeignItemRef { id, ident: _, span: _ } = *fi;
 
         self.visit_nested_foreign_item(id);
     }
diff --git a/compiler/rustc_passes/src/hir_id_validator.rs b/compiler/rustc_passes/src/hir_id_validator.rs
index eff1096c855..0e60ca9f900 100644
--- a/compiler/rustc_passes/src/hir_id_validator.rs
+++ b/compiler/rustc_passes/src/hir_id_validator.rs
@@ -163,14 +163,14 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> {
         self.hir_ids_seen.insert(hir_id.local_id);
     }
 
-    fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef<'hir>) {
+    fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef) {
         // Explicitly do nothing here. ImplItemRefs contain hir::Visibility
         // values that actually belong to an ImplItem instead of the ItemKind::Impl
         // we are currently in. So for those it's correct that they have a
         // different owner.
     }
 
-    fn visit_foreign_item_ref(&mut self, _: &'hir hir::ForeignItemRef<'hir>) {
+    fn visit_foreign_item_ref(&mut self, _: &'hir hir::ForeignItemRef) {
         // Explicitly do nothing here. ForeignItemRefs contain hir::Visibility
         // values that actually belong to an ForeignItem instead of the ItemKind::ForeignMod
         // we are currently in. So for those it's correct that they have a
diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs
index 391e4305423..e14f758ddae 100644
--- a/compiler/rustc_privacy/src/lib.rs
+++ b/compiler/rustc_privacy/src/lib.rs
@@ -743,7 +743,9 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {
             }
             hir::ItemKind::Impl(ref impl_) => {
                 for impl_item_ref in impl_.items {
-                    if impl_.of_trait.is_some() || impl_item_ref.vis.node.is_pub() {
+                    if impl_.of_trait.is_some()
+                        || self.tcx.visibility(impl_item_ref.id.def_id) == ty::Visibility::Public
+                    {
                         self.update(impl_item_ref.id.def_id, item_level);
                     }
                 }
@@ -768,7 +770,7 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {
             }
             hir::ItemKind::ForeignMod { items, .. } => {
                 for foreign_item in items {
-                    if foreign_item.vis.node.is_pub() {
+                    if self.tcx.visibility(foreign_item.id.def_id) == ty::Visibility::Public {
                         self.update(foreign_item.id.def_id, item_level);
                     }
                 }
@@ -1678,7 +1680,10 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
                     // methods will be visible as `Public::foo`.
                     let mut found_pub_static = false;
                     for impl_item_ref in impl_.items {
-                        if self.item_is_public(impl_item_ref.id.def_id, &impl_item_ref.vis) {
+                        if self.access_levels.is_reachable(impl_item_ref.id.def_id)
+                            || self.tcx.visibility(impl_item_ref.id.def_id)
+                                == ty::Visibility::Public
+                        {
                             let impl_item = self.tcx.hir().impl_item(impl_item_ref.id);
                             match impl_item_ref.kind {
                                 AssocItemKind::Const => {
diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs
index 75307f13563..611ff26d652 100644
--- a/compiler/rustc_trait_selection/src/traits/wf.rs
+++ b/compiler/rustc_trait_selection/src/traits/wf.rs
@@ -209,7 +209,7 @@ fn extend_cause_with_original_assoc_item_obligation<'tcx>(
         _ => return,
     };
     let fix_span =
-        |impl_item_ref: &hir::ImplItemRef<'_>| match tcx.hir().impl_item(impl_item_ref.id).kind {
+        |impl_item_ref: &hir::ImplItemRef| match tcx.hir().impl_item(impl_item_ref.id).kind {
             hir::ImplItemKind::Const(ty, _) | hir::ImplItemKind::TyAlias(ty) => ty.span,
             _ => impl_item_ref.span,
         };
diff --git a/compiler/rustc_ty_utils/src/ty.rs b/compiler/rustc_ty_utils/src/ty.rs
index 27ad7bf4c2d..3d3b2743700 100644
--- a/compiler/rustc_ty_utils/src/ty.rs
+++ b/compiler/rustc_ty_utils/src/ty.rs
@@ -100,7 +100,7 @@ fn associated_item_from_trait_item_ref(
 fn associated_item_from_impl_item_ref(
     tcx: TyCtxt<'_>,
     parent_def_id: LocalDefId,
-    impl_item_ref: &hir::ImplItemRef<'_>,
+    impl_item_ref: &hir::ImplItemRef,
 ) -> ty::AssocItem {
     let def_id = impl_item_ref.id.def_id;
     let (kind, has_self) = match impl_item_ref.kind {
diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs
index 54e4eb47688..d6c59312c0b 100644
--- a/compiler/rustc_typeck/src/check/check.rs
+++ b/compiler/rustc_typeck/src/check/check.rs
@@ -906,7 +906,7 @@ pub(super) fn check_impl_items_against_trait<'tcx>(
     full_impl_span: Span,
     impl_id: LocalDefId,
     impl_trait_ref: ty::TraitRef<'tcx>,
-    impl_item_refs: &[hir::ImplItemRef<'_>],
+    impl_item_refs: &[hir::ImplItemRef],
 ) {
     // If the trait reference itself is erroneous (so the compilation is going
     // to fail), skip checking the items here -- the `impl_item` table in `tcx`
diff --git a/compiler/rustc_typeck/src/coherence/inherent_impls.rs b/compiler/rustc_typeck/src/coherence/inherent_impls.rs
index c7be9e21235..f3fe09ac003 100644
--- a/compiler/rustc_typeck/src/coherence/inherent_impls.rs
+++ b/compiler/rustc_typeck/src/coherence/inherent_impls.rs
@@ -401,7 +401,7 @@ impl InherentCollect<'tcx> {
         lang: &str,
         ty: &str,
         span: Span,
-        assoc_items: &[hir::ImplItemRef<'_>],
+        assoc_items: &[hir::ImplItemRef],
     ) {
         match (lang_def_id, lang_def_id2) {
             (Some(lang_def_id), _) if lang_def_id == impl_def_id.to_def_id() => {
diff --git a/compiler/rustc_typeck/src/impl_wf_check.rs b/compiler/rustc_typeck/src/impl_wf_check.rs
index 9b23bf241cc..5d2f8fc4242 100644
--- a/compiler/rustc_typeck/src/impl_wf_check.rs
+++ b/compiler/rustc_typeck/src/impl_wf_check.rs
@@ -97,7 +97,7 @@ impl ItemLikeVisitor<'tcx> for ImplWfCheck<'tcx> {
 fn enforce_impl_params_are_constrained(
     tcx: TyCtxt<'_>,
     impl_def_id: LocalDefId,
-    impl_item_refs: &[hir::ImplItemRef<'_>],
+    impl_item_refs: &[hir::ImplItemRef],
 ) {
     // Every lifetime used in an associated type must be constrained.
     let impl_self_ty = tcx.type_of(impl_def_id);
@@ -228,7 +228,7 @@ fn report_unused_parameter(tcx: TyCtxt<'_>, span: Span, kind: &str, name: &str)
 }
 
 /// Enforce that we do not have two items in an impl with the same name.
-fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_item_refs: &[hir::ImplItemRef<'_>]) {
+fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_item_refs: &[hir::ImplItemRef]) {
     let mut seen_type_items = FxHashMap::default();
     let mut seen_value_items = FxHashMap::default();
     for impl_item_ref in impl_item_refs {
diff --git a/src/test/incremental/hashes/extern_mods.rs b/src/test/incremental/hashes/extern_mods.rs
index 783407a9e04..4bc98fd7cd0 100644
--- a/src/test/incremental/hashes/extern_mods.rs
+++ b/src/test/incremental/hashes/extern_mods.rs
@@ -143,13 +143,13 @@ extern "rust-call" {
 // Make function public --------------------------------------------------------
 #[cfg(any(cfail1,cfail4))]
 extern "C" {
-    fn make_function_public(c: i32);
+    fn     make_function_public(c: i32);
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(cfg = "cfail2", except = "hir_owner")]
+#[rustc_clean(cfg = "cfail2")]
 #[rustc_clean(cfg = "cfail3")]
-#[rustc_clean(cfg = "cfail5", except = "hir_owner")]
+#[rustc_clean(cfg = "cfail5")]
 #[rustc_clean(cfg = "cfail6")]
 extern "C" {
     pub fn make_function_public(c: i32);
diff --git a/src/test/incremental/hashes/inherent_impls.rs b/src/test/incremental/hashes/inherent_impls.rs
index d711cc20dd7..3a59377e819 100644
--- a/src/test/incremental/hashes/inherent_impls.rs
+++ b/src/test/incremental/hashes/inherent_impls.rs
@@ -116,20 +116,24 @@ impl Foo {
 // Change Method Privacy -------------------------------------------------------
 #[cfg(any(cfail1,cfail4))]
 impl Foo {
+    //------------------------------------------------------------------------------
+    //--------------------------
+    //------------------------------------------------------------------------------
+    //--------------------------
     pub fn method_privacy() { }
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(cfg="cfail2", except="hir_owner")]
+#[rustc_clean(cfg="cfail2")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner")]
+#[rustc_clean(cfg="cfail5")]
 #[rustc_clean(cfg="cfail6")]
 impl Foo {
     #[rustc_clean(cfg="cfail2", except="associated_item,hir_owner,hir_owner_nodes")]
     #[rustc_clean(cfg="cfail3")]
-    #[rustc_clean(cfg="cfail5", except="associated_item,hir_owner,hir_owner_nodes,optimized_mir")]
+    #[rustc_clean(cfg="cfail5", except="associated_item,hir_owner,hir_owner_nodes")]
     #[rustc_clean(cfg="cfail6")]
-    fn method_privacy() { }
+    fn     method_privacy() { }
 }
 
 // Change Method Selfness -----------------------------------------------------------
diff --git a/src/tools/clippy/clippy_lints/src/fallible_impl_from.rs b/src/tools/clippy/clippy_lints/src/fallible_impl_from.rs
index 7e4d1b3ef9f..f22f52b949e 100644
--- a/src/tools/clippy/clippy_lints/src/fallible_impl_from.rs
+++ b/src/tools/clippy/clippy_lints/src/fallible_impl_from.rs
@@ -65,7 +65,7 @@ impl<'tcx> LateLintPass<'tcx> for FallibleImplFrom {
     }
 }
 
-fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[hir::ImplItemRef<'_>]) {
+fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[hir::ImplItemRef]) {
     use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
     use rustc_hir::{Expr, ExprKind, ImplItemKind, QPath};