Drop vis in Item.

Author: cjgillot

compiler/rustc_ast_lowering/src/item.rs

@@ -14,7 +14,7 @@ use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID};
 use rustc_index::vec::{Idx, IndexVec};
 use rustc_session::utils::NtToTokenstream;
 use rustc_session::Session;
-use rustc_span::source_map::{respan, DesugaringKind};
+use rustc_span::source_map::DesugaringKind;
 use rustc_span::symbol::{kw, sym, Ident};
 use rustc_span::Span;
 use rustc_target::spec::abi;
@@ -230,15 +230,15 @@ impl<'hir> LoweringContext<'_, '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);
+        let vis_span = self.lower_span(i.vis.span);
         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);
+        let kind = self.lower_item_kind(i.span, i.id, hir_id, &mut ident, attrs, vis_span, &i.kind);
         let item = hir::Item {
             def_id: hir_id.expect_owner(),
             ident: self.lower_ident(ident),
             kind,
-            vis,
+            vis_span,
             span: self.lower_span(i.span),
         };
         self.arena.alloc(item)
@@ -251,7 +251,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         hir_id: hir::HirId,
         ident: &mut Ident,
         attrs: Option<&'hir [Attribute]>,
-        vis: &mut hir::Visibility<'hir>,
+        vis_span: Span,
         i: &ItemKind,
     ) -> hir::ItemKind<'hir> {
         match *i {
@@ -260,7 +260,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 // Start with an empty prefix.
                 let prefix = Path { segments: vec![], span: use_tree.span, tokens: None };
 
-                self.lower_use_tree(use_tree, &prefix, id, vis, ident, attrs)
+                self.lower_use_tree(use_tree, &prefix, id, vis_span, ident, attrs)
             }
             ItemKind::Static(ref t, m, ref e) => {
                 let (ty, body_id) = self.lower_const_item(t, span, e.as_deref());
@@ -527,12 +527,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
         tree: &UseTree,
         prefix: &Path,
         id: NodeId,
-        vis: &mut hir::Visibility<'hir>,
+        vis_span: Span,
         ident: &mut Ident,
         attrs: Option<&'hir [Attribute]>,
     ) -> hir::ItemKind<'hir> {
         debug!("lower_use_tree(tree={:?})", tree);
-        debug!("lower_use_tree: vis = {:?}", vis);
 
         let path = &tree.prefix;
         let segments = prefix.segments.iter().chain(path.segments.iter()).cloned().collect();
@@ -586,7 +585,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
                         let res = this.lower_res(res);
                         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::ItemLocalId::new(0), attrs);
                         }
@@ -595,7 +593,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                             def_id: new_id,
                             ident: this.lower_ident(ident),
                             kind,
-                            vis,
+                            vis_span,
                             span: this.lower_span(span),
                         };
                         hir::OwnerNode::Item(this.arena.alloc(item))
@@ -657,11 +655,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     // own its own names, we have to adjust the owner before
                     // lowering the rest of the import.
                     self.with_hir_id_owner(id, |this| {
-                        let mut vis = this.rebuild_vis(&vis);
                         let mut ident = *ident;
 
                         let kind =
-                            this.lower_use_tree(use_tree, &prefix, id, &mut vis, &mut ident, attrs);
+                            this.lower_use_tree(use_tree, &prefix, id, vis_span, &mut ident, attrs);
                         if let Some(attrs) = attrs {
                             this.attrs.insert(hir::ItemLocalId::new(0), attrs);
                         }
@@ -670,37 +667,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
                             def_id: new_hir_id,
                             ident: this.lower_ident(ident),
                             kind,
-                            vis,
+                            vis_span,
                             span: this.lower_span(use_tree.span),
                         };
                         hir::OwnerNode::Item(this.arena.alloc(item))
                     });
                 }
 
