Skip to content

Commit

Permalink
Auto merge of rust-lang#103690 - GuillaumeGomez:visibility-on-demand,…
Browse files Browse the repository at this point in the history
… r=notriddle

Make rustdoc Item::visibility computed on-demand

This is a take-over of rust-lang#91408.

Helps with rust-lang#90852 (needs to use `ty::Visibility` directly too).

cc `@camelid`
r? `@notriddle`
  • Loading branch information
bors committed Nov 2, 2022
2 parents 822f8c2 + 5515e2c commit c0a7612
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 145 deletions.
2 changes: 1 addition & 1 deletion src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -130,6 +129,7 @@ where
kind: ImplKind::Auto,
}))),
cfg: None,
inline_stmt_id: None,
})
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/blanket_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -128,6 +127,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
))),
}))),
cfg: None,
inline_stmt_id: None,
});
}
}
Expand Down
29 changes: 7 additions & 22 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -559,7 +545,6 @@ pub(crate) fn build_impl(
},
})),
Box::new(merged_attrs),
cx,
cfg,
));
}
Expand Down Expand Up @@ -607,7 +592,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 {
Expand All @@ -626,6 +610,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)
Expand Down
64 changes: 9 additions & 55 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

Expand Down Expand Up @@ -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)
})
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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>(
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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();

Expand All @@ -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),
Expand All @@ -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),
}]
}

Expand Down
84 changes: 66 additions & 18 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>>,
}

Expand All @@ -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);
Expand All @@ -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))
Expand Down Expand Up @@ -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),
)
}
Expand All @@ -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
Expand Down Expand Up @@ -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)]
Expand Down
Loading

0 comments on commit c0a7612

Please sign in to comment.