Skip to content

Commit

Permalink
Rollup merge of rust-lang#88447 - inquisitivecrystal:rustdoc-vis, r=j…
Browse files Browse the repository at this point in the history
…yn514

Use computed visibility in rustdoc

This PR changes `librustdoc` to use computed visibility instead of syntactic visibility. It was initially part of rust-lang#88019, but was separated due to concerns that it might cause a regression somewhere we couldn't predict.

r? `@jyn514`
cc `@cjgillot` `@petrochenkov`
  • Loading branch information
matthiaskrgr authored Nov 10, 2021
2 parents 68ca579 + 6622376 commit a8dff34
Show file tree
Hide file tree
Showing 15 changed files with 38 additions and 38 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
let ctor_res =
Res::Def(DefKind::Ctor(CtorOf::Variant, ctor_kind), ctor_def_id);
let mut vis = self.get_visibility(ctor_def_id.index);
if ctor_def_id == def_id && vis == ty::Visibility::Public {
if ctor_def_id == def_id && vis.is_public() {
// For non-exhaustive variants lower the constructor visibility to
// within the crate. We only need this for fictive constructors,
// for other constructors correct visibilities
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ pub fn provide(providers: &mut Providers) {
}

let mut add_child = |bfs_queue: &mut VecDeque<_>, child: &Export, parent: DefId| {
if child.vis != ty::Visibility::Public {
if !child.vis.is_public() {
return;
}

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,10 @@ impl Visibility {
Visibility::Invisible => false,
}
}

pub fn is_public(self) -> bool {
matches!(self, Visibility::Public)
}
}

/// The crate variances map is computed during typeck and contains the
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2404,7 +2404,7 @@ fn for_each_def(tcx: TyCtxt<'_>, mut collect_fn: impl for<'b> FnMut(&'b Ident, N
// Iterate external crate defs but be mindful about visibility
while let Some(def) = queue.pop() {
for child in tcx.item_children(def).iter() {
if child.vis != ty::Visibility::Public {
if !child.vis.is_public() {
continue;
}

Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ impl EmbargoVisitor<'tcx> {
module: LocalDefId,
) {
let level = Some(AccessLevel::Reachable);
if let ty::Visibility::Public = vis {
if vis.is_public() {
self.update(def_id, level);
}
match def_kind {
Expand Down Expand Up @@ -580,7 +580,7 @@ impl EmbargoVisitor<'tcx> {

DefKind::Struct | DefKind::Union => {
// While structs and unions have type privacy, their fields do not.
if let ty::Visibility::Public = vis {
if vis.is_public() {
let item =
self.tcx.hir().expect_item(self.tcx.hir().local_def_id_to_hir_id(def_id));
if let hir::ItemKind::Struct(ref struct_def, _)
Expand Down Expand Up @@ -933,7 +933,7 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {
let def_id = self.tcx.hir().local_def_id(id);
if let Some(exports) = self.tcx.module_exports(def_id) {
for export in exports.iter() {
if export.vis == ty::Visibility::Public {
if export.vis.is_public() {
if let Some(def_id) = export.res.opt_def_id() {
if let Some(def_id) = def_id.as_local() {
self.update(def_id, Some(AccessLevel::Exported));
Expand Down Expand Up @@ -1918,8 +1918,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'tcx> {
/// 1. It's contained within a public type
/// 2. It comes from a private crate
fn leaks_private_dep(&self, item_id: DefId) -> bool {
let ret = self.required_visibility == ty::Visibility::Public
&& self.tcx.is_private_dep(item_id.krate);
let ret = self.required_visibility.is_public() && self.tcx.is_private_dep(item_id.krate);

tracing::debug!("leaks_private_dep(item_id={:?})={}", item_id, ret);
ret
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use rustc_ast::visit::{self, Visitor};
use rustc_ast_lowering::ResolverAstLowering;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::pluralize;
use rustc_middle::ty;
use rustc_session::lint::builtin::{MACRO_USE_EXTERN_CRATE, UNUSED_IMPORTS};
use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_span::{MultiSpan, Span, DUMMY_SP};
Expand Down Expand Up @@ -228,7 +227,7 @@ impl Resolver<'_> {
for import in self.potentially_unused_imports.iter() {
match import.kind {
_ if import.used.get()
|| import.vis.get() == ty::Visibility::Public
|| import.vis.get().is_public()
|| import.span.is_dummy() =>
{
if let ImportKind::MacroUse = import.kind {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind};
use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::PrimTy;
use rustc_middle::bug;
use rustc_middle::ty::{self, DefIdTree};
use rustc_middle::ty::DefIdTree;
use rustc_session::Session;
use rustc_span::hygiene::MacroKind;
use rustc_span::lev_distance::find_best_match_for_name;
Expand Down Expand Up @@ -1308,7 +1308,7 @@ impl<'a> Resolver<'a> {
);
let def_span = self.session.source_map().guess_head_span(binding.span);
let mut note_span = MultiSpan::from_span(def_span);
if !first && binding.vis == ty::Visibility::Public {
if !first && binding.vis.is_public() {
note_span.push_span_label(def_span, "consider importing it directly".into());
}
err.span_note(note_span, &msg);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ fn pub_use_of_private_extern_crate_hack(import: &Import<'_>, binding: &NameBindi
import: Import { kind: ImportKind::ExternCrate { .. }, .. },
..
},
) => import.vis.get() == ty::Visibility::Public,
) => import.vis.get().is_public(),
_ => false,
}
}
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,11 @@ use rustc_hir::{ExprKind, QPath};
use rustc_infer::infer;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::InferOk;
use rustc_middle::ty;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AllowTwoPhase};
use rustc_middle::ty::error::TypeError::{FieldMisMatch, Sorts};
use rustc_middle::ty::relate::expected_found_bool;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::Ty;
use rustc_middle::ty::TypeFoldable;
use rustc_middle::ty::{AdtKind, Visibility};
use rustc_middle::ty::{self, AdtKind, Ty, TypeFoldable};
use rustc_session::parse::feature_err;
use rustc_span::edition::LATEST_STABLE_EDITION;
use rustc_span::hygiene::DesugaringKind;
Expand Down Expand Up @@ -1732,7 +1729,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.filter_map(|field| {
// ignore already set fields and private fields from non-local crates
if skip.iter().any(|&x| x == field.ident.name)
|| (!variant.def_id.is_local() && field.vis != Visibility::Public)
|| (!variant.def_id.is_local() && !field.vis.is_public())
{
None
} else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1410,7 +1410,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
// We only want to suggest public or local traits (#45781).
item.vis == ty::Visibility::Public || info.def_id.is_local()
item.vis.is_public() || info.def_id.is_local()
})
.is_some()
})
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 @@ -435,7 +435,7 @@ crate fn build_impl(
tcx.associated_items(did)
.in_definition_order()
.filter_map(|item| {
if associated_trait.is_some() || item.vis == ty::Visibility::Public {
if associated_trait.is_some() || item.vis.is_public() {
Some(item.clean(cx))
} else {
None
Expand Down Expand Up @@ -515,7 +515,7 @@ fn build_module(
// two namespaces, so the target may be listed twice. Make sure we only
// visit each node at most once.
for &item in cx.tcx.item_children(did).iter() {
if item.vis == ty::Visibility::Public {
if item.vis.is_public() {
let res = item.res.expect_non_local();
if let Some(def_id) = res.mod_def_id() {
if did == def_id || !visited.insert(def_id) {
Expand Down
12 changes: 8 additions & 4 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1881,7 +1881,7 @@ fn clean_extern_crate(
// this is the ID of the crate itself
let crate_def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX };
let attrs = cx.tcx.hir().attrs(krate.hir_id());
let please_inline = krate.vis.node.is_pub()
let please_inline = cx.tcx.visibility(krate.def_id).is_public()
&& attrs.iter().any(|a| {
a.has_name(sym::doc)
&& match a.meta_item_list() {
Expand Down Expand Up @@ -1933,9 +1933,12 @@ fn clean_use_statement(
return Vec::new();
}

let visibility = cx.tcx.visibility(import.def_id);
let attrs = cx.tcx.hir().attrs(import.hir_id());
let inline_attr = attrs.lists(sym::doc).get_word_attr(sym::inline);
let pub_underscore = import.vis.node.is_pub() && name == kw::Underscore;
let pub_underscore = visibility.is_public() && name == kw::Underscore;
let current_mod = cx.tcx.parent_module_from_def_id(import.def_id);
let parent_mod = cx.tcx.parent_module_from_def_id(current_mod);

if pub_underscore {
if let Some(ref inline) = inline_attr {
Expand All @@ -1954,8 +1957,9 @@ fn clean_use_statement(
// forcefully don't inline if this is not public or if the
// #[doc(no_inline)] attribute is present.
// Don't inline doc(hidden) imports so they can be stripped at a later stage.
let mut denied = !(import.vis.node.is_pub()
|| (cx.render_options.document_private && import.vis.node.is_pub_restricted()))
let mut denied = !(visibility.is_public()
|| (cx.render_options.document_private
&& visibility.is_accessible_from(parent_mod.to_def_id(), cx.tcx)))
|| pub_underscore
|| attrs.iter().any(|a| {
a.has_name(sym::doc)
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ impl ExternalCrate {
as_keyword(Res::Def(DefKind::Mod, id.def_id.to_def_id()))
}
hir::ItemKind::Use(path, hir::UseKind::Single)
if item.vis.node.is_pub() =>
if tcx.visibility(id.def_id).is_public() =>
{
as_keyword(path.res.expect_non_local())
.map(|(_, prim)| (id.def_id.to_def_id(), prim))
Expand Down Expand Up @@ -320,7 +320,7 @@ impl ExternalCrate {
as_primitive(Res::Def(DefKind::Mod, id.def_id.to_def_id()))
}
hir::ItemKind::Use(path, hir::UseKind::Single)
if item.vis.node.is_pub() =>
if tcx.visibility(id.def_id).is_public() =>
{
as_primitive(path.res.expect_non_local()).map(|(_, prim)| {
// Pretend the primitive is local.
Expand Down
13 changes: 5 additions & 8 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use rustc_hir::CRATE_HIR_ID;
use rustc_middle::middle::privacy::AccessLevel;
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE};
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{kw, sym, Symbol};

use std::mem;
Expand Down Expand Up @@ -72,9 +71,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
}

crate fn visit(mut self) -> Module<'tcx> {
let span = self.cx.tcx.def_span(CRATE_DEF_ID);
let mut top_level_module = self.visit_mod_contents(
&Spanned { span, node: hir::VisibilityKind::Public },
hir::CRATE_HIR_ID,
self.cx.tcx.hir().root_module(),
self.cx.tcx.crate_name(LOCAL_CRATE),
Expand Down Expand Up @@ -134,15 +131,15 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {

fn visit_mod_contents(
&mut self,
vis: &hir::Visibility<'_>,
id: hir::HirId,
m: &'tcx hir::Mod<'tcx>,
name: Symbol,
) -> Module<'tcx> {
let mut om = Module::new(name, id, m.inner);
let def_id = self.cx.tcx.hir().local_def_id(id).to_def_id();
// Keep track of if there were any private modules in the path.
let orig_inside_public_path = self.inside_public_path;
self.inside_public_path &= vis.node.is_pub();
self.inside_public_path &= self.cx.tcx.visibility(def_id).is_public();
for &i in m.item_ids {
let item = self.cx.tcx.hir().item(i);
self.visit_item(item, None, &mut om);
Expand Down Expand Up @@ -259,7 +256,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
let name = renamed.unwrap_or(item.ident.name);

let def_id = item.def_id.to_def_id();
let is_pub = item.vis.node.is_pub() || self.cx.tcx.has_attr(def_id, sym::macro_export);
let is_pub = self.cx.tcx.visibility(def_id).is_public();

if is_pub {
self.store_path(item.def_id.to_def_id());
Expand Down Expand Up @@ -332,7 +329,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
}
}
hir::ItemKind::Mod(ref m) => {
om.mods.push(self.visit_mod_contents(&item.vis, item.hir_id(), m, name));
om.mods.push(self.visit_mod_contents(item.hir_id(), m, name));
}
hir::ItemKind::Fn(..)
| hir::ItemKind::ExternCrate(..)
Expand Down Expand Up @@ -368,7 +365,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
om: &mut Module<'tcx>,
) {
// If inlining we only want to include public functions.
if !self.inlining || item.vis.node.is_pub() {
if !self.inlining || self.cx.tcx.visibility(item.def_id).is_public() {
om.foreigns.push((item, renamed));
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/visit_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX};
use rustc_middle::middle::privacy::{AccessLevel, AccessLevels};
use rustc_middle::ty::{TyCtxt, Visibility};
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::sym;

use crate::clean::{AttributesExt, NestedAttributesExt};
Expand Down Expand Up @@ -59,7 +59,7 @@ impl<'a, 'tcx> LibEmbargoVisitor<'a, 'tcx> {
for item in self.tcx.item_children(def_id).iter() {
if let Some(def_id) = item.res.opt_def_id() {
if self.tcx.def_key(def_id).parent.map_or(false, |d| d == def_id.index)
|| item.vis == Visibility::Public
|| item.vis.is_public()
{
self.visit_item(item.res);
}
Expand All @@ -70,7 +70,7 @@ impl<'a, 'tcx> LibEmbargoVisitor<'a, 'tcx> {
fn visit_item(&mut self, res: Res<!>) {
let def_id = res.def_id();
let vis = self.tcx.visibility(def_id);
let inherited_item_level = if vis == Visibility::Public { self.prev_level } else { None };
let inherited_item_level = if vis.is_public() { self.prev_level } else { None };

let item_level = self.update(def_id, inherited_item_level);

Expand Down

0 comments on commit a8dff34

Please sign in to comment.