Skip to content

Commit

Permalink
Rollup merge of rust-lang#107325 - petrochenkov:hiddoc2, r=GuillaumeG…
Browse files Browse the repository at this point in the history 
…omez

rustdoc: Stop using `HirId`s

Use `LocalDefId`s instead.
Rustdoc doesn't work with item bodies, so it almost never needs fine-grained HIR IDs.
  • Loading branch information
@JohnTitor
JohnTitor authored Jan 27, 2023
2 parents 74945ca + 347fa7a commit eee6648
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 151 deletions.
36 changes: 18 additions & 18 deletions src/librustdoc/clean/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_attr as attr;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet, IndexEntry};
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, LOCAL_CRATE};
use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, LocalDefId, LOCAL_CRATE};
use rustc_hir::PredicateOrigin;
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_infer::infer::region_constraints::{Constraint, RegionConstraintData};
Expand Down Expand Up @@ -116,7 +116,8 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
}
});

Item::from_hir_id_and_parts(doc.id, Some(doc.name), ModuleItem(Module { items, span }), cx)
let kind = ModuleItem(Module { items, span });
Item::from_def_id_and_parts(doc.def_id.to_def_id(), Some(doc.name), kind, cx)
}

fn clean_generic_bound<'tcx>(
Expand Down Expand Up @@ -2067,12 +2068,12 @@ struct OneLevelVisitor<'hir> {
map: rustc_middle::hir::map::Map<'hir>,
item: Option<&'hir hir::Item<'hir>>,
looking_for: Ident,
target_hir_id: hir::HirId,
target_def_id: LocalDefId,
}

impl<'hir> OneLevelVisitor<'hir> {
fn new(map: rustc_middle::hir::map::Map<'hir>, target_hir_id: hir::HirId) -> Self {
Self { map, item: None, looking_for: Ident::empty(), target_hir_id }
fn new(map: rustc_middle::hir::map::Map<'hir>, target_def_id: LocalDefId) -> Self {
Self { map, item: None, looking_for: Ident::empty(), target_def_id }
}

fn reset(&mut self, looking_for: Ident) {
Expand All @@ -2092,7 +2093,7 @@ impl<'hir> hir::intravisit::Visitor<'hir> for OneLevelVisitor<'hir> {
if self.item.is_none()
&& item.ident == self.looking_for
&& matches!(item.kind, hir::ItemKind::Use(_, _))
|| item.hir_id() == self.target_hir_id
|| item.owner_id.def_id == self.target_def_id
{
self.item = Some(item);
}
Expand All @@ -2106,11 +2107,11 @@ impl<'hir> hir::intravisit::Visitor<'hir> for OneLevelVisitor<'hir> {
fn get_all_import_attributes<'hir>(
mut item: &hir::Item<'hir>,
tcx: TyCtxt<'hir>,
target_hir_id: hir::HirId,
target_def_id: LocalDefId,
attributes: &mut Vec<ast::Attribute>,
) {
let hir_map = tcx.hir();
let mut visitor = OneLevelVisitor::new(hir_map, target_hir_id);
let mut visitor = OneLevelVisitor::new(hir_map, target_def_id);
// If the item is an import and has at least a path with two parts, we go into it.
while let hir::ItemKind::Use(path, _) = item.kind &&
path.segments.len() > 1 &&
Expand Down Expand Up @@ -2138,7 +2139,7 @@ fn clean_maybe_renamed_item<'tcx>(
cx: &mut DocContext<'tcx>,
item: &hir::Item<'tcx>,
renamed: Option<Symbol>,
import_id: Option<hir::HirId>,
import_id: Option<LocalDefId>,
) -> Vec<Item> {
use hir::ItemKind;

Expand Down Expand Up @@ -2183,7 +2184,7 @@ fn clean_maybe_renamed_item<'tcx>(
generics: clean_generics(generics, cx),
fields: variant_data.fields().iter().map(|x| clean_field(x, cx)).collect(),
}),
ItemKind::Impl(impl_) => return clean_impl(impl_, item.hir_id(), cx),
ItemKind::Impl(impl_) => return clean_impl(impl_, item.owner_id.def_id, cx),
// proc macros can have a name set by attributes
ItemKind::Fn(ref sig, generics, body_id) => {
clean_fn_or_proc_macro(item, sig, generics, body_id, &mut name, cx)
Expand Down Expand Up @@ -2218,10 +2219,10 @@ fn clean_maybe_renamed_item<'tcx>(

let mut extra_attrs = Vec::new();
if let Some(hir::Node::Item(use_node)) =
import_id.and_then(|hir_id| cx.tcx.hir().find(hir_id))
import_id.and_then(|def_id| cx.tcx.hir().find_by_def_id(def_id))
{
// We get all the various imports' attributes.
get_all_import_attributes(use_node, cx.tcx, item.hir_id(), &mut extra_attrs);
get_all_import_attributes(use_node, cx.tcx, item.owner_id.def_id, &mut extra_attrs);
}

if !extra_attrs.is_empty() {
Expand All @@ -2244,12 +2245,12 @@ fn clean_maybe_renamed_item<'tcx>(

fn clean_variant<'tcx>(variant: &hir::Variant<'tcx>, cx: &mut DocContext<'tcx>) -> Item {
let kind = VariantItem(clean_variant_data(&variant.data, &variant.disr_expr, cx));
Item::from_hir_id_and_parts(variant.hir_id, Some(variant.ident.name), kind, cx)
Item::from_def_id_and_parts(variant.def_id.to_def_id(), Some(variant.ident.name), kind, cx)
}

fn clean_impl<'tcx>(
impl_: &hir::Impl<'tcx>,
hir_id: hir::HirId,
def_id: LocalDefId,
cx: &mut DocContext<'tcx>,
) -> Vec<Item> {
let tcx = cx.tcx;
Expand All @@ -2260,7 +2261,6 @@ fn clean_impl<'tcx>(
.iter()
.map(|ii| clean_impl_item(tcx.hir().impl_item(ii.id), cx))
.collect::<Vec<_>>();
let def_id = tcx.hir().local_def_id(hir_id);

// If this impl block is an implementation of the Deref trait, then we
// need to try inlining the target's inherent impl blocks as well.
Expand Down Expand Up @@ -2289,7 +2289,7 @@ fn clean_impl<'tcx>(
ImplKind::Normal
},
}));
Item::from_hir_id_and_parts(hir_id, None, kind, cx)
Item::from_def_id_and_parts(def_id.to_def_id(), None, kind, cx)
};
if let Some(type_alias) = type_alias {
ret.push(make_item(trait_.clone(), type_alias, items.clone()));
Expand Down Expand Up @@ -2510,8 +2510,8 @@ fn clean_maybe_renamed_foreign_item<'tcx>(
hir::ForeignItemKind::Type => ForeignTypeItem,
};

