Skip to content

Replace def_id_no_primitives with def_id #92342

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.

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

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
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
26 changes: 12 additions & 14 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::cell::RefCell;
use std::default::Default;
use std::hash::Hash;
use std::hash::{Hash, Hasher};
use std::lazy::SyncOnceCell as OnceCell;
use std::path::PathBuf;
use std::rc::Rc;
@@ -904,7 +904,7 @@ impl<I: Iterator<Item = ast::NestedMetaItem> + IntoIterator<Item = ast::NestedMe
/// Included files are kept separate from inline doc comments so that proper line-number
/// information can be given when a doctest fails. Sugared doc comments and "raw" doc comments are
/// kept separate because of issue #42760.
#[derive(Clone, PartialEq, Eq, Debug)]
#[derive(Clone, PartialEq, Eq, Debug, Hash)]
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

crate struct DocFragment {
crate span: rustc_span::Span,
/// The module this doc-comment came from.
@@ -1140,6 +1140,15 @@ impl PartialEq for Attributes {

impl Eq for Attributes {}

impl Hash for Attributes {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

fn hash<H: Hasher>(&self, hasher: &mut H) {
self.doc_strings.hash(hasher);
for attr in &self.other_attrs {
attr.id.hash(hasher);
}
}
}

#[derive(Clone, PartialEq, Eq, Debug, Hash)]
crate enum GenericBound {
TraitBound(PolyTrait, hir::TraitBoundModifier),
@@ -1554,23 +1563,12 @@ impl Type {

/// Use this method to get the [DefId] of a [clean] AST node, including [PrimitiveType]s.
///
/// See [`Self::def_id_no_primitives`] for more.
/// See [`Self::def_id`] for more.
///
/// [clean]: crate::clean
crate fn def_id(&self, cache: &Cache) -> Option<DefId> {
self.inner_def_id(Some(cache))
}

/// Use this method to get the [`DefId`] of a [`clean`] AST node.
/// This will return [`None`] when called on a primitive [`clean::Type`].
/// Use [`Self::def_id`] if you want to include primitives.
///
/// [`clean`]: crate::clean
/// [`clean::Type`]: crate::clean::Type
// FIXME: get rid of this function and always use `def_id`
crate fn def_id_no_primitives(&self) -> Option<DefId> {
self.inner_def_id(None)
}
}

/// A primitive (aka, builtin) type.
15 changes: 5 additions & 10 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
@@ -261,7 +261,7 @@ crate fn get_real_types<'tcx>(
tcx: TyCtxt<'_>,
ty: Type,
mut generics: Vec<TypeWithKind>,
_cache: &Cache,
cache: &Cache,
) {
let is_full_generic = ty.is_full_generic();

@@ -320,7 +320,7 @@ crate fn get_real_types<'tcx>(
// We remove the name of the full generic because we have no use for it.
index_ty.name = Some(String::new());
res.push(TypeWithKind::from((index_ty, ItemType::Generic)));
} else if let Some(kind) = ty.def_id_no_primitives().map(|did| tcx.def_kind(did).into()) {
} else if let Some(kind) = ty.def_id(cache).map(|did| tcx.def_kind(did).into()) {
res.push(TypeWithKind::from((index_ty, kind)));
} else if ty.is_primitive() {
// This is a primitive, let's store it as such.
@@ -338,9 +338,7 @@ crate fn get_real_types<'tcx>(
if let Type::Generic(arg_s) = *arg {
// First we check if the bounds are in a `where` predicate...
if let Some(where_pred) = generics.where_predicates.iter().find(|g| match g {
WherePredicate::BoundPredicate { ty, .. } => {
ty.def_id_no_primitives() == arg.def_id_no_primitives()
}
WherePredicate::BoundPredicate { ty, .. } => ty.def_id(cache) == arg.def_id(cache),
_ => false,
}) {
let mut ty_generics = Vec::new();
@@ -413,8 +411,7 @@ crate fn get_all_types<'tcx>(
if !args.is_empty() {
all_types.extend(args);
} else {
if let Some(kind) = arg.type_.def_id_no_primitives().map(|did| tcx.def_kind(did).into())
{
if let Some(kind) = arg.type_.def_id(cache).map(|did| tcx.def_kind(did).into()) {
all_types.push(TypeWithKind::from((get_index_type(&arg.type_, vec![]), kind)));
}
}
@@ -425,9 +422,7 @@ crate fn get_all_types<'tcx>(
FnRetTy::Return(ref return_type) => {
get_real_types(generics, return_type, tcx, 0, &mut ret_types, cache);
if ret_types.is_empty() {
if let Some(kind) =
return_type.def_id_no_primitives().map(|did| tcx.def_kind(did).into())
{
if let Some(kind) = return_type.def_id(cache).map(|did| tcx.def_kind(did).into()) {
ret_types.push(TypeWithKind::from((get_index_type(return_type, vec![]), kind)));
}
}
31 changes: 23 additions & 8 deletions src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@ use crate::clean::*;
use crate::core::DocContext;
use crate::visit::DocVisitor;

use crate::formats::cache::Cache;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::def_id::DefId;
use rustc_middle::ty::DefIdTree;
@@ -47,7 +48,7 @@ crate fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> Crate
inline::build_impl(cx, None, def_id, None, &mut new_items);

// FIXME(eddyb) is this `doc(hidden)` check needed?
if !cx.tcx.is_doc_hidden(def_id) {
if !cx.tcx.get_attrs(def_id).lists(sym::doc).has_word(sym::hidden) {
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't have changed.

let impls = get_auto_trait_and_blanket_impls(cx, def_id);
new_items.extend(impls.filter(|i| cx.inlined.insert(i.def_id)));
}
@@ -64,6 +65,7 @@ crate fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> Crate
map: &FxHashMap<DefId, &Type>,
cleaner: &mut BadImplStripper,
type_did: DefId,
c: &Cache,
Copy link
Member

Choose a reason for hiding this comment

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

cx is already passed in the parameters, so no need to add the cache in here. Just call cx.cache.

) {
if let Some(target) = map.get(&type_did) {
debug!("add_deref_target: type {:?}, target {:?}", type_did, target);
@@ -76,7 +78,7 @@ crate fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> Crate
return;
}
cleaner.items.insert(target_did.into());
add_deref_target(cx, map, cleaner, target_did);
add_deref_target(cx, map, cleaner, target_did, c);
}
}
}
@@ -85,7 +87,7 @@ crate fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> Crate
for it in &new_items {
if let ImplItem(Impl { ref for_, ref trait_, ref items, .. }) = *it.kind {
if trait_.as_ref().map(|t| t.def_id()) == cx.tcx.lang_items().deref_trait()
&& cleaner.keep_impl(for_, true)
&& cleaner.keep_impl(for_, true, &cx.cache)
{
let target = items
.iter()
@@ -100,13 +102,19 @@ crate fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> Crate
} else if let Some(did) = target.def_id(&cx.cache) {
cleaner.items.insert(did.into());
}
if let Some(for_did) = for_.def_id_no_primitives() {
if let Some(for_did) = for_.def_id(&cx.cache) {
if type_did_to_deref_target.insert(for_did, target).is_none() {
// Since only the `DefId` portion of the `Type` instances is known to be same for both the
// `Deref` target type and the impl for type positions, this map of types is keyed by
// `DefId` and for convenience uses a special cleaner that accepts `DefId`s directly.
if cleaner.keep_impl_with_def_id(for_did.into()) {
add_deref_target(cx, &type_did_to_deref_target, &mut cleaner, for_did);
add_deref_target(
cx,
&type_did_to_deref_target,
&mut cleaner,
for_did,
&cx.cache,
);
}
}
}
@@ -119,6 +127,7 @@ crate fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> Crate
cleaner.keep_impl(
for_,
trait_.as_ref().map(|t| t.def_id()) == cx.tcx.lang_items().deref_trait(),
&cx.cache,
) || trait_.as_ref().map_or(false, |t| cleaner.keep_impl_with_def_id(t.def_id().into()))
|| kind.is_blanket()
} else {
@@ -176,7 +185,13 @@ impl<'a, 'tcx> DocVisitor for SyntheticImplCollector<'a, 'tcx> {
fn visit_item(&mut self, i: &Item) {
if i.is_struct() || i.is_enum() || i.is_union() {
// FIXME(eddyb) is this `doc(hidden)` check needed?
if !self.cx.tcx.is_doc_hidden(i.def_id.expect_def_id()) {
if !self
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't have changed.

.cx
.tcx
.get_attrs(i.def_id.expect_def_id())
.lists(sym::doc)
.has_word(sym::hidden)
{
self.impls
.extend(get_auto_trait_and_blanket_impls(self.cx, i.def_id.expect_def_id()));
}
@@ -211,13 +226,13 @@ struct BadImplStripper {
}

impl BadImplStripper {
fn keep_impl(&self, ty: &Type, is_deref: bool) -> bool {
fn keep_impl(&self, ty: &Type, is_deref: bool, c: &Cache) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

Please name it cache and not just c.

if let Generic(_) = ty {
// keep impls made on generics
true
} else if let Some(prim) = ty.primitive_type() {
self.prims.contains(&prim)
} else if let Some(did) = ty.def_id_no_primitives() {
} else if let Some(did) = ty.def_id(c) {
is_deref || self.keep_impl_with_def_id(did.into())
} else {
false
4 changes: 2 additions & 2 deletions src/librustdoc/passes/strip_hidden.rs
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ crate const STRIP_HIDDEN: Pass = Pass {
};

/// Strip items marked `#[doc(hidden)]`
crate fn strip_hidden(krate: clean::Crate, _: &mut DocContext<'_>) -> clean::Crate {
crate fn strip_hidden(krate: clean::Crate, cx: &mut DocContext<'_>) -> clean::Crate {
let mut retained = ItemIdSet::default();

// strip all #[doc(hidden)] items
@@ -25,7 +25,7 @@ crate fn strip_hidden(krate: clean::Crate, _: &mut DocContext<'_>) -> clean::Cra
};

// strip all impls referencing stripped items
let mut stripper = ImplStripper { retained: &retained };
let mut stripper = ImplStripper { retained: &retained, cache: &cx.cache };
stripper.fold_crate(krate)
}

2 changes: 1 addition & 1 deletion src/librustdoc/passes/strip_private.rs
Original file line number Diff line number Diff line change
@@ -29,6 +29,6 @@ crate fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) -> clea
}

// strip all impls referencing private items
let mut stripper = ImplStripper { retained: &retained };
let mut stripper = ImplStripper { retained: &retained, cache: &cx.cache };
stripper.fold_crate(krate)
}
6 changes: 4 additions & 2 deletions src/librustdoc/passes/stripper.rs
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ use std::mem;

use crate::clean::{self, Item, ItemIdSet};
use crate::fold::{strip_item, DocFolder};
use crate::formats::cache::Cache;

crate struct Stripper<'a> {
crate retained: &'a mut ItemIdSet,
@@ -119,6 +120,7 @@ impl<'a> DocFolder for Stripper<'a> {
/// This stripper discards all impls which reference stripped items
crate struct ImplStripper<'a> {
crate retained: &'a ItemIdSet,
crate cache: &'a Cache,
}

impl<'a> DocFolder for ImplStripper<'a> {
@@ -128,7 +130,7 @@ impl<'a> DocFolder for ImplStripper<'a> {
if imp.trait_.is_none() && imp.items.is_empty() {
return None;
}
if let Some(did) = imp.for_.def_id_no_primitives() {
if let Some(did) = imp.for_.def_id(&self.cache) {
if did.is_local() && !imp.for_.is_assoc_ty() && !self.retained.contains(&did.into())
{
debug!("ImplStripper: impl item for stripped type; removing");
@@ -143,7 +145,7 @@ impl<'a> DocFolder for ImplStripper<'a> {
}
if let Some(generics) = imp.trait_.as_ref().and_then(|t| t.generics()) {
for typaram in generics {
if let Some(did) = typaram.def_id_no_primitives() {
if let Some(did) = typaram.def_id(&self.cache) {
if did.is_local() && !self.retained.contains(&did.into()) {
debug!(
"ImplStripper: stripped item in trait's generics; removing impl"