From be321aa473433e5192b02099531020914f42c5da Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 5 Dec 2023 23:16:08 +0300 Subject: [PATCH] resolve: Replace visibility table in resolver outputs with query feeding Also feed missing visibilities for import stems and trait impl items, which were previously evaluated lazily. --- compiler/rustc_ast_lowering/src/lib.rs | 10 +-- compiler/rustc_middle/src/hir/map/mod.rs | 5 +- compiler/rustc_middle/src/ty/mod.rs | 2 +- compiler/rustc_privacy/src/lib.rs | 63 +++---------------- .../rustc_resolve/src/build_reduced_graph.rs | 28 +++++---- .../src/effective_visibilities.rs | 4 +- compiler/rustc_resolve/src/late.rs | 2 + compiler/rustc_resolve/src/lib.rs | 17 ++--- 8 files changed, 49 insertions(+), 82 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 5c9e43f1e064c..96ed3eee02ed9 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -59,10 +59,8 @@ use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res}; use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; use rustc_hir::{ConstArg, GenericArg, ItemLocalId, ParamName, TraitCandidate}; use rustc_index::{Idx, IndexSlice, IndexVec}; -use rustc_middle::{ - span_bug, - ty::{ResolverAstLowering, TyCtxt}, -}; +use rustc_middle::span_bug; +use rustc_middle::ty::{ResolverAstLowering, TyCtxt, Visibility}; use rustc_session::parse::{add_feature_diagnostics, feature_err}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{DesugaringKind, Span, DUMMY_SP}; @@ -1653,6 +1651,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ); debug!(?opaque_ty_def_id); + // Meaningless, but provided so that all items have visibilities. + let parent_mod = self.tcx.parent_module_from_def_id(opaque_ty_def_id).to_def_id(); + self.tcx.feed_local_def_id(opaque_ty_def_id).visibility(Visibility::Restricted(parent_mod)); + // Map from captured (old) lifetime to synthetic (new) lifetime. // Used to resolve lifetimes in the bounds of the opaque. let mut captured_to_synthesized_mapping = FxHashMap::default(); diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 598c26de23f1c..81f34c7b8b0a9 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -1142,7 +1142,10 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh { tcx.sess.opts.dep_tracking_hash(true).hash_stable(&mut hcx, &mut stable_hasher); tcx.stable_crate_id(LOCAL_CRATE).hash_stable(&mut hcx, &mut stable_hasher); // Hash visibility information since it does not appear in HIR. - resolutions.visibilities.hash_stable(&mut hcx, &mut stable_hasher); + // FIXME: Figure out how to remove `visibilities_for_hashing` by hashing visibilities on + // the fly in the resolver, storing only their accumulated hash in `ResolverGlobalCtxt`, + // and combining it with other hashes here. + resolutions.visibilities_for_hashing.hash_stable(&mut hcx, &mut stable_hasher); stable_hasher.finish() }); diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index febab23ea351e..6513bf501e167 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -152,7 +152,7 @@ pub struct ResolverOutputs { #[derive(Debug)] pub struct ResolverGlobalCtxt { - pub visibilities: FxHashMap, + pub visibilities_for_hashing: Vec<(LocalDefId, Visibility)>, /// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`. pub expn_that_defined: FxHashMap, pub effective_visibilities: EffectiveVisibilities, diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 6a8e23d9a8f09..61f6a2b18ae60 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -23,12 +23,11 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId, CRATE_DEF_ID}; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::{AssocItemKind, ForeignItemKind, ItemId, Node, PatKind}; +use rustc_hir::{AssocItemKind, ForeignItemKind, ItemId, PatKind}; use rustc_middle::bug; use rustc_middle::hir::nested_filter; use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility, Level}; use rustc_middle::query::Providers; -use rustc_middle::span_bug; use rustc_middle::ty::GenericArgs; use rustc_middle::ty::{self, Const, GenericParamDefKind}; use rustc_middle::ty::{TraitRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor}; @@ -1766,7 +1765,15 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> { pub fn provide(providers: &mut Providers) { *providers = Providers { - visibility, + visibility: |tcx, def_id| { + // Unique types created for closures participate in type privacy checking. + // They have visibilities inherited from the module they are defined in. + // FIXME: Consider evaluating visibilities for closures eagerly, like for all + // other nodes. However, unlike for others, for closures it may cause a perf + // regression, because closure visibilities are not commonly queried. + assert_eq!(tcx.def_kind(def_id), DefKind::Closure); + ty::Visibility::Restricted(tcx.parent_module_from_def_id(def_id).to_def_id()) + }, effective_visibilities, check_private_in_public, check_mod_privacy, @@ -1774,56 +1781,6 @@ pub fn provide(providers: &mut Providers) { }; } -fn visibility(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Visibility { - local_visibility(tcx, def_id).to_def_id() -} - -fn local_visibility(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Visibility { - match tcx.resolutions(()).visibilities.get(&def_id) { - Some(vis) => *vis, - None => { - let hir_id = tcx.local_def_id_to_hir_id(def_id); - match tcx.hir_node(hir_id) { - // Unique types created for closures participate in type privacy checking. - // They have visibilities inherited from the module they are defined in. - Node::Expr(hir::Expr { kind: hir::ExprKind::Closure{..}, .. }) - // - AST lowering creates dummy `use` items which don't - // get their entries in the resolver's visibility table. - // - AST lowering also creates opaque type items with inherited visibilities. - // Visibility on them should have no effect, but to avoid the visibility - // query failing on some items, we provide it for opaque types as well. - | Node::Item(hir::Item { - kind: hir::ItemKind::Use(_, hir::UseKind::ListStem) - | hir::ItemKind::OpaqueTy(..), - .. - }) => ty::Visibility::Restricted(tcx.parent_module(hir_id).to_local_def_id()), - // Visibilities of trait impl items are inherited from their traits - // and are not filled in resolve. - Node::ImplItem(impl_item) => { - match tcx.hir_node_by_def_id(tcx.hir().get_parent_item(hir_id).def_id) { - Node::Item(hir::Item { - kind: hir::ItemKind::Impl(hir::Impl { of_trait: Some(tr), .. }), - .. - }) => tr.path.res.opt_def_id().map_or_else( - || { - tcx.sess.span_delayed_bug(tr.path.span, "trait without a def-id"); - ty::Visibility::Public - }, - |def_id| tcx.visibility(def_id).expect_local(), - ), - _ => span_bug!(impl_item.span, "the parent is not a trait impl"), - } - } - _ => span_bug!( - tcx.def_span(def_id), - "visibility table unexpectedly missing a def-id: {:?}", - def_id, - ), - } - } - } -} - fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) { // Check privacy of names not checked in previous compilation stages. let mut visitor = NamePrivacyVisitor { diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 213b320ed1a66..7ff3c523685cc 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -236,7 +236,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { // (i.e. variants, fields, and trait items) inherits from the visibility // of the enum or trait. ModuleKind::Def(DefKind::Enum | DefKind::Trait, def_id, _) => { - self.r.visibilities[&def_id.expect_local()] + self.r.tcx.visibility(def_id).expect_local() } // Otherwise, the visibility is restricted to the nearest parent `mod` item. _ => ty::Visibility::Restricted( @@ -404,6 +404,10 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { parent_prefix, use_tree, nested ); + if nested { + self.r.feed_visibility(self.r.local_def_id(id), vis); + } + let mut prefix_iter = parent_prefix .iter() .cloned() @@ -442,8 +446,6 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { let mut source = module_path.pop().unwrap(); let mut type_ns_only = false; - self.r.visibilities.insert(self.r.local_def_id(id), vis); - if nested { // Correctly handle `self` if source.ident.name == kw::SelfLower { @@ -557,7 +559,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { max_vis: Cell::new(None), id, }; - self.r.visibilities.insert(self.r.local_def_id(id), vis); + self.add_import(prefix, kind, use_tree.span, item, root_span, item.id, vis); } ast::UseTreeKind::Nested(ref items) => { @@ -636,7 +638,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { let def_kind = self.r.tcx.def_kind(def_id); let res = Res::Def(def_kind, def_id); - self.r.visibilities.insert(local_def_id, vis); + self.r.feed_visibility(local_def_id, vis); match item.kind { ItemKind::Use(ref use_tree) => { @@ -753,7 +755,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { let ctor_def_id = self.r.local_def_id(ctor_node_id); let ctor_res = self.res(ctor_def_id); self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, sp, expansion)); - self.r.visibilities.insert(ctor_def_id, ctor_vis); + self.r.feed_visibility(ctor_def_id, ctor_vis); // We need the field visibility spans also for the constructor for E0603. self.insert_field_visibilities_local(ctor_def_id.to_def_id(), vdata); @@ -897,7 +899,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { let expansion = self.parent_scope.expansion; let vis = self.resolve_visibility(&item.vis); self.r.define(parent, item.ident, ns, (self.res(def_id), vis, item.span, expansion)); - self.r.visibilities.insert(local_def_id, vis); + self.r.feed_visibility(local_def_id, vis); } fn build_reduced_graph_for_block(&mut self, block: &Block) { @@ -1228,7 +1230,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { self.r.check_reserved_macro_name(ident, res); self.insert_unused_macro(ident, def_id, item.id); } - self.r.visibilities.insert(def_id, vis); + self.r.feed_visibility(def_id, vis); let scope = self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding( self.r.arenas.alloc_macro_rules_binding(MacroRulesBinding { parent_macro_rules_scope: parent_scope.macro_rules, @@ -1252,7 +1254,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { self.insert_unused_macro(ident, def_id, item.id); } self.r.define(module, ident, MacroNS, (res, vis, span, expansion)); - self.r.visibilities.insert(def_id, vis); + self.r.feed_visibility(def_id, vis); self.parent_scope.macro_rules } } @@ -1354,7 +1356,7 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> { // Trait impl item visibility is inherited from its trait when not specified // explicitly. In that case we cannot determine it here in early resolve, // so we leave a hole in the visibility table to be filled later. - self.r.visibilities.insert(local_def_id, vis); + self.r.feed_visibility(local_def_id, vis); } if ctxt == AssocCtxt::Trait { @@ -1432,7 +1434,7 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> { self.visit_invoc(sf.id); } else { let vis = self.resolve_visibility(&sf.vis); - self.r.visibilities.insert(self.r.local_def_id(sf.id), vis); + self.r.feed_visibility(self.r.local_def_id(sf.id), vis); visit::walk_field_def(self, sf); } } @@ -1453,7 +1455,7 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> { let def_id = self.r.local_def_id(variant.id); let vis = self.resolve_visibility(&variant.vis); self.r.define(parent, ident, TypeNS, (self.res(def_id), vis, variant.span, expn_id)); - self.r.visibilities.insert(def_id, vis); + self.r.feed_visibility(def_id, vis); // If the variant is marked as non_exhaustive then lower the visibility to within the crate. let ctor_vis = @@ -1468,7 +1470,7 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> { let ctor_def_id = self.r.local_def_id(ctor_node_id); let ctor_res = self.res(ctor_def_id); self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, variant.span, expn_id)); - self.r.visibilities.insert(ctor_def_id, ctor_vis); + self.r.feed_visibility(ctor_def_id, ctor_vis); } // Record field names for error reporting. diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs index a71c50dd82fdd..50352169221d7 100644 --- a/compiler/rustc_resolve/src/effective_visibilities.rs +++ b/compiler/rustc_resolve/src/effective_visibilities.rs @@ -186,7 +186,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> { ) -> Option> { match parent_id { ParentId::Def(def_id) => (nominal_vis != self.current_private_vis - && self.r.visibilities[&def_id] != self.current_private_vis) + && self.r.tcx.local_visibility(def_id) != self.current_private_vis) .then_some(Some(self.current_private_vis)), ParentId::Import(_) => Some(None), } @@ -222,7 +222,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> { } fn update_field(&mut self, def_id: LocalDefId, parent_id: LocalDefId) { - self.update_def(def_id, self.r.visibilities[&def_id], ParentId::Def(parent_id)); + self.update_def(def_id, self.r.tcx.local_visibility(def_id), ParentId::Def(parent_id)); } } diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 9c96e9a9bd728..037179350f044 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -3112,6 +3112,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { | (DefKind::AssocFn, AssocItemKind::Fn(..)) | (DefKind::AssocConst, AssocItemKind::Const(..)) => { self.r.record_partial_res(id, PartialRes::new(res)); + let vis = self.r.tcx.visibility(id_in_trait).expect_local(); + self.r.feed_visibility(self.r.local_def_id(id), vis); return; } _ => {} diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 70e0eb12c01da..ad637472b47b9 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1007,8 +1007,7 @@ pub struct Resolver<'a, 'tcx> { /// Maps glob imports to the names of items actually imported. glob_map: FxHashMap>, - /// Visibilities in "lowered" form, for all entities that have them. - visibilities: FxHashMap, + visibilities_for_hashing: Vec<(LocalDefId, ty::Visibility)>, used_imports: FxHashSet, maybe_unused_trait_imports: FxIndexSet, @@ -1295,9 +1294,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { &mut FxHashMap::default(), ); - let mut visibilities = FxHashMap::default(); - visibilities.insert(CRATE_DEF_ID, ty::Visibility::Public); - let mut def_id_to_node_id = IndexVec::default(); assert_eq!(def_id_to_node_id.push(CRATE_NODE_ID), CRATE_DEF_ID); let mut node_id_to_def_id = FxHashMap::default(); @@ -1363,7 +1359,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { ast_transform_scopes: FxHashMap::default(), glob_map: Default::default(), - visibilities, + visibilities_for_hashing: Default::default(), used_imports: FxHashSet::default(), maybe_unused_trait_imports: Default::default(), @@ -1450,6 +1446,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let root_parent_scope = ParentScope::module(graph_root, &resolver); resolver.invocation_parent_scopes.insert(LocalExpnId::ROOT, root_parent_scope); + resolver.feed_visibility(CRATE_DEF_ID, ty::Visibility::Public); resolver } @@ -1497,10 +1494,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Default::default() } + fn feed_visibility(&mut self, def_id: LocalDefId, vis: ty::Visibility) { + self.tcx.feed_local_def_id(def_id).visibility(vis.to_def_id()); + self.visibilities_for_hashing.push((def_id, vis)); + } + pub fn into_outputs(self) -> ResolverOutputs { let proc_macros = self.proc_macros.iter().map(|id| self.local_def_id(*id)).collect(); let expn_that_defined = self.expn_that_defined; - let visibilities = self.visibilities; let extern_crate_map = self.extern_crate_map; let maybe_unused_trait_imports = self.maybe_unused_trait_imports; let glob_map = self.glob_map; @@ -1517,7 +1518,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let global_ctxt = ResolverGlobalCtxt { expn_that_defined, - visibilities, + visibilities_for_hashing: self.visibilities_for_hashing, effective_visibilities, extern_crate_map, module_children: self.module_children,