Skip to content

Make rustdoc Item::visibility computed on-demand #103690

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

Merged
merged 1 commit into from
Nov 2, 2022
Merged
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
2 changes: 1 addition & 1 deletion src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
@@ -118,7 +118,6 @@ where
Some(Item {
name: None,
attrs: Default::default(),
visibility: Inherited,
item_id: ItemId::Auto { trait_: trait_def_id, for_: item_def_id },
kind: Box::new(ImplItem(Box::new(Impl {
unsafety: hir::Unsafety::Normal,
@@ -130,6 +129,7 @@ where
kind: ImplKind::Auto,
}))),
cfg: None,
inline_stmt_id: None,
})
}

2 changes: 1 addition & 1 deletion src/librustdoc/clean/blanket_impl.rs
Original file line number Diff line number Diff line change
@@ -97,7 +97,6 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
impls.push(Item {
name: None,
attrs: Default::default(),
visibility: Inherited,
item_id: ItemId::Blanket { impl_id: impl_def_id, for_: item_def_id },
kind: Box::new(ImplItem(Box::new(Impl {
unsafety: hir::Unsafety::Normal,
@@ -128,6 +127,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
))),
}))),
cfg: None,
inline_stmt_id: None,
});
}
}
29 changes: 7 additions & 22 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ use crate::clean::{
self, clean_fn_decl_from_did_and_sig, clean_generics, clean_impl_item, clean_middle_assoc_item,
clean_middle_field, clean_middle_ty, clean_trait_ref_with_bindings, clean_ty,
clean_ty_generics, clean_variant_def, clean_visibility, utils, Attributes, AttributesExt,
ImplKind, ItemId, Type, Visibility,
ImplKind, ItemId, Type,
};
use crate::core::DocContext;
use crate::formats::item_type::ItemType;
@@ -152,18 +152,10 @@ pub(crate) fn try_inline(

let (attrs, cfg) = merge_attrs(cx, Some(parent_module), load_attrs(cx, did), attrs);
cx.inlined.insert(did.into());
let mut item = clean::Item::from_def_id_and_attrs_and_parts(
did,
Some(name),
kind,
Box::new(attrs),
cx,
cfg,
);
if let Some(import_def_id) = import_def_id {
// The visibility needs to reflect the one from the reexport and not from the "source" DefId.
item.visibility = clean_visibility(cx.tcx.visibility(import_def_id));
}
let mut item =
clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, Box::new(attrs), cfg);
// The visibility needs to reflect the one from the reexport and not from the "source" DefId.
item.inline_stmt_id = import_def_id;
ret.push(item);
Some(ret)
}
@@ -239,13 +231,7 @@ pub(crate) fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean
.tcx
.associated_items(did)
.in_definition_order()
.map(|item| {
// When building an external trait, the cleaned trait will have all items public,
// which causes methods to have a `pub` prefix, which is invalid since items in traits
// can not have a visibility prefix. Thus we override the visibility here manually.
// See https://github.com/rust-lang/rust/issues/81274
clean::Item { visibility: Visibility::Inherited, ..clean_middle_assoc_item(item, cx) }
})
.map(|item| clean_middle_assoc_item(item, cx))
.collect();

let predicates = cx.tcx.predicates_of(did);
@@ -544,7 +530,6 @@ pub(crate) fn build_impl(
},
})),
Box::new(merged_attrs),
cx,
cfg,
));
}
@@ -592,7 +577,6 @@ fn build_module_items(
name: None,
attrs: Box::new(clean::Attributes::default()),
item_id: ItemId::Primitive(prim_ty, did.krate),
visibility: clean::Public,
kind: Box::new(clean::ImportItem(clean::Import::new_simple(
item.ident.name,
clean::ImportSource {
@@ -611,6 +595,7 @@ fn build_module_items(
true,
))),
cfg: None,
inline_stmt_id: None,
});
} else if let Some(i) = try_inline(cx, did, None, res, item.ident.name, None, visited) {
items.extend(i)
64 changes: 9 additions & 55 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
@@ -1083,10 +1083,7 @@ fn clean_trait_item<'tcx>(trait_item: &hir::TraitItem<'tcx>, cx: &mut DocContext
TyAssocTypeItem(Box::new(generics), bounds)
}
};
let what_rustc_thinks =
Item::from_def_id_and_parts(local_did, Some(trait_item.ident.name), inner, cx);
// Trait items always inherit the trait's visibility -- we don't want to show `pub`.
Item { visibility: Inherited, ..what_rustc_thinks }
Item::from_def_id_and_parts(local_did, Some(trait_item.ident.name), inner, cx)
})
}

@@ -1117,18 +1114,7 @@ pub(crate) fn clean_impl_item<'tcx>(
}
};

let mut what_rustc_thinks =
Item::from_def_id_and_parts(local_did, Some(impl_.ident.name), inner, cx);

