]> git.lizzy.rs Git - rust.git/blobdiff - compiler/rustc_privacy/src/lib.rs
Auto merge of #89459 - tspiteri:idiv-overflow-bitand, r=kennytm
[rust.git] / compiler / rustc_privacy / src / lib.rs
index 1a0510d23cf1366c9e9273ecd492216f1645ec21..ae3a9c71c59686fc23e0120cff1abade2dfe610f 100644 (file)
@@ -6,26 +6,28 @@
 #![feature(associated_type_defaults)]
 #![recursion_limit = "256"]
 
+use rustc_ast::MacroDef;
 use rustc_attr as attr;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::struct_span_err;
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, CRATE_DEF_INDEX, LOCAL_CRATE};
+use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet};
+use rustc_hir::def_id::{CRATE_DEF_ID, CRATE_DEF_INDEX, LOCAL_CRATE};
 use rustc_hir::intravisit::{self, DeepVisitor, NestedVisitorMap, Visitor};
 use rustc_hir::{AssocItemKind, HirIdSet, Node, PatKind};
 use rustc_middle::bug;
 use rustc_middle::hir::map::Map;
 use rustc_middle::middle::privacy::{AccessLevel, AccessLevels};
-use rustc_middle::mir::abstract_const::Node as ACNode;
 use rustc_middle::span_bug;
+use rustc_middle::thir::abstract_const::Node as ACNode;
 use rustc_middle::ty::fold::TypeVisitor;
 use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::subst::{InternalSubsts, Subst};
 use rustc_middle::ty::{self, Const, GenericParamDefKind, TraitRef, Ty, TyCtxt, TypeFoldable};
 use rustc_session::lint;
 use rustc_span::hygiene::Transparency;
-use rustc_span::symbol::{kw, Ident};
+use rustc_span::symbol::{kw, sym, Ident};
 use rustc_span::Span;
 use rustc_trait_selection::traits::const_evaluatable::{self, AbstractConst};
 