Item::from_hir_id_and_parts(
item.hir_id(),
Item::from_def_id_and_parts(
item.owner_id.def_id.to_def_id(),
Some(renamed.unwrap_or(item.ident.name)),
kind,
cx,
Expand Down
16 changes: 1 addition & 15 deletions src/librustdoc/clean/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,17 +439,6 @@ impl Item {
self.attrs.doc_value()
}

/// Convenience wrapper around [`Self::from_def_id_and_parts`] which converts
/// `hir_id` to a [`DefId`]
pub(crate) fn from_hir_id_and_parts(
hir_id: hir::HirId,
name: Option<Symbol>,
kind: ItemKind,
cx: &mut DocContext<'_>,
) -> Item {
Item::from_def_id_and_parts(cx.tcx.hir().local_def_id(hir_id).to_def_id(), name, kind, cx)
}

pub(crate) fn from_def_id_and_parts(
def_id: DefId,
name: Option<Symbol>,
Expand Down Expand Up @@ -2416,10 +2405,7 @@ impl ConstantKind {

pub(crate) fn is_literal(&self, tcx: TyCtxt<'_>) -> bool {
match *self {
ConstantKind::TyConst { .. } => false,
ConstantKind::Extern { def_id } => def_id.as_local().map_or(false, |def_id| {
is_literal_expr(tcx, tcx.hir().local_def_id_to_hir_id(def_id))
}),
ConstantKind::TyConst { .. } | ConstantKind::Extern { .. } => false,
ConstantKind::Local { body, .. } | ConstantKind::Anonymous { body } => {
is_literal_expr(tcx, body.hir_id)
}
Expand Down
26 changes: 12 additions & 14 deletions src/librustdoc/doctest.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ use rustc_ast as ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::Lrc;
use rustc_errors::{ColorConfig, ErrorGuaranteed, FatalError};
use rustc_hir as hir;
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_hir::intravisit;
use rustc_hir::{HirId, CRATE_HIR_ID};
use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
use rustc_hir::{self as hir, intravisit, CRATE_HIR_ID};
use rustc_interface::interface;
use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter;
Expand Down Expand Up @@ -140,7 +138,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
};
hir_collector.visit_testable(
"".to_string(),
CRATE_HIR_ID,
CRATE_DEF_ID,
tcx.hir().span(CRATE_HIR_ID),
|this| tcx.hir().walk_toplevel_module(this),
);
Expand Down Expand Up @@ -1214,11 +1212,11 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
fn visit_testable<F: FnOnce(&mut Self)>(
&mut self,
name: String,
hir_id: HirId,
def_id: LocalDefId,
sp: Span,
nested: F,
) {
let ast_attrs = self.tcx.hir().attrs(hir_id);
let ast_attrs = self.tcx.hir().attrs(self.tcx.hir().local_def_id_to_hir_id(def_id));
if let Some(ref cfg) = ast_attrs.cfg(self.tcx, &FxHashSet::default()) {
if !cfg.matches(&self.sess.parse_sess, Some(self.tcx.features())) {
return;
Expand Down Expand Up @@ -1247,7 +1245,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
self.collector.enable_per_target_ignores,
Some(&crate::html::markdown::ExtraInfo::new(
self.tcx,
hir_id,
def_id.to_def_id(),
span_of_attrs(&attrs).unwrap_or(sp),
)),
);
Expand Down Expand Up @@ -1276,37 +1274,37 @@ impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx>
_ => item.ident.to_string(),
};

self.visit_testable(name, item.hir_id(), item.span, |this| {
self.visit_testable(name, item.owner_id.def_id, item.span, |this| {
intravisit::walk_item(this, item);
});
}

fn visit_trait_item(&mut self, item: &'hir hir::TraitItem<'_>) {
self.visit_testable(item.ident.to_string(), item.hir_id(), item.span, |this| {
self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| {
intravisit::walk_trait_item(this, item);
});
}

fn visit_impl_item(&mut self, item: &'hir hir::ImplItem<'_>) {
self.visit_testable(item.ident.to_string(), item.hir_id(), item.span, |this| {
self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| {
intravisit::walk_impl_item(this, item);
});
}

fn visit_foreign_item(&mut self, item: &'hir hir::ForeignItem<'_>) {
self.visit_testable(item.ident.to_string(), item.hir_id(), item.span, |this| {
self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| {
intravisit::walk_foreign_item(this, item);
});
}

fn visit_variant(&mut self, v: &'hir hir::Variant<'_>) {
self.visit_testable(v.ident.to_string(), v.hir_id, v.span, |this| {
self.visit_testable(v.ident.to_string(), v.def_id, v.span, |this| {
intravisit::walk_variant(this, v);
});
}

fn visit_field_def(&mut self, f: &'hir hir::FieldDef<'_>) {
self.visit_testable(f.ident.to_string(), f.hir_id, f.span, |this| {
self.visit_testable(f.ident.to_string(), f.def_id, f.span, |this| {
intravisit::walk_field_def(this, f);
});
}
Expand Down
44 changes: 12 additions & 32 deletions src/librustdoc/html/markdown.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def_id::DefId;
use rustc_hir::HirId;
use rustc_middle::ty::TyCtxt;
use rustc_span::edition::Edition;
use rustc_span::{Span, Symbol};
Expand Down Expand Up @@ -801,45 +800,26 @@ pub(crate) fn find_testable_code<T: doctest::Tester>(
}

pub(crate) struct ExtraInfo<'tcx> {
id: ExtraInfoId,
def_id: DefId,
sp: Span,
tcx: TyCtxt<'tcx>,
}

enum ExtraInfoId {
Hir(HirId),
Def(DefId),
}

impl<'tcx> ExtraInfo<'tcx> {
pub(crate) fn new(tcx: TyCtxt<'tcx>, hir_id: HirId, sp: Span) -> ExtraInfo<'tcx> {
ExtraInfo { id: ExtraInfoId::Hir(hir_id), sp, tcx }
}

pub(crate) fn new_did(tcx: TyCtxt<'tcx>, did: DefId, sp: Span) -> ExtraInfo<'tcx> {
ExtraInfo { id: ExtraInfoId::Def(did), sp, tcx }
pub(crate) fn new(tcx: TyCtxt<'tcx>, def_id: DefId, sp: Span) -> ExtraInfo<'tcx> {
ExtraInfo { def_id, sp, tcx }
}

fn error_invalid_codeblock_attr(&self, msg: &str, help: &str) {
let hir_id = match self.id {
ExtraInfoId::Hir(hir_id) => hir_id,
ExtraInfoId::Def(item_did) => {
match item_did.as_local() {
Some(item_did) => self.tcx.hir().local_def_id_to_hir_id(item_did),
None => {
// If non-local, no need to check anything.
return;
}
}
}
};
self.tcx.struct_span_lint_hir(
crate::lint::INVALID_CODEBLOCK_ATTRIBUTES,
hir_id,
self.sp,
msg,
|lint| lint.help(help),
);
if let Some(def_id) = self.def_id.as_local() {
self.tcx.struct_span_lint_hir(
crate::lint::INVALID_CODEBLOCK_ATTRIBUTES,
self.tcx.hir().local_def_id_to_hir_id(def_id),
self.sp,
msg,
|lint| lint.help(help),
);
}
}
}

Expand Down
8 changes: 1 addition & 7 deletions src/librustdoc/passes/calculate_doc_coverage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,7 @@ impl<'a, 'b> DocVisitor for CoverageCalculator<'a, 'b> {
);

let has_doc_example = tests.found_tests != 0;
// The `expect_def_id()` should be okay because `local_def_id_to_hir_id`
// would presumably panic if a fake `DefIndex` were passed.
let hir_id = self
.ctx
.tcx
.hir()
.local_def_id_to_hir_id(i.item_id.expect_def_id().expect_local());
let hir_id = DocContext::as_local_hir_id(self.ctx.tcx, i.item_id).unwrap();
let (level, source) = self.ctx.tcx.lint_level_at_node(MISSING_DOCS, hir_id);

// In case we have:
Expand Down
19 changes: 11 additions & 8 deletions src/librustdoc/passes/check_doc_test_visibility.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use crate::visit::DocVisitor;
use crate::visit_ast::inherits_doc_hidden;
use rustc_hir as hir;
use rustc_middle::lint::LintLevelSource;
use rustc_middle::ty::DefIdTree;
use rustc_session::lint;
use rustc_span::symbol::sym;

pub(crate) const CHECK_DOC_TEST_VISIBILITY: Pass = Pass {
name: "check_doc_test_visibility",
Expand Down Expand Up @@ -79,11 +79,11 @@ pub(crate) fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -

// The `expect_def_id()` should be okay because `local_def_id_to_hir_id`
// would presumably panic if a fake `DefIndex` were passed.
let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.item_id.expect_def_id().expect_local());
let def_id = item.item_id.expect_def_id().expect_local();

// check if parent is trait impl
if let Some(parent_hir_id) = cx.tcx.hir().opt_parent_id(hir_id) {
if let Some(parent_node) = cx.tcx.hir().find(parent_hir_id) {
if let Some(parent_def_id) = cx.tcx.opt_local_parent(def_id) {
if let Some(parent_node) = cx.tcx.hir().find_by_def_id(parent_def_id) {
if matches!(
parent_node,
hir::Node::Item(hir::Item {
Expand All @@ -96,13 +96,16 @@ pub(crate) fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -
}
}

if cx.tcx.hir().attrs(hir_id).lists(sym::doc).has_word(sym::hidden)
|| inherits_doc_hidden(cx.tcx, hir_id)
|| cx.tcx.hir().span(hir_id).in_derive_expansion()
if cx.tcx.is_doc_hidden(def_id.to_def_id())
|| inherits_doc_hidden(cx.tcx, def_id)
|| cx.tcx.def_span(def_id.to_def_id()).in_derive_expansion()
{
return false;
}
let (level, source) = cx.tcx.lint_level_at_node(crate::lint::MISSING_DOC_CODE_EXAMPLES, hir_id);
let (level, source) = cx.tcx.lint_level_at_node(
crate::lint::MISSING_DOC_CODE_EXAMPLES,
cx.tcx.hir().local_def_id_to_hir_id(def_id),
);
level != lint::Level::Allow || matches!(source, LintLevelSource::Default)
}

Expand Down
11 changes: 3 additions & 8 deletions src/librustdoc/passes/collect_intra_doc_links.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1194,14 +1194,9 @@ impl LinkCollector<'_, '_> {
}

// item can be non-local e.g. when using #[doc(primitive = "pointer")]
if let Some((src_id, dst_id)) = id
.as_local()
// The `expect_def_id()` should be okay because `local_def_id_to_hir_id`
// would presumably panic if a fake `DefIndex` were passed.
.and_then(|dst_id| {
item.item_id.expect_def_id().as_local().map(|src_id| (src_id, dst_id))
})
{
if let Some((src_id, dst_id)) = id.as_local().and_then(|dst_id| {
item.item_id.expect_def_id().as_local().map(|src_id| (src_id, dst_id))
}) {
if self.cx.tcx.effective_visibilities(()).is_exported(src_id)
&& !self.cx.tcx.effective_visibilities(()).is_exported(dst_id)
{
Expand Down
Loading

0 comments on commit eee6648

Please sign in to comment.