Skip to content
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

rustdoc: Simplify modifications of effective visibility table #103010

Merged
merged 3 commits into from
Oct 30, 2022
Merged
Show file tree
Hide file tree
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
48 changes: 16 additions & 32 deletions compiler/rustc_middle/src/middle/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_macros::HashStable;
use rustc_query_system::ich::StableHashingContext;
use rustc_span::def_id::{DefId, LocalDefId};
use std::hash::Hash;
use rustc_span::def_id::LocalDefId;

/// Represents the levels of effective visibility an item can have.
///
Expand Down Expand Up @@ -75,33 +74,33 @@ impl EffectiveVisibility {
}

/// Holds a map of effective visibilities for reachable HIR nodes.
#[derive(Debug, Clone)]
pub struct EffectiveVisibilities<Id = LocalDefId> {
map: FxHashMap<Id, EffectiveVisibility>,
#[derive(Default, Clone, Debug)]
pub struct EffectiveVisibilities {
map: FxHashMap<LocalDefId, EffectiveVisibility>,
}

impl<Id: Hash + Eq + Copy> EffectiveVisibilities<Id> {
pub fn is_public_at_level(&self, id: Id, level: Level) -> bool {
impl EffectiveVisibilities {
pub fn is_public_at_level(&self, id: LocalDefId, level: Level) -> bool {
self.effective_vis(id)
.map_or(false, |effective_vis| effective_vis.is_public_at_level(level))
}

/// See `Level::Reachable`.
pub fn is_reachable(&self, id: Id) -> bool {
pub fn is_reachable(&self, id: LocalDefId) -> bool {
self.is_public_at_level(id, Level::Reachable)
}

/// See `Level::Reexported`.
pub fn is_exported(&self, id: Id) -> bool {
pub fn is_exported(&self, id: LocalDefId) -> bool {
self.is_public_at_level(id, Level::Reexported)
}

/// See `Level::Direct`.
pub fn is_directly_public(&self, id: Id) -> bool {
pub fn is_directly_public(&self, id: LocalDefId) -> bool {
self.is_public_at_level(id, Level::Direct)
}

pub fn public_at_level(&self, id: Id) -> Option<Level> {
pub fn public_at_level(&self, id: LocalDefId) -> Option<Level> {
self.effective_vis(id).and_then(|effective_vis| {
for level in Level::all_levels() {
if effective_vis.is_public_at_level(level) {
Expand All @@ -112,24 +111,17 @@ impl<Id: Hash + Eq + Copy> EffectiveVisibilities<Id> {
})
}

pub fn effective_vis(&self, id: Id) -> Option<&EffectiveVisibility> {
pub fn effective_vis(&self, id: LocalDefId) -> Option<&EffectiveVisibility> {
self.map.get(&id)
}

pub fn iter(&self) -> impl Iterator<Item = (&Id, &EffectiveVisibility)> {
pub fn iter(&self) -> impl Iterator<Item = (&LocalDefId, &EffectiveVisibility)> {
self.map.iter()
}

pub fn map_id<OutId: Hash + Eq + Copy>(
&self,
f: impl Fn(Id) -> OutId,
) -> EffectiveVisibilities<OutId> {
EffectiveVisibilities { map: self.map.iter().map(|(k, v)| (f(*k), *v)).collect() }
}

pub fn set_public_at_level(
&mut self,
id: Id,
id: LocalDefId,
default_vis: impl FnOnce() -> Visibility,
level: Level,
) {
Expand All @@ -144,23 +136,21 @@ impl<Id: Hash + Eq + Copy> EffectiveVisibilities<Id> {
}
self.map.insert(id, effective_vis);
}
}

impl<Id: Hash + Eq + Copy + Into<DefId>> EffectiveVisibilities<Id> {
// `parent_id` is not necessarily a parent in source code tree,
// it is the node from which the maximum effective visibility is inherited.
pub fn update(
&mut self,
id: Id,
id: LocalDefId,
nominal_vis: Visibility,
default_vis: impl FnOnce() -> Visibility,
parent_id: Id,
parent_id: LocalDefId,
level: Level,
tree: impl DefIdTree,
) -> bool {
let mut changed = false;
let mut current_effective_vis = self.effective_vis(id).copied().unwrap_or_else(|| {
if id.into().is_crate_root() {
if id.is_top_level_module() {
EffectiveVisibility::from_vis(Visibility::Public)
} else {
EffectiveVisibility::from_vis(default_vis())
Expand Down Expand Up @@ -204,12 +194,6 @@ impl<Id: Hash + Eq + Copy + Into<DefId>> EffectiveVisibilities<Id> {
}
}

impl<Id> Default for EffectiveVisibilities<Id> {
fn default() -> Self {
EffectiveVisibilities { map: Default::default() }
}
}

impl<'a> HashStable<StableHashingContext<'a>> for EffectiveVisibilities {
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
let EffectiveVisibilities { ref map } = *self;
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 @@ -20,7 +20,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
trace!("get_blanket_impls({:?})", ty);
let mut impls = Vec::new();
for trait_def_id in cx.tcx.all_traits() {
if !cx.cache.effective_visibilities.is_directly_public(trait_def_id)
if !cx.cache.effective_visibilities.is_directly_public(cx.tcx, trait_def_id)
|| cx.generated_synthetics.get(&(ty.0, trait_def_id)).is_some()
{
continue;
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ pub(crate) fn build_impl(
if !did.is_local() {
if let Some(traitref) = associated_trait {
let did = traitref.def_id;
if !cx.cache.effective_visibilities.is_directly_public(did) {
if !cx.cache.effective_visibilities.is_directly_public(tcx, did) {
return;
}

Expand Down Expand Up @@ -403,7 +403,7 @@ pub(crate) fn build_impl(
// reachable in rustdoc generated documentation
if !did.is_local() {
if let Some(did) = for_.def_id(&cx.cache) {
if !cx.cache.effective_visibilities.is_directly_public(did) {
if !cx.cache.effective_visibilities.is_directly_public(tcx, did) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1431,7 +1431,7 @@ fn maybe_expand_private_type_alias<'tcx>(
let Res::Def(DefKind::TyAlias, def_id) = path.res else { return None };
// Substitute private type aliases
let def_id = def_id.as_local()?;
let alias = if !cx.cache.effective_visibilities.is_exported(def_id.to_def_id()) {
let alias = if !cx.cache.effective_visibilities.is_exported(cx.tcx, def_id.to_def_id()) {
&cx.tcx.hir().expect_item(def_id).kind
} else {
return None;
Expand Down
3 changes: 1 addition & 2 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use crate::clean::{
};
use crate::core::DocContext;
use crate::formats::item_type::ItemType;
use crate::visit_lib::LibEmbargoVisitor;

use rustc_ast as ast;
use rustc_ast::tokenstream::TokenTree;
Expand All @@ -32,7 +31,7 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate {

for &cnum in cx.tcx.crates(()) {
// Analyze doc-reachability for extern items
LibEmbargoVisitor::new(cx).visit_lib(cnum);
crate::visit_lib::lib_embargo_visit_item(cx, cnum.as_def_id());
}

// Clean the crate, translating the entire librustc_ast AST to one that is
Expand Down
3 changes: 1 addition & 2 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ pub(crate) fn run_global_ctxt(

let auto_traits =
tcx.all_traits().filter(|&trait_def_id| tcx.trait_is_auto(trait_def_id)).collect();
let effective_visibilities = tcx.effective_visibilities(()).map_id(Into::into);

let mut ctxt = DocContext {
tcx,
Expand All @@ -361,7 +360,7 @@ pub(crate) fn run_global_ctxt(
impl_trait_bounds: Default::default(),
generated_synthetics: Default::default(),
auto_traits,
cache: Cache::new(effective_visibilities, render_options.document_private),
cache: Cache::new(render_options.document_private),
inlined: FxHashSet::default(),
output_format,
render_options,
Expand Down
13 changes: 5 additions & 8 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::mem;

use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::def_id::{CrateNum, DefId};
use rustc_middle::middle::privacy::EffectiveVisibilities;
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::Symbol;

Expand All @@ -15,6 +14,7 @@ use crate::html::format::join_with_double_colon;
use crate::html::markdown::short_markdown_summary;
use crate::html::render::search_index::get_function_type_for_search;
use crate::html::render::IndexItem;
use crate::visit_lib::RustdocEffectiveVisibilities;

/// This cache is used to store information about the [`clean::Crate`] being
/// rendered in order to provide more useful documentation. This contains
Expand Down Expand Up @@ -78,7 +78,7 @@ pub(crate) struct Cache {
// Note that external items for which `doc(hidden)` applies to are shown as
// non-reachable while local items aren't. This is because we're reusing
// the effective visibilities from the privacy check pass.
pub(crate) effective_visibilities: EffectiveVisibilities<DefId>,
pub(crate) effective_visibilities: RustdocEffectiveVisibilities,

/// The version of the crate being documented, if given from the `--crate-version` flag.
pub(crate) crate_version: Option<String>,
Expand Down Expand Up @@ -132,11 +132,8 @@ struct CacheBuilder<'a, 'tcx> {
}

impl Cache {
pub(crate) fn new(
effective_visibilities: EffectiveVisibilities<DefId>,
document_private: bool,
) -> Self {
Cache { effective_visibilities, document_private, ..Cache::default() }
pub(crate) fn new(document_private: bool) -> Self {
Cache { document_private, ..Cache::default() }
}

/// Populates the `Cache` with more data. The returned `Crate` will be missing some data that was
Expand Down Expand Up @@ -387,7 +384,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
|| self
.cache
.effective_visibilities
.is_directly_public(item.item_id.expect_def_id())
.is_directly_public(self.tcx, item.item_id.expect_def_id())
{
self.cache.paths.insert(
item.item_id.expect_def_id(),
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ pub(crate) fn href_with_root_path(
}

if !did.is_local()
&& !cache.effective_visibilities.is_directly_public(did)
&& !cache.effective_visibilities.is_directly_public(tcx, did)
&& !cache.document_private
&& !cache.primitive_locations.values().any(|&id| id == did)
{
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/passes/check_doc_test_visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl crate::doctest::Tester for Tests {
}

pub(crate) fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> bool {
if !cx.cache.effective_visibilities.is_directly_public(item.item_id.expect_def_id())
if !cx.cache.effective_visibilities.is_directly_public(cx.tcx, item.item_id.expect_def_id())
|| matches!(
*item.kind,
clean::StructFieldItem(_)
Expand Down Expand Up @@ -130,7 +130,7 @@ pub(crate) fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item
);
}
} else if tests.found_tests > 0
&& !cx.cache.effective_visibilities.is_exported(item.item_id.expect_def_id())
&& !cx.cache.effective_visibilities.is_exported(cx.tcx, item.item_id.expect_def_id())
{
cx.tcx.struct_span_lint_hir(
crate::lint::PRIVATE_DOC_TESTS,
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/passes/strip_hidden.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub(crate) fn strip_hidden(krate: clean::Crate, cx: &mut DocContext<'_>) -> clea

// strip all impls referencing stripped items
let mut stripper = ImplStripper {
tcx: cx.tcx,
retained: &retained,
cache: &cx.cache,
is_json_output,
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/passes/strip_private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub(crate) fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) ->
// strip all private items
{
let mut stripper = Stripper {
tcx: cx.tcx,
retained: &mut retained,
effective_visibilities: &cx.cache.effective_visibilities,
update_retained: true,
Expand All @@ -32,6 +33,7 @@ pub(crate) fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) ->

// strip all impls referencing private items
let mut stripper = ImplStripper {
tcx: cx.tcx,
retained: &retained,
cache: &cx.cache,
is_json_output,
Expand Down
34 changes: 22 additions & 12 deletions src/librustdoc/passes/stripper.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
//! A collection of utility functions for the `strip_*` passes.
use rustc_hir::def_id::DefId;
use rustc_middle::middle::privacy::EffectiveVisibilities;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::sym;

use std::mem;

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

pub(crate) struct Stripper<'a> {
pub(crate) struct Stripper<'a, 'tcx> {
pub(crate) tcx: TyCtxt<'tcx>,
pub(crate) retained: &'a mut ItemIdSet,
pub(crate) effective_visibilities: &'a EffectiveVisibilities<DefId>,
pub(crate) effective_visibilities: &'a RustdocEffectiveVisibilities,
pub(crate) update_retained: bool,
pub(crate) is_json_output: bool,
}
Expand All @@ -21,18 +23,19 @@ pub(crate) struct Stripper<'a> {
// are in the public API, which is not enough.
#[inline]
fn is_item_reachable(
tcx: TyCtxt<'_>,
is_json_output: bool,
effective_visibilities: &EffectiveVisibilities<DefId>,
effective_visibilities: &RustdocEffectiveVisibilities,
item_id: ItemId,
) -> bool {
if is_json_output {
effective_visibilities.is_reachable(item_id.expect_def_id())
effective_visibilities.is_reachable(tcx, item_id.expect_def_id())
} else {
effective_visibilities.is_exported(item_id.expect_def_id())
effective_visibilities.is_exported(tcx, item_id.expect_def_id())
}
}

impl<'a> DocFolder for Stripper<'a> {
impl<'a> DocFolder for Stripper<'a, '_> {
fn fold_item(&mut self, i: Item) -> Option<Item> {
match *i.kind {
clean::StrippedItem(..) => {
Expand Down Expand Up @@ -66,7 +69,12 @@ impl<'a> DocFolder for Stripper<'a> {
| clean::ForeignTypeItem => {
let item_id = i.item_id;
if item_id.is_local()
&& !is_item_reachable(self.is_json_output, self.effective_visibilities, item_id)
&& !is_item_reachable(
self.tcx,
self.is_json_output,
self.effective_visibilities,
item_id,
)
{
debug!("Stripper: stripping {:?} {:?}", i.type_(), i.name);
return None;
Expand Down Expand Up @@ -146,30 +154,31 @@ impl<'a> DocFolder for Stripper<'a> {
}

/// This stripper discards all impls which reference stripped items
pub(crate) struct ImplStripper<'a> {
pub(crate) struct ImplStripper<'a, 'tcx> {
pub(crate) tcx: TyCtxt<'tcx>,
pub(crate) retained: &'a ItemIdSet,
pub(crate) cache: &'a Cache,
pub(crate) is_json_output: bool,
pub(crate) document_private: bool,
}

impl<'a> ImplStripper<'a> {
impl<'a> ImplStripper<'a, '_> {
#[inline]
fn should_keep_impl(&self, item: &Item, for_def_id: DefId) -> bool {
if !for_def_id.is_local() || self.retained.contains(&for_def_id.into()) {
true
} else if self.is_json_output {
// If the "for" item is exported and the impl block isn't `#[doc(hidden)]`, then we
// need to keep it.
self.cache.effective_visibilities.is_exported(for_def_id)
self.cache.effective_visibilities.is_exported(self.tcx, for_def_id)
&& !item.attrs.lists(sym::doc).has_word(sym::hidden)
} else {
false
}
}
}

impl<'a> DocFolder for ImplStripper<'a> {
impl<'a> DocFolder for ImplStripper<'a, '_> {
fn fold_item(&mut self, i: Item) -> Option<Item> {
if let clean::ImplItem(ref imp) = *i.kind {
// Impl blocks can be skipped if they are: empty; not a trait impl; and have no
Expand All @@ -185,6 +194,7 @@ impl<'a> DocFolder for ImplStripper<'a> {
let item_id = i.item_id;
item_id.is_local()
&& !is_item_reachable(
self.tcx,
self.is_json_output,
&self.cache.effective_visibilities,
item_id,
Expand Down
Loading