Skip to content

rustc: Replace HirIds with LocalDefIds in AccessLevels tables #87568

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 2 commits into from
Aug 4, 2021
+299 −333
Merged

rustc: Replace HirIds with LocalDefIds in AccessLevels tables #87568

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b08576b
rustc: Replace `HirId`s with `LocalDefId`s in `AccessLevels` tables
petrochenkov Jul 28, 2021
f921410
Fix clippy
petrochenkov Jul 28, 2021
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
44 changes: 23 additions & 21 deletions compiler/rustc_lint/src/builtin.rs
View file
Original file line number Diff line number Diff line change
@@ -38,7 +38,7 @@ use rustc_feature::{deprecated_attributes, AttributeGate, AttributeTemplate, Att
use rustc_feature::{GateIssue, Stability};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet};
use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID};
use rustc_hir::{ForeignItemKind, GenericParamKind, PatKind};
use rustc_hir::{HirId, Node};
use rustc_index::vec::Idx;
@@ -511,7 +511,7 @@ impl MissingDoc {
fn check_missing_docs_attrs(
&self,
cx: &LateContext<'_>,
id: hir::HirId,
def_id: LocalDefId,
sp: Span,
article: &'static str,
desc: &'static str,
@@ -530,13 +530,13 @@ impl MissingDoc {
// Only check publicly-visible items, using the result from the privacy pass.
// It's an option so the crate root can also use this function (it doesn't
// have a `NodeId`).
if id != hir::CRATE_HIR_ID {
if !cx.access_levels.is_exported(id) {
if def_id != CRATE_DEF_ID {
if !cx.access_levels.is_exported(def_id) {
return;
}
}

let attrs = cx.tcx.hir().attrs(id);
let attrs = cx.tcx.get_attrs(def_id.to_def_id());
let has_doc = attrs.iter().any(|a| has_doc(cx.sess(), a));
if !has_doc {
cx.struct_span_lint(
@@ -568,12 +568,12 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
}

fn check_crate(&mut self, cx: &LateContext<'_>, krate: &hir::Crate<'_>) {
self.check_missing_docs_attrs(cx, hir::CRATE_HIR_ID, krate.module().inner, "the", "crate");
self.check_missing_docs_attrs(cx, CRATE_DEF_ID, krate.module().inner, "the", "crate");

for macro_def in krate.exported_macros() {
// Non exported macros should be skipped, since `missing_docs` only
// applies to externally visible items.
if !cx.access_levels.is_exported(macro_def.hir_id()) {
if !cx.access_levels.is_exported(macro_def.def_id) {
continue;
}

@@ -632,7 +632,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {

let (article, desc) = cx.tcx.article_and_description(it.def_id.to_def_id());

self.check_missing_docs_attrs(cx, it.hir_id(), it.span, article, desc);
self.check_missing_docs_attrs(cx, it.def_id, it.span, article, desc);
}

fn check_trait_item(&mut self, cx: &LateContext<'_>, trait_item: &hir::TraitItem<'_>) {
@@ -642,7 +642,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {

let (article, desc) = cx.tcx.article_and_description(trait_item.def_id.to_def_id());

self.check_missing_docs_attrs(cx, trait_item.hir_id(), trait_item.span, article, desc);
self.check_missing_docs_attrs(cx, trait_item.def_id, trait_item.span, article, desc);
}

fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
@@ -652,22 +652,23 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
}

let (article, desc) = cx.tcx.article_and_description(impl_item.def_id.to_def_id());
self.check_missing_docs_attrs(cx, impl_item.hir_id(), impl_item.span, article, desc);
self.check_missing_docs_attrs(cx, impl_item.def_id, impl_item.span, article, desc);
}

fn check_foreign_item(&mut self, cx: &LateContext<'_>, foreign_item: &hir::ForeignItem<'_>) {
let (article, desc) = cx.tcx.article_and_description(foreign_item.def_id.to_def_id());
self.check_missing_docs_attrs(cx, foreign_item.hir_id(), foreign_item.span, article, desc);
self.check_missing_docs_attrs(cx, foreign_item.def_id, foreign_item.span, article, desc);
}

fn check_field_def(&mut self, cx: &LateContext<'_>, sf: &hir::FieldDef<'_>) {
if !sf.is_positional() {
self.check_missing_docs_attrs(cx, sf.hir_id, sf.span, "a", "struct field")
let def_id = cx.tcx.hir().local_def_id(sf.hir_id);
self.check_missing_docs_attrs(cx, def_id, sf.span, "a", "struct field")
}
}

fn check_variant(&mut self, cx: &LateContext<'_>, v: &hir::Variant<'_>) {
self.check_missing_docs_attrs(cx, v.id, v.span, "a", "variant");
self.check_missing_docs_attrs(cx, cx.tcx.hir().local_def_id(v.id), v.span, "a", "variant");
}
}

@@ -709,7 +710,7 @@ declare_lint_pass!(MissingCopyImplementations => [MISSING_COPY_IMPLEMENTATIONS])

impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
if !cx.access_levels.is_reachable(item.hir_id()) {
if !cx.access_levels.is_reachable(item.def_id) {
return;
}
let (def, ty) = match item.kind {
@@ -796,7 +797,7 @@ impl_lint_pass!(MissingDebugImplementations => [MISSING_DEBUG_IMPLEMENTATIONS]);

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

@@ -1314,14 +1315,14 @@ impl UnreachablePub {
&self,
cx: &LateContext<'_>,
what: &str,
id: hir::HirId,
def_id: LocalDefId,
vis: &hir::Visibility<'_>,
span: Span,
exportable: bool,
) {
let mut applicability = Applicability::MachineApplicable;
match vis.node {
hir::VisibilityKind::Public if !cx.access_levels.is_reachable(id) => {
hir::VisibilityKind::Public if !cx.access_levels.is_reachable(def_id) => {
if span.from_expansion() {
applicability = Applicability::MaybeIncorrect;
}
@@ -1354,26 +1355,27 @@ impl UnreachablePub {

impl<'tcx> LateLintPass<'tcx> for UnreachablePub {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
self.perform_lint(cx, "item", item.hir_id(), &item.vis, item.span, true);
self.perform_lint(cx, "item", item.def_id, &item.vis, item.span, true);
}

fn check_foreign_item(&mut self, cx: &LateContext<'_>, foreign_item: &hir::ForeignItem<'tcx>) {
self.perform_lint(
cx,
"item",
foreign_item.hir_id(),
foreign_item.def_id,
&foreign_item.vis,
foreign_item.span,
true,
);
}

fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) {
self.perform_lint(cx, "field", field.hir_id, &field.vis, field.span, false);
let def_id = cx.tcx.hir().local_def_id(field.hir_id);
self.perform_lint(cx, "field", def_id, &field.vis, field.span, false);
}

fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
self.perform_lint(cx, "item", impl_item.hir_id(), &impl_item.vis, impl_item.span, false);
self.perform_lint(cx, "item", impl_item.def_id, &impl_item.vis, impl_item.span, false);
}
}

8 changes: 0 additions & 8 deletions compiler/rustc_middle/src/hir/map/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -581,14 +581,6 @@ impl<'hir> Map<'hir> {
self.body_const_context(self.local_def_id(self.enclosing_body_owner(hir_id))).is_some()
}

/// Whether `hir_id` corresponds to a `mod` or a crate.
pub fn is_hir_id_module(&self, hir_id: HirId) -> bool {
matches!(
self.get(hir_id),
Node::Item(Item { kind: ItemKind::Mod(_), .. }) | Node::Crate(..)
)
}

/// Retrieves the `HirId` for `id`'s enclosing method, unless there's a
/// `while` or `loop` before reaching it, as block tail returns are not
/// available in them.
15 changes: 4 additions & 11 deletions compiler/rustc_middle/src/middle/privacy.rs
View file
Original file line number Diff line number Diff line change
@@ -3,9 +3,8 @@
//! which are available for use externally when compiled as a library.

use rustc_data_structures::fx::FxHashMap;
use rustc_hir::HirId;
use rustc_macros::HashStable;
use std::fmt;
use rustc_span::def_id::LocalDefId;
use std::hash::Hash;

/// Represents the levels of accessibility an item can have.
@@ -27,8 +26,8 @@ pub enum AccessLevel {
}

/// Holds a map of accessibility levels for reachable HIR nodes.
#[derive(Clone)]
pub struct AccessLevels<Id = HirId> {
#[derive(Debug)]
pub struct AccessLevels<Id = LocalDefId> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used somewhere with another Id?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, rustdoc uses AccessLevels<DefId> and actually adds foreign ids to the table as public or exported, I didn't investigate why exactly.

Copy link
Member

@jyn514 jyn514 Jul 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC it's because rustdoc needs to do its own privacy pass because rustc's is incorrect (#64762). It may be possible to remove when that's fixed, but don't quote me on that.

pub map: FxHashMap<Id, AccessLevel>,
}

@@ -49,14 +48,8 @@ impl<Id: Hash + Eq> AccessLevels<Id> {
}
}

impl<Id: Hash + Eq> Default for AccessLevels<Id> {
impl<Id> Default for AccessLevels<Id> {
fn default() -> Self {
AccessLevels { map: Default::default() }
}
}

impl<Id: Hash + Eq + fmt::Debug> fmt::Debug for AccessLevels<Id> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Debug::fmt(&self.map, f)
}
}
26 changes: 13 additions & 13 deletions compiler/rustc_middle/src/middle/stability.rs
View file
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_feature::GateIssue;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX};
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_INDEX};
use rustc_hir::{self, HirId};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_session::lint::builtin::{DEPRECATED, DEPRECATED_IN_FUTURE, SOFT_UNSTABLE};
@@ -36,12 +36,12 @@ pub struct DeprecationEntry {
pub attr: Deprecation,
/// The `DefId` where the attr was originally attached. `None` for non-local
/// `DefId`'s.
origin: Option<HirId>,
origin: Option<LocalDefId>,
}

impl DeprecationEntry {
pub fn local(attr: Deprecation, id: HirId) -> DeprecationEntry {
DeprecationEntry { attr, origin: Some(id) }
pub fn local(attr: Deprecation, def_id: LocalDefId) -> DeprecationEntry {
DeprecationEntry { attr, origin: Some(def_id) }
}

pub fn external(attr: Deprecation) -> DeprecationEntry {
@@ -61,9 +61,9 @@ impl DeprecationEntry {
pub struct Index<'tcx> {
/// This is mostly a cache, except the stabilities of local items
/// are filled by the annotator.
pub stab_map: FxHashMap<HirId, &'tcx Stability>,
pub const_stab_map: FxHashMap<HirId, &'tcx ConstStability>,
pub depr_map: FxHashMap<HirId, DeprecationEntry>,
pub stab_map: FxHashMap<LocalDefId, &'tcx Stability>,
pub const_stab_map: FxHashMap<LocalDefId, &'tcx ConstStability>,
pub depr_map: FxHashMap<LocalDefId, DeprecationEntry>,

/// Maps for each crate whether it is part of the staged API.
pub staged_api: FxHashMap<CrateNum, bool>,
@@ -73,16 +73,16 @@ pub struct Index<'tcx> {
}

impl<'tcx> Index<'tcx> {
pub fn local_stability(&self, id: HirId) -> Option<&'tcx Stability> {
self.stab_map.get(&id).cloned()
pub fn local_stability(&self, def_id: LocalDefId) -> Option<&'tcx Stability> {
self.stab_map.get(&def_id).copied()
}

pub fn local_const_stability(&self, id: HirId) -> Option<&'tcx ConstStability> {
self.const_stab_map.get(&id).cloned()
pub fn local_const_stability(&self, def_id: LocalDefId) -> Option<&'tcx ConstStability> {
self.const_stab_map.get(&def_id).copied()
}

pub fn local_deprecation_entry(&self, id: HirId) -> Option<DeprecationEntry> {
self.depr_map.get(&id).cloned()
pub fn local_deprecation_entry(&self, def_id: LocalDefId) -> Option<DeprecationEntry> {
self.depr_map.get(&def_id).cloned()
}
}

17 changes: 5 additions & 12 deletions compiler/rustc_middle/src/ty/context.rs
View file
Original file line number Diff line number Diff line change
@@ -2854,18 +2854,11 @@ pub fn provide(providers: &mut ty::query::Providers) {
tcx.arena.alloc(tcx.resolutions(()).glob_map.get(&id).cloned().unwrap_or_default())
};

providers.lookup_stability = |tcx, id| {
let id = tcx.hir().local_def_id_to_hir_id(id.expect_local());
tcx.stability().local_stability(id)
};
providers.lookup_const_stability = |tcx, id| {
let id = tcx.hir().local_def_id_to_hir_id(id.expect_local());
tcx.stability().local_const_stability(id)
};
providers.lookup_deprecation_entry = |tcx, id| {
let id = tcx.hir().local_def_id_to_hir_id(id.expect_local());
tcx.stability().local_deprecation_entry(id)
};
providers.lookup_stability = |tcx, id| tcx.stability().local_stability(id.expect_local());
providers.lookup_const_stability =
|tcx, id| tcx.stability().local_const_stability(id.expect_local());
providers.lookup_deprecation_entry =
|tcx, id| tcx.stability().local_deprecation_entry(id.expect_local());
providers.extern_mod_stmt_cnum =
|tcx, id| tcx.resolutions(()).extern_crate_map.get(&id).cloned();
providers.output_filenames = |tcx, ()| tcx.output_filenames.clone();
Loading