let impl_ref = cx.tcx.impl_trait_ref(cx.tcx.local_parent(impl_.owner_id.def_id));

// Trait impl items always inherit the impl's visibility --
// we don't want to show `pub`.
if impl_ref.is_some() {
what_rustc_thinks.visibility = Inherited;
}

what_rustc_thinks
Item::from_def_id_and_parts(local_did, Some(impl_.ident.name), inner, cx)
})
}

@@ -1340,18 +1326,7 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
}
};

let mut what_rustc_thinks =
Item::from_def_id_and_parts(assoc_item.def_id, Some(assoc_item.name), kind, cx);

let impl_ref = tcx.impl_trait_ref(tcx.parent(assoc_item.def_id));

// Trait impl items always inherit the impl's visibility --
// we don't want to show `pub`.
if impl_ref.is_some() {
what_rustc_thinks.visibility = Visibility::Inherited;
}

what_rustc_thinks
Item::from_def_id_and_parts(assoc_item.def_id, Some(assoc_item.name), kind, cx)
}

fn clean_qpath<'tcx>(hir_ty: &hir::Ty<'tcx>, cx: &mut DocContext<'tcx>) -> Type {
@@ -1821,23 +1796,7 @@ pub(crate) fn clean_field_with_def_id(
ty: Type,
cx: &mut DocContext<'_>,
) -> Item {
let what_rustc_thinks =
Item::from_def_id_and_parts(def_id, Some(name), StructFieldItem(ty), cx);
if is_field_vis_inherited(cx.tcx, def_id) {
// Variant fields inherit their enum's visibility.
Item { visibility: Visibility::Inherited, ..what_rustc_thinks }
} else {
what_rustc_thinks
}
}

fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
let parent = tcx.parent(def_id);
match tcx.def_kind(parent) {
DefKind::Struct | DefKind::Union => false,
DefKind::Variant => true,
parent_kind => panic!("unexpected parent kind: {:?}", parent_kind),
}
Item::from_def_id_and_parts(def_id, Some(name), StructFieldItem(ty), cx)
}

pub(crate) fn clean_visibility(vis: ty::Visibility<DefId>) -> Visibility {
@@ -1861,10 +1820,7 @@ pub(crate) fn clean_variant_def<'tcx>(variant: &ty::VariantDef, cx: &mut DocCont
fields: variant.fields.iter().map(|field| clean_middle_field(field, cx)).collect(),
}),
};
let what_rustc_thinks =
Item::from_def_id_and_parts(variant.def_id, Some(variant.name), VariantItem(kind), cx);
// don't show `pub` for variants, which always inherit visibility
Item { visibility: Inherited, ..what_rustc_thinks }
Item::from_def_id_and_parts(variant.def_id, Some(variant.name), VariantItem(kind), cx)
}

fn clean_variant_data<'tcx>(
@@ -2038,10 +1994,7 @@ 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));
let what_rustc_thinks =
Item::from_hir_id_and_parts(variant.id, Some(variant.ident.name), kind, cx);
// don't show `pub` for variants, which are always public
Item { visibility: Inherited, ..what_rustc_thinks }
Item::from_hir_id_and_parts(variant.id, Some(variant.ident.name), kind, cx)
}

fn clean_impl<'tcx>(
@@ -2114,6 +2067,7 @@ fn clean_extern_crate<'tcx>(
}
});

let krate_owner_def_id = krate.owner_id.to_def_id();
if please_inline {
let mut visited = FxHashSet::default();

@@ -2122,7 +2076,7 @@ fn clean_extern_crate<'tcx>(
if let Some(items) = inline::try_inline(
cx,
cx.tcx.parent_module(krate.hir_id()).to_def_id(),
Some(krate.owner_id.to_def_id()),
Some(krate_owner_def_id),
res,
name,
Some(attrs),
@@ -2137,9 +2091,9 @@ fn clean_extern_crate<'tcx>(
name: Some(name),
attrs: Box::new(Attributes::from_ast(attrs)),
item_id: crate_def_id.into(),
visibility: clean_visibility(ty_vis),
kind: Box::new(ExternCrateItem { src: orig_name }),
cfg: attrs.cfg(cx.tcx, &cx.cache.hidden_cfg),
inline_stmt_id: Some(krate_owner_def_id),
}]
}

