Skip to content

Remove visibility information from HIR #93970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 24, 2022
Merged

Remove visibility information from HIR #93970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
10d10ef
Stop visiting visibility.
cjgillot Feb 13, 2022
4e8046f
Stop pretty-printing HIR visibility.
cjgillot Feb 13, 2022
a62680d
Drop vis in FieldDef.
cjgillot Feb 13, 2022
2827007
Drop vis from ForeignItem.
cjgillot Feb 13, 2022
a6e3124
Drop vis in ImplItem.
cjgillot Feb 13, 2022
4bbe078
Drop vis in Item.
cjgillot Feb 13, 2022
daccecc
Bless incremental tests.
cjgillot Feb 13, 2022
423a712
Fix lints.
cjgillot Feb 13, 2022
88de3e5
Make clippy inspector more precise.
cjgillot Feb 14, 2022
9861bc8
Compute has_pub_restricted in the resolver.
cjgillot Apr 18, 2022
c3276c6
Hash visibilities in crate_hash.
cjgillot Feb 18, 2022
0a6e135
Bless pretty-print output.
cjgillot Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions compiler/rustc_ast_lowering/src/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
106 changes: 13 additions & 93 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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());
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand All @@ -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))
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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))
Expand Down
47 changes: 11 additions & 36 deletions compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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<'_> {
Expand Down Expand Up @@ -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>,
}
Expand Down Expand Up @@ -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<'_> {
Expand Down Expand Up @@ -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<'_> {
Expand Down Expand Up @@ -3210,7 +3187,6 @@ pub enum Node<'hir> {

Lifetime(&'hir Lifetime),
GenericParam(&'hir GenericParam<'hir>),
Visibility(&'hir Visibility<'hir>),

Crate(&'hir Mod<'hir>),

Expand Down Expand Up @@ -3253,7 +3229,6 @@ impl<'hir> Node<'hir> {
| Node::Binding(..)
| Node::Arm(..)
| Node::Local(..)
| Node::Visibility(..)
| Node::Crate(..)
| Node::Ty(..)
| Node::TraitRef(..)
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}
Loading