-                // Subtle and a bit hacky: we lower the privacy level
-                // of the list stem to "private" most of the time, but
-                // not for "restricted" paths. The key thing is that
-                // we don't want it to stay as `pub` (with no caveats)
-                // because that affects rustdoc and also the lints
-                // about `pub` items. But we can't *always* make it
-                // private -- particularly not for restricted paths --
-                // because it contains node-ids that would then be
-                // unused, failing the check that HirIds are "densely
-                // assigned".
-                match vis.node {
-                    hir::VisibilityKind::Public
-                    | hir::VisibilityKind::Crate(_)
-                    | hir::VisibilityKind::Inherited => {
-                        *vis = respan(
-                            self.lower_span(prefix.span.shrink_to_lo()),
-                            hir::VisibilityKind::Inherited,
-                        );
-                    }
-                    hir::VisibilityKind::Restricted { .. } => {
-                        // Do nothing here, as described in the comment on the match.
-                    }
-                }
-
                 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);
@@ -709,37 +682,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
         }
     }
 
-    /// Paths like the visibility path in `pub(super) use foo::{bar, baz}` are repeated
-    /// many times in the HIR tree; for each occurrence, we need to assign distinct
-    /// `NodeId`s. (See, e.g., #56128.)
-    fn rebuild_use_path(&mut self, path: &hir::Path<'hir>) -> &'hir hir::Path<'hir> {
-        debug!("rebuild_use_path(path = {:?})", path);
-        let segments =
-            self.arena.alloc_from_iter(path.segments.iter().map(|seg| hir::PathSegment {
-                ident: seg.ident,
-                hir_id: seg.hir_id.map(|_| self.next_id()),
-                res: seg.res,
-                args: None,
-                infer_args: seg.infer_args,
-            }));
-        self.arena.alloc(hir::Path { span: path.span, res: path.res, segments })
-    }
-
-    fn rebuild_vis(&mut self, vis: &hir::Visibility<'hir>) -> hir::Visibility<'hir> {
-        let vis_kind = match vis.node {
-            hir::VisibilityKind::Public => hir::VisibilityKind::Public,
-            hir::VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
-            hir::VisibilityKind::Inherited => hir::VisibilityKind::Inherited,
-            hir::VisibilityKind::Restricted { ref path, hir_id: _ } => {
-                hir::VisibilityKind::Restricted {
-                    path: self.rebuild_use_path(path),
-                    hir_id: self.next_id(),
-                }
-            }
-        };
-        respan(self.lower_span(vis.span), vis_kind)
-    }
-
     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();
@@ -1044,28 +986,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
         }
     }
 
-    /// If an `explicit_owner` is given, this method allocates the `HirId` in
-    /// the address space of that item instead of the item currently being
-    /// 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) -> 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 = self.lower_node_id(id);
-                hir::VisibilityKind::Restricted {
-                    path: self.lower_path(id, path, ParamMode::Explicit),
-                    hir_id: lowered_id,
-                }
-            }
-            VisibilityKind::Inherited => hir::VisibilityKind::Inherited,
-        };
-        respan(self.lower_span(v.span), node)
-    }
-
     fn lower_defaultness(
         &self,
         d: Defaultness,

compiler/rustc_ast_lowering/src/lib.rs

@@ -61,7 +61,7 @@ use rustc_session::parse::feature_err;
 use rustc_session::utils::{FlattenNonterminals, NtToTokenstream};
 use rustc_session::Session;
 use rustc_span::hygiene::{ExpnId, MacroKind};
-use rustc_span::source_map::{respan, DesugaringKind};
+use rustc_span::source_map::DesugaringKind;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{Span, DUMMY_SP};
 
@@ -1530,7 +1530,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             def_id: opaque_ty_id,
             ident: Ident::empty(),
             kind: opaque_ty_item_kind,
-            vis: respan(self.lower_span(span.shrink_to_lo()), hir::VisibilityKind::Inherited),
+            vis_span: self.lower_span(span.shrink_to_lo()),
             span: self.lower_span(opaque_ty_span),
         };
         hir::OwnerNode::Item(self.arena.alloc(opaque_ty_item))

compiler/rustc_hir/src/hir.rs

@@ -4,8 +4,8 @@ crate use crate::hir_id::{HirId, ItemLocalId};
 use crate::intravisit::FnKind;
 use crate::LangItem;
 
+use rustc_ast as ast;
 use rustc_ast::util::parser::ExprPrecedence;
