Skip to content

Commit

Permalink
Rollup merge of rust-lang#59096 - ljedrz:HirIdify_AccessLevel, r=Zoxc
Browse files Browse the repository at this point in the history 
middle: replace NodeId with HirId in AccessLevels

Pushing the limits of HirIdification (rust-lang#57578).

Replaces `NodeId` with `HirId` in `middle::privacy::AccessLevels`. Actually this time I was more successful and cracked it; I probably tried to HirIdify too much at once when I attempted it last time ^^.

r? @Zoxc
  • Loading branch information
@Centril
Centril authored Mar 16, 2019
2 parents cadb47c + 856b081 commit f0cdccd
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 52 deletions.
2 changes: 1 addition & 1 deletion src/librustc/middle/dead.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ fn create_and_seed_worklist<'a, 'tcx>(
) -> (Vec<hir::HirId>, FxHashMap<hir::HirId, hir::HirId>) {
let worklist = access_levels.map.iter().filter_map(|(&id, level)| {
if level >= &privacy::AccessLevel::Reachable {
Some(tcx.hir().node_to_hir_id(id))
Some(id)
} else {
None
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/privacy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
//! outside their scopes. This pass will also generate a set of exported items
//! which are available for use externally when compiled as a library.

use crate::hir::HirId;
use crate::util::nodemap::{DefIdSet, FxHashMap};

use std::hash::Hash;
use std::fmt;
use syntax::ast::NodeId;
use rustc_macros::HashStable;

// Accessibility levels, sorted in ascending order
Expand All @@ -27,7 +27,7 @@ pub enum AccessLevel {

// Accessibility levels for reachable HIR nodes
#[derive(Clone)]
pub struct AccessLevels<Id = NodeId> {
pub struct AccessLevels<Id = HirId> {
pub map: FxHashMap<Id, AccessLevel>
}

Expand Down
5 changes: 2 additions & 3 deletions src/librustc/middle/reachable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,7 @@ impl<'a, 'tcx: 'a> ItemLikeVisitor<'tcx> for CollectPrivateImplItemsVisitor<'a,

// We need only trait impls here, not inherent impls, and only non-exported ones
if let hir::ItemKind::Impl(.., Some(ref trait_ref), _, ref impl_item_refs) = item.node {
let node_id = self.tcx.hir().hir_to_node_id(item.hir_id);
if !self.access_levels.is_reachable(node_id) {
if !self.access_levels.is_reachable(item.hir_id) {
self.worklist.extend(impl_item_refs.iter().map(|ii_ref| ii_ref.id.hir_id));

let trait_def_id = match trait_ref.path.def {
Expand Down Expand Up @@ -415,7 +414,7 @@ fn reachable_set<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum) ->
// use the lang items, so we need to be sure to mark them as
// exported.
reachable_context.worklist.extend(
access_levels.map.iter().map(|(id, _)| tcx.hir().node_to_hir_id(*id)));
access_levels.map.iter().map(|(id, _)| *id));
for item in tcx.lang_items().items().iter() {
if let Some(did) = *item {
if let Some(hir_id) = tcx.hir().as_local_hir_id(did) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/stability.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ impl<'a, 'tcx: 'a> MissingStabilityAnnotations<'a, 'tcx> {
let stab = self.tcx.stability().local_stability(hir_id);
let is_error = !self.tcx.sess.opts.test &&
stab.is_none() &&
self.access_levels.is_reachable(self.tcx.hir().hir_to_node_id(hir_id));
self.access_levels.is_reachable(hir_id);
if is_error {
self.tcx.sess.span_err(
span,
Expand Down
12 changes: 4 additions & 8 deletions src/librustc_lint/builtin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,7 @@ impl MissingDoc {
// It's an option so the crate root can also use this function (it doesn't
// have a NodeId).
if let Some(id) = id {
let node_id = cx.tcx.hir().hir_to_node_id(id);
if !cx.access_levels.is_exported(node_id) {
if !cx.access_levels.is_exported(id) {
return;
}
}
Expand Down Expand Up @@ -557,8 +556,7 @@ impl LintPass for MissingCopyImplementations {

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingCopyImplementations {
fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &hir::Item) {
let node_id = cx.tcx.hir().hir_to_node_id(item.hir_id);
if !cx.access_levels.is_reachable(node_id) {
if !cx.access_levels.is_reachable(item.hir_id) {
return;
}
let (def, ty) = match item.node {
Expand Down Expand Up @@ -629,8 +627,7 @@ impl LintPass for MissingDebugImplementations {

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDebugImplementations {
fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &hir::Item) {
let node_id = cx.tcx.hir().hir_to_node_id(item.hir_id);
if !cx.access_levels.is_reachable(node_id) {
if !cx.access_levels.is_reachable(item.hir_id) {
return;
}

Expand Down Expand Up @@ -1169,9 +1166,8 @@ impl UnreachablePub {
fn perform_lint(&self, cx: &LateContext<'_, '_>, what: &str, id: hir::HirId,
vis: &hir::Visibility, span: Span, exportable: bool) {
let mut applicability = Applicability::MachineApplicable;
let node_id = cx.tcx.hir().hir_to_node_id(id);
match vis.node {
hir::VisibilityKind::Public if !cx.access_levels.is_reachable(node_id) => {
hir::VisibilityKind::Public if !cx.access_levels.is_reachable(id) => {
if span.ctxt().outer().expn_info().is_some() {
applicability = Applicability::MaybeIncorrect;
}
Expand Down
25 changes: 9 additions & 16 deletions src/librustc_privacy/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,8 @@ impl VisibilityLike for Option<AccessLevel> {
// (which require reaching the `DefId`s in them).
const SHALLOW: bool = true;
fn new_min<'a, 'tcx>(find: &FindMin<'a, 'tcx, Self>, def_id: DefId) -> Self {
cmp::min(if let Some(node_id) = find.tcx.hir().as_local_node_id(def_id) {
find.access_levels.map.get(&node_id).cloned()
cmp::min(if let Some(hir_id) = find.tcx.hir().as_local_hir_id(def_id) {
find.access_levels.map.get(&hir_id).cloned()
} else {
Self::MAX
}, find.min)
Expand Down Expand Up @@ -410,17 +410,15 @@ struct ReachEverythingInTheInterfaceVisitor<'b, 'a: 'b, 'tcx: 'a> {

impl<'a, 'tcx> EmbargoVisitor<'a, 'tcx> {
fn get(&self, id: hir::HirId) -> Option<AccessLevel> {
let node_id = self.tcx.hir().hir_to_node_id(id);
self.access_levels.map.get(&node_id).cloned()
self.access_levels.map.get(&id).cloned()
}

// Updates node level and returns the updated level.
fn update(&mut self, id: hir::HirId, level: Option<AccessLevel>) -> Option<AccessLevel> {
let old_level = self.get(id);
// Accessibility levels can only grow.
if level > old_level {
let node_id = self.tcx.hir().hir_to_node_id(id);
self.access_levels.map.insert(node_id, level.unwrap());
self.access_levels.map.insert(id, level.unwrap());
self.changed = true;
level
} else {
Expand Down Expand Up @@ -1197,8 +1195,7 @@ impl<'a, 'tcx> ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
fn trait_is_public(&self, trait_id: hir::HirId) -> bool {
// FIXME: this would preferably be using `exported_items`, but all
// traits are exported currently (see `EmbargoVisitor.exported_trait`).
let node_id = self.tcx.hir().hir_to_node_id(trait_id);
self.access_levels.is_public(node_id)
self.access_levels.is_public(trait_id)
}

fn check_generic_bound(&mut self, bound: &hir::GenericBound) {
Expand All @@ -1210,8 +1207,7 @@ impl<'a, 'tcx> ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
}

fn item_is_public(&self, id: &hir::HirId, vis: &hir::Visibility) -> bool {
let node_id = self.tcx.hir().hir_to_node_id(*id);
self.access_levels.is_reachable(node_id) || vis.node.is_pub()
self.access_levels.is_reachable(*id) || vis.node.is_pub()
}
}

Expand Down Expand Up @@ -1325,8 +1321,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
hir::ImplItemKind::Const(..) |
hir::ImplItemKind::Method(..) => {
self.access_levels.is_reachable(
self.tcx.hir().hir_to_node_id(
impl_item_ref.id.hir_id))
impl_item_ref.id.hir_id)
}
hir::ImplItemKind::Existential(..) |
hir::ImplItemKind::Type(_) => false,
Expand Down Expand Up @@ -1455,8 +1450,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
}

fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem) {
let node_id = self.tcx.hir().hir_to_node_id(item.hir_id);
if self.access_levels.is_reachable(node_id) {
if self.access_levels.is_reachable(item.hir_id) {
intravisit::walk_foreign_item(self, item)
}
}
Expand All @@ -1474,8 +1468,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> {
v: &'tcx hir::Variant,
g: &'tcx hir::Generics,
item_id: hir::HirId) {
let node_id = self.tcx.hir().hir_to_node_id(v.node.data.hir_id());
if self.access_levels.is_reachable(node_id) {
if self.access_levels.is_reachable(v.node.data.hir_id()) {
self.in_variant = true;
intravisit::walk_variant(self, v, g, item_id);
self.in_variant = false;
Expand Down
51 changes: 33 additions & 18 deletions src/librustc_save_analysis/dump_visitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,19 @@ macro_rules! down_cast_data {
}

macro_rules! access_from {
($save_ctxt:expr, $vis:expr, $id:expr) => {
($save_ctxt:expr, $item:expr, $id:expr) => {
Access {
public: $vis.node.is_pub(),
public: $item.vis.node.is_pub(),
reachable: $save_ctxt.access_levels.is_reachable($id),
}
};
}

($save_ctxt:expr, $item:expr) => {
macro_rules! access_from_vis {
($save_ctxt:expr, $vis:expr, $id:expr) => {
Access {
public: $item.vis.node.is_pub(),
reachable: $save_ctxt.access_levels.is_reachable($item.id),
public: $vis.node.is_pub(),
reachable: $save_ctxt.access_levels.is_reachable($id),
}
};
}
Expand Down Expand Up @@ -303,7 +305,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {

method_data.value = sig_str;
method_data.sig = sig::method_signature(id, ident, generics, sig, &self.save_ctxt);
self.dumper.dump_def(&access_from!(self.save_ctxt, vis, id), method_data);
let hir_id = self.tcx.hir().node_to_hir_id(id);
self.dumper.dump_def(&access_from_vis!(self.save_ctxt, vis, hir_id), method_data);
}

// walk arg and return types
Expand All @@ -324,7 +327,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
fn process_struct_field_def(&mut self, field: &ast::StructField, parent_id: NodeId) {
let field_data = self.save_ctxt.get_field_data(field, parent_id);
if let Some(field_data) = field_data {
self.dumper.dump_def(&access_from!(self.save_ctxt, field), field_data);
let hir_id = self.tcx.hir().node_to_hir_id(field.id);
self.dumper.dump_def(&access_from!(self.save_ctxt, field, hir_id), field_data);
}
}

Expand Down Expand Up @@ -389,7 +393,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
|v| v.process_formals(&decl.inputs, &fn_data.qualname),
);
self.process_generic_params(ty_params, &fn_data.qualname, item.id);
self.dumper.dump_def(&access_from!(self.save_ctxt, item), fn_data);
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
self.dumper.dump_def(&access_from!(self.save_ctxt, item, hir_id), fn_data);
}

for arg in &decl.inputs {
Expand All @@ -409,10 +414,11 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
typ: &'l ast::Ty,
expr: &'l ast::Expr,
) {
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
self.nest_tables(item.id, |v| {
if let Some(var_data) = v.save_ctxt.get_item_data(item) {
down_cast_data!(var_data, DefData, item.span);
v.dumper.dump_def(&access_from!(v.save_ctxt, item), var_data);
v.dumper.dump_def(&access_from!(v.save_ctxt, item, hir_id), var_data);
}
v.visit_ty(&typ);
v.visit_expr(expr);
Expand All @@ -435,9 +441,10 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
if !self.span.filter_generated(ident.span) {
let sig = sig::assoc_const_signature(id, ident.name, typ, expr, &self.save_ctxt);
let span = self.span_from_span(ident.span);
let hir_id = self.tcx.hir().node_to_hir_id(id);

self.dumper.dump_def(
&access_from!(self.save_ctxt, vis, id),
&access_from_vis!(self.save_ctxt, vis, hir_id),
Def {
kind: DefKind::Const,
id: id_from_node_id(id, &self.save_ctxt),
Expand Down Expand Up @@ -512,8 +519,9 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {

if !self.span.filter_generated(item.ident.span) {
let span = self.span_from_span(item.ident.span);
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
self.dumper.dump_def(
&access_from!(self.save_ctxt, item),
&access_from!(self.save_ctxt, item, hir_id),
Def {
kind,
id: id_from_node_id(item.id, &self.save_ctxt),
Expand Down Expand Up @@ -552,7 +560,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
};
down_cast_data!(enum_data, DefData, item.span);

let access = access_from!(self.save_ctxt, item);
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
let access = access_from!(self.save_ctxt, item, hir_id);

for variant in &enum_definition.variants {
let name = variant.node.ident.name.to_string();
Expand Down Expand Up @@ -701,8 +710,9 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
.iter()
.map(|i| id_from_node_id(i.id, &self.save_ctxt))
.collect();
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
self.dumper.dump_def(
&access_from!(self.save_ctxt, item),
&access_from!(self.save_ctxt, item, hir_id),
Def {
kind: DefKind::Trait,
id,
Expand Down Expand Up @@ -760,7 +770,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
fn process_mod(&mut self, item: &ast::Item) {
if let Some(mod_data) = self.save_ctxt.get_item_data(item) {
down_cast_data!(mod_data, DefData, item.span);
self.dumper.dump_def(&access_from!(self.save_ctxt, item), mod_data);
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
self.dumper.dump_def(&access_from!(self.save_ctxt, item, hir_id), mod_data);
}
}

Expand Down Expand Up @@ -1201,7 +1212,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {

// The access is calculated using the current tree ID, but with the root tree's visibility
// (since nested trees don't have their own visibility).
let access = access_from!(self.save_ctxt, root_item.vis, id);
let hir_id = self.tcx.hir().node_to_hir_id(id);
let access = access_from!(self.save_ctxt, root_item, hir_id);

// The parent def id of a given use tree is always the enclosing item.
let parent = self.save_ctxt.tcx.hir().opt_local_def_id(id)
Expand Down Expand Up @@ -1400,9 +1412,10 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
if !self.span.filter_generated(item.ident.span) {
let span = self.span_from_span(item.ident.span);
let id = id_from_node_id(item.id, &self.save_ctxt);
let hir_id = self.tcx.hir().node_to_hir_id(item.id);

self.dumper.dump_def(
&access_from!(self.save_ctxt, item),
&access_from!(self.save_ctxt, item, hir_id),
Def {
kind: DefKind::Type,
id,
Expand Down Expand Up @@ -1431,9 +1444,10 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
if !self.span.filter_generated(item.ident.span) {
let span = self.span_from_span(item.ident.span);
let id = id_from_node_id(item.id, &self.save_ctxt);
let hir_id = self.tcx.hir().node_to_hir_id(item.id);

self.dumper.dump_def(
&access_from!(self.save_ctxt, item),
&access_from!(self.save_ctxt, item, hir_id),
Def {
kind: DefKind::Type,
id,
Expand Down Expand Up @@ -1631,7 +1645,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> Visitor<'l> for DumpVisitor<'l, 'tc
}

fn visit_foreign_item(&mut self, item: &'l ast::ForeignItem) {
let access = access_from!(self.save_ctxt, item);
let hir_id = self.tcx.hir().node_to_hir_id(item.id);
let access = access_from!(self.save_ctxt, item, hir_id);

match item.node {
ast::ForeignItemKind::Fn(ref decl, ref generics) => {
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/core.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,11 +461,11 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
sess.abort_if_errors();

let access_levels = tcx.privacy_access_levels(LOCAL_CRATE);
// Convert from a NodeId set to a DefId set since we don't always have easy access
// to the map from defid -> nodeid
// Convert from a HirId set to a DefId set since we don't always have easy access
// to the map from defid -> hirid
let access_levels = AccessLevels {
map: access_levels.map.iter()
.map(|(&k, &v)| (tcx.hir().local_def_id(k), v))
.map(|(&k, &v)| (tcx.hir().local_def_id_from_hir_id(k), v))
.collect()
};

Expand Down

0 comments on commit f0cdccd

Please sign in to comment.