@@ -122,7 +124,7 @@ fn visit_projection_ty(
 
     fn visit_predicate(&mut self, predicate: ty::Predicate<'tcx>) -> ControlFlow<V::BreakTy> {
         match predicate.kind().skip_binder() {
-            ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref }, _) => {
+            ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _ }) => {
                 self.visit_trait(trait_ref)
             }
             ty::PredicateKind::Projection(ty::ProjectionPredicate { projection_ty, ty }) => {
@@ -133,11 +135,11 @@ fn visit_predicate(&mut self, predicate: ty::Predicate<'tcx>) -> ControlFlow<V::
                 ty.visit_with(self)
             }
             ty::PredicateKind::RegionOutlives(..) => ControlFlow::CONTINUE,
-            ty::PredicateKind::ConstEvaluatable(defs, substs)
-                if self.def_id_visitor.tcx().features().const_evaluatable_checked =>
+            ty::PredicateKind::ConstEvaluatable(uv)
+                if self.def_id_visitor.tcx().features().generic_const_exprs =>
             {
                 let tcx = self.def_id_visitor.tcx();
-                if let Ok(Some(ct)) = AbstractConst::new(tcx, defs, substs) {
+                if let Ok(Some(ct)) = AbstractConst::new(tcx, uv) {
                     self.visit_abstract_const_expr(tcx, ct)?;
                 }
                 ControlFlow::CONTINUE
@@ -178,6 +180,10 @@ impl<'tcx, V> TypeVisitor<'tcx> for DefIdVisitorSkeleton<'_, 'tcx, V>
 {
     type BreakTy = V::BreakTy;
 
+    fn tcx_for_anon_const_substs(&self) -> Option<TyCtxt<'tcx>> {
+        Some(self.def_id_visitor.tcx())
+    }
+
     fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<V::BreakTy> {
         let tcx = self.def_id_visitor.tcx();
         // InternalSubsts are not visited here because they are visited below in `super_visit_with`.
@@ -354,9 +360,8 @@ trait VisibilityLike: Sized {
 
     // Returns an over-approximation (`skip_assoc_tys` = true) of visibility due to
     // associated types for which we can't determine visibility precisely.
-    fn of_impl(hir_id: hir::HirId, tcx: TyCtxt<'_>, access_levels: &AccessLevels) -> Self {
+    fn of_impl(def_id: LocalDefId, tcx: TyCtxt<'_>, access_levels: &AccessLevels) -> Self {
         let mut find = FindMin { tcx, access_levels, min: Self::MAX };
-        let def_id = tcx.hir().local_def_id(hir_id);
         find.visit(tcx.type_of(def_id));
         if let Some(trait_ref) = tcx.impl_trait_ref(def_id) {
             find.visit_trait(trait_ref);
@@ -424,7 +429,7 @@ struct EmbargoVisitor<'tcx> {
 
 struct ReachEverythingInTheInterfaceVisitor<'a, 'tcx> {
     access_level: Option<AccessLevel>,
-    item_def_id: DefId,
+    item_def_id: LocalDefId,
     ev: &'a mut EmbargoVisitor<'tcx>,
 }
 
@@ -448,16 +453,53 @@ fn update(&mut self, def_id: LocalDefId, level: Option<AccessLevel>) -> Option<A
 
     fn reach(
         &mut self,
-        item_id: hir::HirId,
+        def_id: LocalDefId,
         access_level: Option<AccessLevel>,
     ) -> ReachEverythingInTheInterfaceVisitor<'_, 'tcx> {
         ReachEverythingInTheInterfaceVisitor {
             access_level: cmp::min(access_level, Some(AccessLevel::Reachable)),
-            item_def_id: self.tcx.hir().local_def_id(item_id).to_def_id(),
+            item_def_id: def_id,
             ev: self,
         }
     }
 
+    // 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) {
+        // 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 {
+            return;
+        }
+
+        let item_def_id = local_def_id.to_def_id();
+        let macro_module_def_id =
+            ty::DefIdTree::parent(self.tcx, item_def_id).unwrap().expect_local();
+        if self.tcx.hir().opt_def_kind(macro_module_def_id) != Some(DefKind::Mod) {
+            // The macro's parent doesn't correspond to a `mod`, return early (#63164, #65252).
+            return;
+        }
+
+        if self.get(local_def_id).is_none() {
+            return;
+        }
+
+        // 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;
+        loop {
+            let changed_reachability =
+                self.update_macro_reachable(module_def_id, macro_module_def_id);
+            if changed_reachability || module_def_id == CRATE_DEF_ID {
+                break;
+            }
+            module_def_id =
+                ty::DefIdTree::parent(self.tcx, module_def_id.to_def_id()).unwrap().expect_local();
+        }
+    }
+
     /// Updates the item as being reachable through a macro defined in the given
     /// module. Returns `true` if the level has changed.
     fn update_macro_reachable(
@@ -507,16 +549,26 @@ fn update_macro_reachable_def(
         }
         match def_kind {
             // No type privacy, so can be directly marked as reachable.
-            DefKind::Const
-            | DefKind::Macro(_)
-            | DefKind::Static
-            | DefKind::TraitAlias
-            | DefKind::TyAlias => {
+            DefKind::Const | DefKind::Static | DefKind::TraitAlias | DefKind::TyAlias => {
                 if vis.is_accessible_from(module.to_def_id(), self.tcx) {
                     self.update(def_id, level);
                 }
             }
 
+            // Hygine isn't really implemented for `macro_rules!` macros at the
+            // moment. Accordingly, marking them as reachable is unwise. `macro` macros
+            // have normal  hygine, so we can treat them like other items without type
+            // privacy and mark them reachable.
+            DefKind::Macro(_) => {
+                let hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id);
+                let item = self.tcx.hir().expect_item(hir_id);
+                if let hir::ItemKind::Macro(MacroDef { macro_rules: false, .. }) = item.kind {
+                    if vis.is_accessible_from(module.to_def_id(), self.tcx) {
+                        self.update(def_id, level);
+                    }
+                }
+            }
+
             // We can't use a module name as the final segment of a path, except
             // in use statements. Since re-export checking doesn't consider
             // hygiene these don't need to be marked reachable. The contents of
@@ -536,10 +588,10 @@ fn update_macro_reachable_def(
                     | hir::ItemKind::Union(ref struct_def, _) = item.kind
                     {
                         for field in struct_def.fields() {
-                            let field_vis =
-                                self.tcx.visibility(self.tcx.hir().local_def_id(field.hir_id));
+                            let def_id = self.tcx.hir().local_def_id(field.hir_id);
+                            let field_vis = self.tcx.visibility(def_id);
                             if field_vis.is_accessible_from(module.to_def_id(), self.tcx) {
-                                self.reach(field.hir_id, level).ty();
+                                self.reach(def_id, level).ty();
                             }
                         }
                     } else {
@@ -575,7 +627,7 @@ fn update_macro_reachable_def(
         }
     }
 
-    /// Given the path segments of a `ItemKind::Use`, then we need
+    /// Given the path segments of an `ItemKind::Use`, then we need
     /// to update the visibility of the intermediate use so that it isn't linted
     /// by `unreachable_pub`.
     ///
@@ -638,7 +690,13 @@ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
     fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
         let inherited_item_level = match item.kind {
             hir::ItemKind::Impl { .. } => {
-                Option::<AccessLevel>::of_impl(item.hir_id(), self.tcx, &self.access_levels)
+                Option::<AccessLevel>::of_impl(item.def_id, self.tcx, &self.access_levels)
+            }
+            // Only exported `macro_rules!` items are public, but they always are.
+            hir::ItemKind::Macro(MacroDef { macro_rules: true, .. }) => {
+                let def_id = item.def_id.to_def_id();
+                let is_macro_export = self.tcx.has_attr(def_id, sym::macro_export);
+                if is_macro_export { Some(AccessLevel::Public) } else { None }
             }
             // Foreign modules inherit level from parents.
             hir::ItemKind::ForeignMod { .. } => self.prev_level,
@@ -648,6 +706,7 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             | hir::ItemKind::ExternCrate(..)
             | hir::ItemKind::GlobalAsm(..)
             | hir::ItemKind::Fn(..)
+            | hir::ItemKind::Macro(..)
             | hir::ItemKind::Mod(..)
             | hir::ItemKind::Static(..)
             | hir::ItemKind::Struct(..)
@@ -684,7 +743,9 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'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);
                     }
                 }
@@ -704,13 +765,17 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
                     }
                 }
             }
+            hir::ItemKind::Macro(ref macro_def) => {
+                self.update_reachability_from_macro(item.def_id, macro_def);
+            }
             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);
                     }
                 }
             }
+
             hir::ItemKind::OpaqueTy(..)
             | hir::ItemKind::Use(..)
             | hir::ItemKind::Static(..)
@@ -726,13 +791,13 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
         // Mark all items in interfaces of reachable items as reachable.
         match item.kind {
             // The interface is empty.
-            hir::ItemKind::ExternCrate(..) => {}
+            hir::ItemKind::Macro(..) | hir::ItemKind::ExternCrate(..) => {}
             // All nested items are checked by `visit_item`.
             hir::ItemKind::Mod(..) => {}
             // Re-exports are handled in `visit_mod`. However, in order to avoid looping over
             // all of the items of a mod in `visit_mod` looking for use statements, we handle
             // making sure that intermediate use statements have their visibilities updated here.
-            hir::ItemKind::Use(ref path, _) => {
+            hir::ItemKind::Use(path, _) => {
                 if item_level.is_some() {
                     self.update_visibility_of_intermediate_use_statements(path.segments.as_ref());
                 }
@@ -750,7 +815,7 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
                     // reachable if they are returned via `impl Trait`, even from private functions.
                     let exist_level =
                         cmp::max(item_level, Some(AccessLevel::ReachableFromImplTrait));
-                    self.reach(item.hir_id(), exist_level).generics().predicates().ty();
+                    self.reach(item.def_id, exist_level).generics().predicates().ty();
                 }
             }
             // Visit everything.
@@ -759,15 +824,15 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             | hir::ItemKind::Fn(..)
             | hir::ItemKind::TyAlias(..) => {
                 if item_level.is_some() {
-                    self.reach(item.hir_id(), item_level).generics().predicates().ty();
+                    self.reach(item.def_id, item_level).generics().predicates().ty();
                 }
             }
             hir::ItemKind::Trait(.., trait_item_refs) => {
                 if item_level.is_some() {
-                    self.reach(item.hir_id(), item_level).generics().predicates();
+                    self.reach(item.def_id, item_level).generics().predicates();
 
                     for trait_item_ref in trait_item_refs {
-                        let mut reach = self.reach(trait_item_ref.id.hir_id(), item_level);
+                        let mut reach = self.reach(trait_item_ref.id.def_id, item_level);
                         reach.generics().predicates();
 
                         if trait_item_ref.kind == AssocItemKind::Type
@@ -782,18 +847,18 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             }
             hir::ItemKind::TraitAlias(..) => {
                 if item_level.is_some() {
-                    self.reach(item.hir_id(), item_level).generics().predicates();
+                    self.reach(item.def_id, item_level).generics().predicates();
                 }
             }
             // Visit everything except for private impl items.
             hir::ItemKind::Impl(ref impl_) => {
                 if item_level.is_some() {
-                    self.reach(item.hir_id(), item_level).generics().predicates().ty().trait_ref();
+                    self.reach(item.def_id, item_level).generics().predicates().ty().trait_ref();
 
                     for impl_item_ref in impl_.items {
                         let impl_item_level = self.get(impl_item_ref.id.def_id);
                         if impl_item_level.is_some() {
-                            self.reach(impl_item_ref.id.hir_id(), impl_item_level)
+                            self.reach(impl_item_ref.id.def_id, impl_item_level)
                                 .generics()
                                 .predicates()
                                 .ty();
@@ -805,13 +870,14 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             // Visit everything, but enum variants have their own levels.
             hir::ItemKind::Enum(ref def, _) => {
                 if item_level.is_some() {
-                    self.reach(item.hir_id(), item_level).generics().predicates();
+                    self.reach(item.def_id, item_level).generics().predicates();
                 }
                 for variant in def.variants {
                     let variant_level = self.get(self.tcx.hir().local_def_id(variant.id));
                     if variant_level.is_some() {
                         for field in variant.data.fields() {
-                            self.reach(field.hir_id, variant_level).ty();
+                            self.reach(self.tcx.hir().local_def_id(field.hir_id), variant_level)
+                                .ty();
                         }
                         // Corner case: if the variant is reachable, but its
                         // enum is not, make the enum reachable as well.
@@ -824,7 +890,7 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
                 for foreign_item in items {
                     let foreign_item_level = self.get(foreign_item.id.def_id);
                     if foreign_item_level.is_some() {
-                        self.reach(foreign_item.id.hir_id(), foreign_item_level)
+                        self.reach(foreign_item.id.def_id, foreign_item_level)
                             .generics()
                             .predicates()
                             .ty();
@@ -834,11 +900,12 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             // Visit everything except for private fields.
             hir::ItemKind::Struct(ref struct_def, _) | hir::ItemKind::Union(ref struct_def, _) => {
                 if item_level.is_some() {
-                    self.reach(item.hir_id(), item_level).generics().predicates();
+                    self.reach(item.def_id, item_level).generics().predicates();
                     for field in struct_def.fields() {
-                        let field_level = self.get(self.tcx.hir().local_def_id(field.hir_id));
+                        let def_id = self.tcx.hir().local_def_id(field.hir_id);
+                        let field_level = self.get(def_id);
                         if field_level.is_some() {
-                            self.reach(field.hir_id, field_level).ty();
+                            self.reach(def_id, field_level).ty();
                         }
                     }
                 }
@@ -879,47 +946,6 @@ fn visit_mod(&mut self, m: &'tcx hir::Mod<'tcx>, _sp: Span, id: hir::HirId) {
 
         intravisit::walk_mod(self, m, id);
     }
-
-    fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef<'tcx>) {
-        // Non-opaque macros cannot make other items more accessible than they already are.
-        let attrs = self.tcx.hir().attrs(md.hir_id());
-        if attr::find_transparency(&self.tcx.sess, &attrs, md.ast.macro_rules).0
-            != Transparency::Opaque
-        {
-            // `#[macro_export]`-ed `macro_rules!` are `Public` since they
-            // ignore their containing path to always appear at the crate root.
-            if md.ast.macro_rules {
-                self.update(md.def_id, Some(AccessLevel::Public));
-            }
-            return;
-        }
-
-        let macro_module_def_id =
-            ty::DefIdTree::parent(self.tcx, md.def_id.to_def_id()).unwrap().expect_local();
-        if self.tcx.hir().opt_def_kind(macro_module_def_id) != Some(DefKind::Mod) {
-            // The macro's parent doesn't correspond to a `mod`, return early (#63164, #65252).
-            return;
-        }
-
-        let level = if md.vis.node.is_pub() { self.get(macro_module_def_id) } else { None };
-        let new_level = self.update(md.def_id, level);
-        if new_level.is_none() {
-            return;
-        }
-
-        // 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;
-        loop {
-            let changed_reachability =
-                self.update_macro_reachable(module_def_id, macro_module_def_id);
-            if changed_reachability || module_def_id == CRATE_DEF_ID {
-                break;
-            }
-            module_def_id =
-                ty::DefIdTree::parent(self.tcx, module_def_id.to_def_id()).unwrap().expect_local();
-        }
-    }
 }
 
 impl ReachEverythingInTheInterfaceVisitor<'_, 'tcx> {
@@ -992,7 +1018,7 @@ fn visit_def_id(
 struct NamePrivacyVisitor<'tcx> {
     tcx: TyCtxt<'tcx>,
     maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>,
-    current_item: Option<hir::HirId>,
+    current_item: LocalDefId,
 }
 
 impl<'tcx> NamePrivacyVisitor<'tcx> {
@@ -1014,11 +1040,15 @@ fn check_field(
         field: &'tcx ty::FieldDef,
         in_update_syntax: bool,
     ) {
+        if def.is_enum() {
+            return;
+        }
+
         // definition of the field
         let ident = Ident::new(kw::Empty, use_ctxt);
-        let current_hir = self.current_item.unwrap();
-        let def_id = self.tcx.adjust_ident_and_get_scope(ident, def.did, current_hir).1;
-        if !def.is_enum() && !field.vis.is_accessible_from(def_id, self.tcx) {
+        let hir_id = self.tcx.hir().local_def_id_to_hir_id(self.current_item);
+        let def_id = self.tcx.adjust_ident_and_get_scope(ident, def.did, hir_id).1;
+        if !field.vis.is_accessible_from(def_id, self.tcx) {
             let label = if in_update_syntax {
                 format!("field `{}` is private", field.ident)
             } else {
@@ -1063,17 +1093,17 @@ fn visit_nested_body(&mut self, body: hir::BodyId) {
     }
 
     fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
-        let orig_current_item = self.current_item.replace(item.hir_id());
+        let orig_current_item = mem::replace(&mut self.current_item, item.def_id);
         intravisit::walk_item(self, item);
         self.current_item = orig_current_item;
     }
 
     fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
-        if let hir::ExprKind::Struct(ref qpath, fields, ref base) = expr.kind {
+        if let hir::ExprKind::Struct(qpath, fields, ref base) = expr.kind {
             let res = self.typeck_results().qpath_res(qpath, expr.hir_id);
             let adt = self.typeck_results().expr_ty(expr).ty_adt_def().unwrap();
             let variant = adt.variant_of_res(res);
-            if let Some(ref base) = *base {
+            if let Some(base) = *base {
                 // If the expression uses FRU we need to make sure all the unmentioned fields
                 // are checked for privacy (RFC 736). Rather than computing the set of
                 // unmentioned fields, just check them all.
@@ -1282,7 +1312,7 @@ fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
             return;
         }
         match expr.kind {
-            hir::ExprKind::Assign(_, ref rhs, _) | hir::ExprKind::Match(ref rhs, ..) => {
+            hir::ExprKind::Assign(_, rhs, _) | hir::ExprKind::Match(rhs, ..) => {
                 // Do not report duplicate errors for `x = y` and `match x { ... }`.
                 if self.check_expr_pat_type(rhs.hir_id, rhs.span) {
                     return;
@@ -1367,7 +1397,7 @@ fn visit_pat(&mut self, pattern: &'tcx hir::Pat<'tcx>) {
     }
 
     fn visit_local(&mut self, local: &'tcx hir::Local<'tcx>) {
-        if let Some(ref init) = local.init {
+        if let Some(init) = local.init {
             if self.check_expr_pat_type(init.hir_id, init.span) {
                 // Do not report duplicate errors for `let x = y`.
                 return;
@@ -1444,7 +1474,7 @@ fn path_is_private_type(&self, path: &hir::Path<'_>) -> bool {
             // .. and it corresponds to a private type in the AST (this returns
             // `None` for type parameters).
             match self.tcx.hir().find(self.tcx.hir().local_def_id_to_hir_id(did)) {
-                Some(Node::Item(ref item)) => !item.vis.node.is_pub(),
+                Some(Node::Item(item)) => !item.vis.node.is_pub(),
                 Some(_) | None => false,
             }
         } else {
@@ -1460,7 +1490,7 @@ fn trait_is_public(&self, trait_id: LocalDefId) -> bool {
 
     fn check_generic_bound(&mut self, bound: &hir::GenericBound<'_>) {
         if let hir::GenericBound::Trait(ref trait_ref, _) = *bound {
-            if self.path_is_private_type(&trait_ref.trait_ref.path) {
+            if self.path_is_private_type(trait_ref.trait_ref.path) {
                 self.old_error_set.insert(trait_ref.trait_ref.hir_ref_id);
             }
         }
@@ -1487,7 +1517,7 @@ fn visit_generic_arg(&mut self, generic_arg: &'v hir::GenericArg<'v>) {
     }
 
     fn visit_ty(&mut self, ty: &hir::Ty<'_>) {
-        if let hir::TyKind::Path(hir::QPath::Resolved(_, ref path)) = ty.kind {
+        if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind {
             if self.inner.path_is_private_type(path) {
                 self.contains_private = true;
                 // Found what we're looking for, so let's stop working.
@@ -1526,7 +1556,7 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             // namespace (the contents have their own privacies).
             hir::ItemKind::ForeignMod { .. } => {}
 
-            hir::ItemKind::Trait(.., ref bounds, _) => {
+            hir::ItemKind::Trait(.., bounds, _) => {
                 if !self.trait_is_public(item.def_id) {
                     return;
                 }
@@ -1556,7 +1586,7 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
                         at_outer_type: true,
                         outer_type_is_public_path: false,
                     };
-                    visitor.visit_ty(&impl_.self_ty);
+                    visitor.visit_ty(impl_.self_ty);
                     self_contains_private = visitor.contains_private;
                     self_is_public_path = visitor.outer_type_is_public_path;
                 }
@@ -1634,12 +1664,12 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
                             //
                             // Those in 2. are warned via walk_generics and this
                             // call here.
-                            intravisit::walk_path(self, &tr.path);
+                            intravisit::walk_path(self, tr.path);
 
                             // Those in 3. are warned with this call.
                             for impl_item_ref in impl_.items {
                                 let impl_item = self.tcx.hir().impl_item(impl_item_ref.id);
-                                if let hir::ImplItemKind::TyAlias(ref ty) = impl_item.kind {
+                                if let hir::ImplItemKind::TyAlias(ty) = impl_item.kind {
                                     self.visit_ty(ty);
                                 }
                             }
@@ -1650,7 +1680,10 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'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 => {
@@ -1706,7 +1739,7 @@ fn visit_generics(&mut self, generics: &'tcx hir::Generics<'tcx>) {
                 }
                 hir::WherePredicate::RegionPredicate(_) => {}
                 hir::WherePredicate::EqPredicate(eq_pred) => {
-                    self.visit_ty(&eq_pred.rhs_ty);
+                    self.visit_ty(eq_pred.rhs_ty);
                 }
             }
         }
@@ -1719,7 +1752,7 @@ fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem<'tcx>) {
     }
 
     fn visit_ty(&mut self, t: &'tcx hir::Ty<'tcx>) {
-        if let hir::TyKind::Path(hir::QPath::Resolved(_, ref path)) = t.kind {
+        if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = t.kind {
             if self.path_is_private_type(path) {
                 self.old_error_set.insert(t.hir_id);
             }
@@ -1763,9 +1796,7 @@ fn visit_expr(&mut self, _: &'tcx hir::Expr<'tcx>) {}
 
 struct SearchInterfaceForPrivateItemsVisitor<'tcx> {
     tcx: TyCtxt<'tcx>,
-    item_id: hir::HirId,
-    item_def_id: DefId,
-    span: Span,
+    item_def_id: LocalDefId,
     /// The visitor checks that each component type is at least this visible.
     required_visibility: ty::Visibility,
     has_pub_restricted: bool,
@@ -1783,7 +1814,7 @@ fn generics(&mut self) -> &mut Self {
                         self.visit(self.tcx.type_of(param.def_id));
                     }
                 }
-                // FIXME(const_evaluatable_checked): May want to look inside const here
+                // FIXME(generic_const_exprs): May want to look inside const here
                 GenericParamDefKind::Const { .. } => {
                     self.visit(self.tcx.type_of(param.def_id));
                 }
@@ -1820,8 +1851,8 @@ fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display)
         if self.leaks_private_dep(def_id) {
             self.tcx.struct_span_lint_hir(
                 lint::builtin::EXPORTED_PRIVATE_DEPENDENCIES,
-                self.item_id,
-                self.span,
+                self.tcx.hir().local_def_id_to_hir_id(self.item_def_id),
+                self.tcx.def_span(self.item_def_id.to_def_id()),
                 |lint| {
                     lint.build(&format!(
                         "{} `{}` from private dependency '{}' in public \
@@ -1856,15 +1887,16 @@ fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display)
                 }
             };
             let make_msg = || format!("{} {} `{}` in public interface", vis_descr, kind, descr);
+            let span = self.tcx.def_span(self.item_def_id.to_def_id());
             if self.has_pub_restricted || self.has_old_errors || self.in_assoc_ty {
                 let mut err = if kind == "trait" {
-                    struct_span_err!(self.tcx.sess, self.span, E0445, "{}", make_msg())
+                    struct_span_err!(self.tcx.sess, span, E0445, "{}", make_msg())
                 } else {
-                    struct_span_err!(self.tcx.sess, self.span, E0446, "{}", make_msg())
+                    struct_span_err!(self.tcx.sess, span, E0446, "{}", make_msg())
                 };
                 let vis_span =
                     self.tcx.sess.source_map().guess_head_span(self.tcx.def_span(def_id));
-                err.span_label(self.span, format!("can't leak {} {}", vis_descr, kind));
+                err.span_label(span, format!("can't leak {} {}", vis_descr, kind));
                 err.span_label(vis_span, format!("`{}` declared as {}", descr, vis_descr));
                 err.emit();
             } else {
@@ -1872,7 +1904,7 @@ fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display)
                 self.tcx.struct_span_lint_hir(
                     lint::builtin::PRIVATE_IN_PUBLIC,
                     hir_id,
-                    self.span,
+                    span,
                     |lint| lint.build(&format!("{} (error {})", make_msg(), err_code)).emit(),
                 );
             }
@@ -1915,35 +1947,33 @@ fn visit_def_id(
 struct PrivateItemsInPublicInterfacesVisitor<'tcx> {
     tcx: TyCtxt<'tcx>,
     has_pub_restricted: bool,
-    old_error_set_ancestry: HirIdSet,
+    old_error_set_ancestry: LocalDefIdSet,
 }
 
 impl<'tcx> PrivateItemsInPublicInterfacesVisitor<'tcx> {
     fn check(
         &self,
-        item_id: hir::HirId,
+        def_id: LocalDefId,
         required_visibility: ty::Visibility,
     ) -> SearchInterfaceForPrivateItemsVisitor<'tcx> {
         SearchInterfaceForPrivateItemsVisitor {
             tcx: self.tcx,
-            item_id,
-            item_def_id: self.tcx.hir().local_def_id(item_id).to_def_id(),
-            span: self.tcx.hir().span(item_id),
+            item_def_id: def_id,
             required_visibility,
             has_pub_restricted: self.has_pub_restricted,
-            has_old_errors: self.old_error_set_ancestry.contains(&item_id),
+            has_old_errors: self.old_error_set_ancestry.contains(&def_id),
             in_assoc_ty: false,
         }
     }
 
     fn check_assoc_item(
         &self,
-        hir_id: hir::HirId,
+        def_id: LocalDefId,
         assoc_item_kind: AssocItemKind,
         defaultness: hir::Defaultness,
         vis: ty::Visibility,
     ) {
-        let mut check = self.check(hir_id, vis);
+        let mut check = self.check(def_id, vis);
 
         let (check_ty, is_assoc_ty) = match assoc_item_kind {
             AssocItemKind::Const | AssocItemKind::Fn { .. } => (true, false),
@@ -1976,44 +2006,44 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             // Checked in resolve.
             hir::ItemKind::Use(..) => {}
             // No subitems.
-            hir::ItemKind::GlobalAsm(..) => {}
+            hir::ItemKind::Macro(..) | hir::ItemKind::GlobalAsm(..) => {}
             // Subitems of these items have inherited publicity.
             hir::ItemKind::Const(..)
             | hir::ItemKind::Static(..)
             | hir::ItemKind::Fn(..)
             | hir::ItemKind::TyAlias(..) => {
-                self.check(item.hir_id(), item_visibility).generics().predicates().ty();
+                self.check(item.def_id, item_visibility).generics().predicates().ty();
             }
             hir::ItemKind::OpaqueTy(..) => {
                 // `ty()` for opaque types is the underlying type,
                 // it's not a part of interface, so we skip it.
-                self.check(item.hir_id(), item_visibility).generics().bounds();
+                self.check(item.def_id, item_visibility).generics().bounds();
             }
             hir::ItemKind::Trait(.., trait_item_refs) => {
-                self.check(item.hir_id(), item_visibility).generics().predicates();
+                self.check(item.def_id, item_visibility).generics().predicates();
 
                 for trait_item_ref in trait_item_refs {
                     self.check_assoc_item(
-                        trait_item_ref.id.hir_id(),
+                        trait_item_ref.id.def_id,
                         trait_item_ref.kind,
                         trait_item_ref.defaultness,
                         item_visibility,
                     );
 
                     if let AssocItemKind::Type = trait_item_ref.kind {
-                        self.check(trait_item_ref.id.hir_id(), item_visibility).bounds();
+                        self.check(trait_item_ref.id.def_id, item_visibility).bounds();
                     }
                 }
             }
             hir::ItemKind::TraitAlias(..) => {
-                self.check(item.hir_id(), item_visibility).generics().predicates();
+                self.check(item.def_id, item_visibility).generics().predicates();
             }
             hir::ItemKind::Enum(ref def, _) => {
-                self.check(item.hir_id(), item_visibility).generics().predicates();
+                self.check(item.def_id, item_visibility).generics().predicates();
 
                 for variant in def.variants {
                     for field in variant.data.fields() {
-                        self.check(field.hir_id, item_visibility).ty();
+                        self.check(self.tcx.hir().local_def_id(field.hir_id), item_visibility).ty();
                     }
                 }
             }
@@ -2021,16 +2051,17 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             hir::ItemKind::ForeignMod { items, .. } => {
                 for foreign_item in items {
                     let vis = tcx.visibility(foreign_item.id.def_id);
-                    self.check(foreign_item.id.hir_id(), vis).generics().predicates().ty();
+                    self.check(foreign_item.id.def_id, vis).generics().predicates().ty();
                 }
             }
             // Subitems of structs and unions have their own publicity.
             hir::ItemKind::Struct(ref struct_def, _) | hir::ItemKind::Union(ref struct_def, _) => {
-                self.check(item.hir_id(), item_visibility).generics().predicates();
+                self.check(item.def_id, item_visibility).generics().predicates();
 
                 for field in struct_def.fields() {
-                    let field_visibility = tcx.visibility(tcx.hir().local_def_id(field.hir_id));
-                    self.check(field.hir_id, min(item_visibility, field_visibility, tcx)).ty();
+                    let def_id = tcx.hir().local_def_id(field.hir_id);
+                    let field_visibility = tcx.visibility(def_id);
+                    self.check(def_id, min(item_visibility, field_visibility, tcx)).ty();
                 }
             }
             // An inherent impl is public when its type is public
@@ -2038,8 +2069,8 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
             // A trait impl is public when both its type and its trait are public
             // Subitems of trait impls have inherited publicity.
             hir::ItemKind::Impl(ref impl_) => {
-                let impl_vis = ty::Visibility::of_impl(item.hir_id(), tcx, &Default::default());
-                self.check(item.hir_id(), impl_vis).generics().predicates();
+                let impl_vis = ty::Visibility::of_impl(item.def_id, tcx, &Default::default());
+                self.check(item.def_id, impl_vis).generics().predicates();
                 for impl_item_ref in impl_.items {
                     let impl_item_vis = if impl_.of_trait.is_none() {
                         min(tcx.visibility(impl_item_ref.id.def_id), impl_vis, tcx)
@@ -2047,7 +2078,7 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
                         impl_vis
                     };
                     self.check_assoc_item(
-                        impl_item_ref.id.hir_id(),
+                        impl_item_ref.id.def_id,
                         impl_item_ref.kind,
                         impl_item_ref.defaultness,
                         impl_item_vis,
@@ -2119,7 +2150,8 @@ fn visibility(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Visibility {
 
 fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
     // Check privacy of names not checked in previous compilation stages.
-    let mut visitor = NamePrivacyVisitor { tcx, maybe_typeck_results: None, current_item: None };
+    let mut visitor =
+        NamePrivacyVisitor { tcx, maybe_typeck_results: None, current_item: module_def_id };
     let (module, span, hir_id) = tcx.hir().get_module(module_def_id);
 
     intravisit::walk_mod(&mut visitor, module, hir_id);
@@ -2142,7 +2174,7 @@ fn privacy_access_levels(tcx: TyCtxt<'_>, (): ()) -> &AccessLevels {
         changed: false,
     };
     loop {
-        intravisit::walk_crate(&mut visitor, tcx.hir().krate());
+        tcx.hir().walk_toplevel_module(&mut visitor);
         if visitor.changed {
             visitor.changed = false;
         } else {
@@ -2157,19 +2189,17 @@ fn privacy_access_levels(tcx: TyCtxt<'_>, (): ()) -> &AccessLevels {
 fn check_private_in_public(tcx: TyCtxt<'_>, (): ()) {
     let access_levels = tcx.privacy_access_levels(());
 
-    let krate = tcx.hir().krate();
-
     let mut visitor = ObsoleteVisiblePrivateTypesVisitor {
         tcx,
-        access_levels: &access_levels,
+        access_levels,
         in_variant: false,
         old_error_set: Default::default(),
     };
-    intravisit::walk_crate(&mut visitor, krate);
+    tcx.hir().walk_toplevel_module(&mut visitor);
 
     let has_pub_restricted = {
         let mut pub_restricted_visitor = PubRestrictedVisitor { tcx, has_pub_restricted: false };
-        intravisit::walk_crate(&mut pub_restricted_visitor, krate);
+        tcx.hir().walk_toplevel_module(&mut pub_restricted_visitor);
         pub_restricted_visitor.has_pub_restricted
     };
 
@@ -2188,7 +2218,15 @@ fn check_private_in_public(tcx: TyCtxt<'_>, (): ()) {
     }
 
     // Check for private types and traits in public interfaces.
-    let mut visitor =
-        PrivateItemsInPublicInterfacesVisitor { tcx, has_pub_restricted, old_error_set_ancestry };
-    krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor));
+    let mut visitor = PrivateItemsInPublicInterfacesVisitor {
+        tcx,
+        has_pub_restricted,
+        // Only definition IDs are ever searched in `old_error_set_ancestry`,
+        // so we can filter away all non-definition IDs at this point.
+        old_error_set_ancestry: old_error_set_ancestry
+            .into_iter()
+            .filter_map(|hir_id| tcx.hir().opt_local_def_id(hir_id))
+            .collect(),
+    };
+    tcx.hir().visit_all_item_likes(&mut DeepVisitor::new(&mut visitor));
 }