Auto merge of #93970 - cjgillot:novis, r=petrochenkov

Remove visibility information from HIR

The resolver exports all the necessary visibility information through the `tcx.visibility` query.
This PR stops having a dedicated visibility field in HIR, in order to use this query.
We keep a `vis_span` field for diagnostic purposes.

Author: bors

compiler/rustc_ast_lowering/src/index.rs

@@ -290,18 +290,6 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         self.insert(lifetime.span, lifetime.hir_id, Node::Lifetime(lifetime));
     }
 
-    fn visit_vis(&mut self, visibility: &'hir Visibility<'hir>) {
-        match visibility.node {
-            VisibilityKind::Public | VisibilityKind::Crate(_) | VisibilityKind::Inherited => {}
-            VisibilityKind::Restricted { hir_id, .. } => {
-                self.insert(visibility.span, hir_id, Node::Visibility(visibility));
-                self.with_parent(hir_id, |this| {
-                    intravisit::walk_vis(this, visibility);
-                });
-            }
-        }
-    }
-
     fn visit_variant(&mut self, v: &'hir Variant<'hir>, g: &'hir Generics<'hir>, item_id: HirId) {
         self.insert(v.span, v.id, Node::Variant(v));
         self.with_parent(v.id, |this| {

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();
@@ -773,7 +715,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 ForeignItemKind::TyAlias(..) => hir::ForeignItemKind::Type,
                 ForeignItemKind::MacCall(_) => panic!("macro shouldn't exist here"),
             },
-            vis: self.lower_visibility(&i.vis),
+            vis_span: self.lower_span(i.vis.span),
             span: self.lower_span(i.span),
         };
         self.arena.alloc(item)
@@ -851,7 +793,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),
+            vis_span: self.lower_span(f.vis.span),
             ty,
         }
     }
@@ -1016,8 +958,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
             def_id: hir_id.expect_owner(),
             ident: self.lower_ident(i.ident),
             generics,
-            vis: self.lower_visibility(&i.vis),
             kind,
+            vis_span: self.lower_span(i.vis.span),
             span: self.lower_span(i.span),
         };
         self.arena.alloc(item)
@@ -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};
@@ -2140,10 +2140,10 @@ impl ImplItemId {
 pub struct ImplItem<'hir> {
     pub ident: Ident,
     pub def_id: LocalDefId,
-    pub vis: Visibility<'hir>,
     pub generics: Generics<'hir>,
     pub kind: ImplItemKind<'hir>,
     pub span: Span,
+    pub vis_span: Span,
 }
 
 impl ImplItem<'_> {
@@ -2645,34 +2645,11 @@ 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,
+    pub vis_span: Span,
     pub ident: Ident,
-    pub vis: Visibility<'hir>,
     pub hir_id: HirId,
     pub ty: &'hir Ty<'hir>,
 }
@@ -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<'_> {
@@ -3002,7 +2979,7 @@ pub struct ForeignItem<'hir> {
     pub kind: ForeignItemKind<'hir>,
     pub def_id: LocalDefId,
     pub span: Span,
-    pub vis: Visibility<'hir>,
+    pub vis_span: Span,
 }
 
 impl ForeignItem<'_> {
@@ -3210,7 +3187,6 @@ pub enum Node<'hir> {
 
     Lifetime(&'hir Lifetime),
     GenericParam(&'hir GenericParam<'hir>),
-    Visibility(&'hir Visibility<'hir>),
 
     Crate(&'hir Mod<'hir>),
 
@@ -3253,7 +3229,6 @@ impl<'hir> Node<'hir> {
             | Node::Binding(..)
             | Node::Arm(..)
             | Node::Local(..)
-            | Node::Visibility(..)
             | Node::Crate(..)
             | Node::Ty(..)
             | Node::TraitRef(..)
@@ -3318,18 +3293,18 @@ impl<'hir> Node<'hir> {
         match self {
             Node::Item(i) => match i.kind {
                 ItemKind::Fn(ref sig, ref generics, _) => {
-                    Some(FnKind::ItemFn(i.ident, generics, sig.header, &i.vis))
+                    Some(FnKind::ItemFn(i.ident, generics, sig.header))
                 }
                 _ => None,
             },
             Node::TraitItem(ti) => match ti.kind {
                 TraitItemKind::Fn(ref sig, TraitFn::Provided(_)) => {
-                    Some(FnKind::Method(ti.ident, sig, None))
+                    Some(FnKind::Method(ti.ident, sig))
                 }
                 _ => None,
             },
             Node::ImplItem(ii) => match ii.kind {
-                ImplItemKind::Fn(ref sig, _) => Some(FnKind::Method(ii.ident, sig, Some(&ii.vis))),
+                ImplItemKind::Fn(ref sig, _) => Some(FnKind::Method(ii.ident, sig)),
                 _ => None,
             },
             Node::Expr(e) => match e.kind {
@@ -3350,8 +3325,8 @@ 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>, 144);
-    rustc_data_structures::static_assert_size!(super::ForeignItem<'static>, 136);
+    rustc_data_structures::static_assert_size!(super::ImplItem<'static>, 120);
+    rustc_data_structures::static_assert_size!(super::ForeignItem<'static>, 112);
 }