From 9a987b04668641772e2d481bdacb6e00ab3d95ef Mon Sep 17 00:00:00 2001 From: inquisitivecrystal <22333129+inquisitivecrystal@users.noreply.github.com> Date: Sun, 7 Nov 2021 19:53:26 -0800 Subject: [PATCH 1/2] Add `ty::Visibility::is_public()` --- compiler/rustc_metadata/src/rmeta/decoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs | 2 +- compiler/rustc_middle/src/ty/mod.rs | 4 ++++ compiler/rustc_middle/src/ty/print/pretty.rs | 2 +- compiler/rustc_privacy/src/lib.rs | 9 ++++----- compiler/rustc_resolve/src/check_unused.rs | 3 +-- compiler/rustc_resolve/src/diagnostics.rs | 4 ++-- compiler/rustc_resolve/src/imports.rs | 2 +- compiler/rustc_typeck/src/check/expr.rs | 7 ++----- compiler/rustc_typeck/src/check/method/suggest.rs | 2 +- src/librustdoc/clean/inline.rs | 4 ++-- src/librustdoc/visit_lib.rs | 6 +++--- 12 files changed, 23 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 5e90aec003e9b..b2c7818a54218 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -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 diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 7ea004b16f23b..eeb0a77adc0ad 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -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; } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 40670f1fdcaef..673733faa766f 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -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 diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 742005e245f9d..5d9e7aaf72f8e 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -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; } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index a01efc5d85c6e..11668146f7b10 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -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 { @@ -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, _) @@ -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)); @@ -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 diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 760b746996196..63699128e9e16 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -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}; @@ -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 { diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index ff0d76e94fdf5..094a5ed7bfbfe 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -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; @@ -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); diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 936ab81914a99..4262c1e9051ee 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -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, } } diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index a9c0b65a0981e..5c79a067e9f04 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -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; @@ -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 { diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 13f475cd9e026..71cd8a43329c5 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -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() }) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index d670288270a40..4a8a316037961 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -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 @@ -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) { diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs index 3e98ba08fb9ca..791f7ff437c8b 100644 --- a/src/librustdoc/visit_lib.rs +++ b/src/librustdoc/visit_lib.rs @@ -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}; @@ -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); } @@ -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); From 6622376ff65d5c5f60f8a8877b223bb69e9a9552 Mon Sep 17 00:00:00 2001 From: inquisitivecrystal <22333129+inquisitivecrystal@users.noreply.github.com> Date: Sun, 7 Nov 2021 19:54:19 -0800 Subject: [PATCH 2/2] Use computed visibility in rustdoc --- src/librustdoc/clean/mod.rs | 12 ++++++++---- src/librustdoc/clean/types.rs | 4 ++-- src/librustdoc/visit_ast.rs | 13 +++++-------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 3db0ef17fd810..a44641f4488e8 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -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() { @@ -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 { @@ -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) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index fd4d620c9591e..2dba52afcd9cd 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -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)) @@ -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. diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 5d1f934240f03..1191a94a7039b 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -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; @@ -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), @@ -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); @@ -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()); @@ -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(..) @@ -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)); } }