-use rustc_ast::{self as ast, CrateSugar};
 use rustc_ast::{Attribute, FloatTy, IntTy, Label, LitKind, TraitObjectSyntax, UintTy};
 pub use rustc_ast::{BorrowKind, ImplPolarity, IsAuto};
 pub use rustc_ast::{CaptureBy, Movability, Mutability};
@@ -2645,29 +2645,6 @@ pub struct PolyTraitRef<'hir> {
     pub span: Span,
 }
 
-pub type Visibility<'hir> = Spanned<VisibilityKind<'hir>>;
-
-#[derive(Copy, Clone, Debug, HashStable_Generic)]
-pub enum VisibilityKind<'hir> {
-    Public,
-    Crate(CrateSugar),
-    Restricted { path: &'hir Path<'hir>, hir_id: HirId },
-    Inherited,
-}
-
-impl VisibilityKind<'_> {
-    pub fn is_pub(&self) -> bool {
-        matches!(*self, VisibilityKind::Public)
-    }
-
-    pub fn is_pub_restricted(&self) -> bool {
-        match *self {
-            VisibilityKind::Public | VisibilityKind::Inherited => false,
-            VisibilityKind::Crate(..) | VisibilityKind::Restricted { .. } => true,
-        }
-    }
-}
-
 #[derive(Debug, HashStable_Generic)]
 pub struct FieldDef<'hir> {
     pub span: Span,
@@ -2744,8 +2721,8 @@ pub struct Item<'hir> {
     pub ident: Ident,
     pub def_id: LocalDefId,
     pub kind: ItemKind<'hir>,
-    pub vis: Visibility<'hir>,
     pub span: Span,
+    pub vis_span: Span,
 }
 
 impl Item<'_> {