84 changes: 66 additions & 18 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@ use rustc_hir::{BodyId, Mutability};
use rustc_hir_analysis::check::intrinsic::intrinsic_operation_unsafety;
use rustc_index::vec::IndexVec;
use rustc_middle::ty::fast_reject::SimplifiedType;
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::{self, DefIdTree, TyCtxt};
use rustc_session::Session;
use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::DUMMY_SP;
@@ -348,12 +348,12 @@ pub(crate) struct Item {
/// Optional because not every item has a name, e.g. impls.
pub(crate) name: Option<Symbol>,
pub(crate) attrs: Box<Attributes>,
pub(crate) visibility: Visibility,
/// Information about this item that is specific to what kind of item it is.
/// E.g., struct vs enum vs function.
pub(crate) kind: Box<ItemKind>,
pub(crate) item_id: ItemId,

/// This is the `DefId` of the `use` statement if the item was inlined.
pub(crate) inline_stmt_id: Option<DefId>,
pub(crate) cfg: Option<Arc<Cfg>>,
}

@@ -364,9 +364,7 @@ impl fmt::Debug for Item {
let alternate = f.alternate();
// hand-picked fields that don't bloat the logs too much
let mut fmt = f.debug_struct("Item");
fmt.field("name", &self.name)
.field("visibility", &self.visibility)
.field("item_id", &self.item_id);
fmt.field("name", &self.name).field("item_id", &self.item_id);
// allow printing the full item if someone really wants to
if alternate {
fmt.field("attrs", &self.attrs).field("kind", &self.kind).field("cfg", &self.cfg);
@@ -388,6 +386,15 @@ pub(crate) fn rustc_span(def_id: DefId, tcx: TyCtxt<'_>) -> Span {
))
}

fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
let parent = tcx.parent(def_id);
match tcx.def_kind(parent) {
DefKind::Struct | DefKind::Union => false,
DefKind::Variant => true,
parent_kind => panic!("unexpected parent kind: {:?}", parent_kind),
}
}

impl Item {
pub(crate) fn stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<Stability> {
self.item_id.as_def_id().and_then(|did| tcx.lookup_stability(did))
@@ -462,7 +469,6 @@ impl Item {
name,
kind,
Box::new(Attributes::from_ast(ast_attrs)),
cx,
ast_attrs.cfg(cx.tcx, &cx.cache.hidden_cfg),
)
}
@@ -472,21 +478,18 @@ impl Item {
name: Option<Symbol>,
kind: ItemKind,
attrs: Box<Attributes>,
cx: &mut DocContext<'_>,
cfg: Option<Arc<Cfg>>,
) -> Item {
trace!("name={:?}, def_id={:?} cfg={:?}", name, def_id, cfg);

// Primitives and Keywords are written in the source code as private modules.
// The modules need to be private so that nobody actually uses them, but the
// keywords and primitives that they are documenting are public.
let visibility = if matches!(&kind, ItemKind::KeywordItem | ItemKind::PrimitiveItem(..)) {
Visibility::Public
} else {
clean_visibility(cx.tcx.visibility(def_id))
};

Item { item_id: def_id.into(), kind: Box::new(kind), name, attrs, visibility, cfg }
Item {
item_id: def_id.into(),
kind: Box::new(kind),
name,
attrs,
cfg,
inline_stmt_id: None,
}
}

/// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
@@ -702,6 +705,51 @@ impl Item {
};
Some(header)
}

pub(crate) fn visibility(&self, tcx: TyCtxt<'_>) -> Visibility {
let def_id = match self.item_id {
// Anything but DefId *shouldn't* matter, but return a reasonable value anyway.
ItemId::Auto { .. } | ItemId::Blanket { .. } => return Visibility::Inherited,
// Primitives and Keywords are written in the source code as private modules.
// The modules need to be private so that nobody actually uses them, but the
// keywords and primitives that they are documenting are public.
ItemId::Primitive(..) => return Visibility::Public,
ItemId::DefId(def_id) => def_id,
};

match *self.kind {
// Explication on `ItemId::Primitive` just above.
ItemKind::KeywordItem | ItemKind::PrimitiveItem(_) => return Visibility::Public,
// Variant fields inherit their enum's visibility.
StructFieldItem(..) if is_field_vis_inherited(tcx, def_id) => {
return Visibility::Inherited;
}
// Variants always inherit visibility
VariantItem(..) => return Visibility::Inherited,
// Trait items inherit the trait's visibility
AssocConstItem(..) | TyAssocConstItem(..) | AssocTypeItem(..) | TyAssocTypeItem(..)
| TyMethodItem(..) | MethodItem(..) => {
let assoc_item = tcx.associated_item(def_id);
let is_trait_item = match assoc_item.container {
ty::TraitContainer => true,
ty::ImplContainer => {
// Trait impl items always inherit the impl's visibility --
// we don't want to show `pub`.
tcx.impl_trait_ref(tcx.parent(assoc_item.def_id)).is_some()
}
};
if is_trait_item {
return Visibility::Inherited;
}
}
_ => {}
}
let def_id = match self.inline_stmt_id {
Some(inlined) => inlined,
None => def_id,
};
clean_visibility(tcx.visibility(def_id))
}
}

#[derive(Clone, Debug)]
Loading