Do not store visibility in *ItemRef.

Author: cjgillot

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(

compiler/rustc_ast_lowering/src/item.rs

@@ -188,7 +188,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
     pub fn lower_item(&mut self, i: &Item) -> 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);
@@ -493,7 +493,7 @@ 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 {
@@ -510,7 +510,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     });
                 }
 
-                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 => {
@@ -610,7 +610,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)
             }
         }
@@ -679,17 +679,16 @@ 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),
         }
     }
 
-    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) },
             ident: self.lower_ident(i.ident),
             span: self.lower_span(i.span),
-            vis: self.lower_visibility(&i.vis, Some(i.id)),
         }
     }
 
@@ -757,7 +756,7 @@ 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,
         }
     }
@@ -899,22 +898,21 @@ impl<'hir> LoweringContext<'_, 'hir> {
             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),
         }
     }
 
-    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) },
             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,
                 }
             }

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,

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>,

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)]

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);
 }
 

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,

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);
     }

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

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 => {