@@ -3348,7 +3325,7 @@ mod size_asserts {
     rustc_data_structures::static_assert_size!(super::QPath<'static>, 24);
     rustc_data_structures::static_assert_size!(super::Ty<'static>, 72);
 
-    rustc_data_structures::static_assert_size!(super::Item<'static>, 184);
+    rustc_data_structures::static_assert_size!(super::Item<'static>, 160);
     rustc_data_structures::static_assert_size!(super::TraitItem<'static>, 128);
     rustc_data_structures::static_assert_size!(super::ImplItem<'static>, 120);
     rustc_data_structures::static_assert_size!(super::ForeignItem<'static>, 112);

compiler/rustc_lint/src/builtin.rs

@@ -36,8 +36,7 @@ use rustc_feature::{deprecated_attributes, AttributeGate, BuiltinAttribute, Gate
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID};
-use rustc_hir::{ForeignItemKind, GenericParamKind, PatKind};
-use rustc_hir::{HirId, Node};
+use rustc_hir::{ForeignItemKind, GenericParamKind, HirId, PatKind};
 use rustc_index::vec::Idx;
 use rustc_middle::lint::LintDiagnosticBuilder;
 use rustc_middle::ty::layout::{LayoutError, LayoutOf};
@@ -600,7 +599,11 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
         match it.kind {
             hir::ItemKind::Trait(.., trait_item_refs) => {
                 // Issue #11592: traits are always considered exported, even when private.
-                if let hir::VisibilityKind::Inherited = it.vis.node {
+                if cx.tcx.visibility(it.def_id)
+                    == ty::Visibility::Restricted(
+                        cx.tcx.parent_module_from_def_id(it.def_id).to_def_id(),
+                    )
+                {
                     self.private_traits.insert(it.hir_id());
                     for trait_item_ref in trait_item_refs {
                         self.private_traits.insert(trait_item_ref.id.hir_id());
@@ -613,15 +616,17 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
                 // reported for missing docs.
                 let real_trait = trait_ref.path.res.def_id();
                 let Some(def_id) = real_trait.as_local() else { return };
-                let Some(Node::Item(item)) = cx.tcx.hir().find_by_def_id(def_id) else { return };
-                if let hir::VisibilityKind::Inherited = item.vis.node {
+                if cx.tcx.visibility(def_id)
+                    == ty::Visibility::Restricted(
+                        cx.tcx.parent_module_from_def_id(it.def_id).to_def_id(),
+                    )
+                {
                     for impl_item_ref in items {
                         self.private_traits.insert(impl_item_ref.id.hir_id());
                     }
                 }
                 return;
             }
-
             hir::ItemKind::TyAlias(..)
             | hir::ItemKind::Fn(..)
             | hir::ItemKind::Macro(..)
@@ -1420,7 +1425,7 @@ impl UnreachablePub {
 impl<'tcx> LateLintPass<'tcx> for UnreachablePub {
     fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
         if cx.tcx.visibility(item.def_id).is_public() {
-            self.perform_lint(cx, "item", item.def_id, item.vis.span, true);
+            self.perform_lint(cx, "item", item.def_id, item.vis_span, true);
         }
     }
 

compiler/rustc_middle/src/ty/mod.rs

@@ -36,7 +36,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::tagged_ptr::CopyTaggedPtr;
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
-use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LocalDefIdMap, CRATE_DEF_ID};
+use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LocalDefIdMap};
 use rustc_hir::Node;
 use rustc_macros::HashStable;
 use rustc_query_system::ich::StableHashingContext;
@@ -317,22 +317,6 @@ impl<'tcx> DefIdTree for TyCtxt<'tcx> {
 }
 
 impl Visibility {
-    pub fn from_hir(visibility: &hir::Visibility<'_>, id: hir::HirId, tcx: TyCtxt<'_>) -> Self {
-        match visibility.node {
-            hir::VisibilityKind::Public => Visibility::Public,
-            hir::VisibilityKind::Crate(_) => Visibility::Restricted(CRATE_DEF_ID.to_def_id()),
-            hir::VisibilityKind::Restricted { ref path, .. } => match path.res {
-                // If there is no resolution, `resolve` will have already reported an error, so
-                // assume that the visibility is public to avoid reporting more privacy errors.
-                Res::Err => Visibility::Public,
-                def => Visibility::Restricted(def.def_id()),
-            },
-            hir::VisibilityKind::Inherited => {
-                Visibility::Restricted(tcx.parent_module(id).to_def_id())
-            }
-        }
-    }
-
     /// Returns `true` if an item with this visibility is accessible from the given block.
     pub fn is_accessible_from<T: DefIdTree>(self, module: DefId, tree: T) -> bool {
         let restriction = match self {

compiler/rustc_passes/src/dead.rs

@@ -291,7 +291,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
         self.pub_visibility = false;
         match node {
             Node::Item(item) => {
-                self.pub_visibility = item.vis.node.is_pub();
+                self.pub_visibility = self.tcx.visibility(item.def_id).is_public();
 
                 match item.kind {
                     hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {

compiler/rustc_privacy/src/lib.rs

@@ -1341,7 +1341,7 @@ impl<'a, 'tcx> ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
             // .. 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(item)) => !item.vis.node.is_pub(),
+                Some(Node::Item(_)) => !self.tcx.visibility(did).is_public(),
                 Some(_) | None => false,
             }
         } else {
@@ -1975,19 +1975,16 @@ fn visibility(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Visibility {
             match tcx.hir().get(hir_id) {
                 // Unique types created for closures participate in type privacy checking.
                 // They have visibilities inherited from the module they are defined in.
-                Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(..), .. }) => {
-                    ty::Visibility::Restricted(tcx.parent_module(hir_id).to_def_id())
-                }
-                // - AST lowering may clone `use` items and the clones don't
+                Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(..), .. })
+                // - AST lowering creates dummy `use` items which don't
                 //   get their entries in the resolver's visibility table.
                 // - AST lowering also creates opaque type items with inherited visibilities.
                 //   Visibility on them should have no effect, but to avoid the visibility
                 //   query failing on some items, we provide it for opaque types as well.
-                Node::Item(hir::Item {
-                    vis,
+                | Node::Item(hir::Item {
                     kind: hir::ItemKind::Use(..) | hir::ItemKind::OpaqueTy(..),
                     ..
-                }) => ty::Visibility::from_hir(vis, hir_id, tcx),
+                }) => ty::Visibility::Restricted(tcx.parent_module(hir_id).to_def_id()),
                 // Visibilities of trait impl items are inherited from their traits
                 // and are not filled in resolve.
                 Node::ImplItem(impl_item) => {