Skip to content

Commit 526830d

Browse files
borsbors
committed
Auto merge of #118657 - petrochenkov:feedvis, r=<try>
resolve: Replace visibility table in resolver outputs with query feeding I suspect that in general this approach should work for queries that are 1) executed for most keys and 2) have results that are cheap to hash (do not have spans, in particular). Visibility query matches that description, so let's check.
2 parents 56278a6 + 1f539c6 commit 526830d

File tree

6 files changed

+52
-66
lines changed

6 files changed

+52
-66
lines changed

‎compiler/rustc_middle/src/hir/map/mod.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,8 +1077,6 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh {
10771077

10781078
let upstream_crates = upstream_crates(tcx);
10791079

1080-
let resolutions = tcx.resolutions(());
1081-
10821080
// We hash the final, remapped names of all local source files so we
10831081
// don't have to include the path prefix remapping commandline args.
10841082
// If we included the full mapping in the SVH, we could only have
@@ -1133,8 +1131,6 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh {
11331131
}
11341132
tcx.sess.opts.dep_tracking_hash(true).hash_stable(&mut hcx, &mut stable_hasher);
11351133
tcx.stable_crate_id(LOCAL_CRATE).hash_stable(&mut hcx, &mut stable_hasher);
1136-
// Hash visibility information since it does not appear in HIR.
1137-
resolutions.visibilities.hash_stable(&mut hcx, &mut stable_hasher);
11381134
stable_hasher.finish()
11391135
});
11401136

‎compiler/rustc_middle/src/ty/mod.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,6 @@ pub struct ResolverOutputs {
158158

159159
#[derive(Debug)]
160160
pub struct ResolverGlobalCtxt {
161-
pub visibilities: FxHashMap<LocalDefId, Visibility>,
162161
/// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`.
163162
pub expn_that_defined: FxHashMap<LocalDefId, ExpnId>,
164163
pub effective_visibilities: EffectiveVisibilities,

‎compiler/rustc_privacy/src/lib.rs‎

Lines changed: 33 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1801,48 +1801,43 @@ fn visibility(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Visibility<DefId> {
18011801
}
18021802

18031803
fn local_visibility(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Visibility {
1804-
match tcx.resolutions(()).visibilities.get(&def_id) {
1805-
Some(vis) => *vis,
1806-
None => {
1807-
let hir_id = tcx.local_def_id_to_hir_id(def_id);
1808-
match tcx.hir().get(hir_id) {
1809-
// Unique types created for closures participate in type privacy checking.
1810-
// They have visibilities inherited from the module they are defined in.
1811-
Node::Expr(hir::Expr { kind: hir::ExprKind::Closure{..}, .. })
1812-
// - AST lowering creates dummy `use` items which don't
1813-
// get their entries in the resolver's visibility table.
1814-
// - AST lowering also creates opaque type items with inherited visibilities.
1815-
// Visibility on them should have no effect, but to avoid the visibility
1816-
// query failing on some items, we provide it for opaque types as well.
1817-
| Node::Item(hir::Item {
1818-
kind: hir::ItemKind::Use(_, hir::UseKind::ListStem)
1819-
| hir::ItemKind::OpaqueTy(..),
1804+
let hir_id = tcx.local_def_id_to_hir_id(def_id);
1805+
match tcx.hir().get(hir_id) {
1806+
// Unique types created for closures participate in type privacy checking.
1807+
// They have visibilities inherited from the module they are defined in.
1808+
Node::Expr(hir::Expr { kind: hir::ExprKind::Closure{..}, .. })
1809+
// - AST lowering creates dummy `use` items which don't
1810+
// get their entries in the resolver's visibility table.
1811+
// - AST lowering also creates opaque type items with inherited visibilities.
1812+
// Visibility on them should have no effect, but to avoid the visibility
1813+
// query failing on some items, we provide it for opaque types as well.
1814+
| Node::Item(hir::Item {
1815+
kind: hir::ItemKind::Use(_, hir::UseKind::ListStem)
1816+
| hir::ItemKind::OpaqueTy(..),
1817+
..
1818+
}) => ty::Visibility::Restricted(tcx.parent_module(hir_id).to_local_def_id()),
1819+
// Visibilities of trait impl items are inherited from their traits
1820+
// and are not filled in resolve.
1821+
Node::ImplItem(impl_item) => {
1822+
match tcx.hir().get_by_def_id(tcx.hir().get_parent_item(hir_id).def_id) {
1823+
Node::Item(hir::Item {
1824+
kind: hir::ItemKind::Impl(hir::Impl { of_trait: Some(tr), .. }),
18201825
..
1821-
}) => ty::Visibility::Restricted(tcx.parent_module(hir_id).to_local_def_id()),
1822-
// Visibilities of trait impl items are inherited from their traits
1823-
// and are not filled in resolve.
1824-
Node::ImplItem(impl_item) => {
1825-
match tcx.hir().get_by_def_id(tcx.hir().get_parent_item(hir_id).def_id) {
1826-
Node::Item(hir::Item {
1827-
kind: hir::ItemKind::Impl(hir::Impl { of_trait: Some(tr), .. }),
1828-
..
1829-
}) => tr.path.res.opt_def_id().map_or_else(
1830-
|| {
1831-
tcx.sess.span_delayed_bug(tr.path.span, "trait without a def-id");
1832-
ty::Visibility::Public
1833-
},
1834-
|def_id| tcx.visibility(def_id).expect_local(),
1835-
),
1836-
_ => span_bug!(impl_item.span, "the parent is not a trait impl"),
1837-
}
1838-
}
1839-
_ => span_bug!(
1840-
tcx.def_span(def_id),
1841-
"visibility table unexpectedly missing a def-id: {:?}",
1842-
def_id,
1826+
}) => tr.path.res.opt_def_id().map_or_else(
1827+
|| {
1828+
tcx.sess.span_delayed_bug(tr.path.span, "trait without a def-id");
1829+
ty::Visibility::Public
1830+
},
1831+
|def_id| tcx.visibility(def_id).expect_local(),
18431832
),
1833+
_ => span_bug!(impl_item.span, "the parent is not a trait impl"),
18441834
}
18451835
}
1836+
_ => span_bug!(
1837+
tcx.def_span(def_id),
1838+
"visibility table unexpectedly missing a def-id: {:?}",
1839+
def_id,
1840+
),
18461841
}
18471842
}
18481843

‎compiler/rustc_resolve/src/build_reduced_graph.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
231231
// (i.e. variants, fields, and trait items) inherits from the visibility
232232
// of the enum or trait.
233233
ModuleKind::Def(DefKind::Enum | DefKind::Trait, def_id, _) => {
234-
self.r.visibilities[&def_id.expect_local()]
234+
self.r.tcx.visibility(def_id).expect_local()
235235
}
236236
// Otherwise, the visibility is restricted to the nearest parent `mod` item.
237237
_ => ty::Visibility::Restricted(
@@ -437,7 +437,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
437437
let mut source = module_path.pop().unwrap();
438438
let mut type_ns_only = false;
439439

440-
self.r.visibilities.insert(self.r.local_def_id(id), vis);
440+
self.r.feed_visibility(self.r.local_def_id(id), vis);
441441

442442
if nested {
443443
// Correctly handle `self`
@@ -552,7 +552,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
552552
max_vis: Cell::new(None),
553553
id,
554554
};
555-
self.r.visibilities.insert(self.r.local_def_id(id), vis);
555+
self.r.feed_visibility(self.r.local_def_id(id), vis);
556556
self.add_import(prefix, kind, use_tree.span, item, root_span, item.id, vis);
557557
}
558558
ast::UseTreeKind::Nested(ref items) => {
@@ -629,7 +629,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
629629
let local_def_id = self.r.local_def_id(item.id);
630630
let def_id = local_def_id.to_def_id();
631631

632-
self.r.visibilities.insert(local_def_id, vis);
632+
self.r.feed_visibility(local_def_id, vis);
633633

634634
match item.kind {
635635
ItemKind::Use(ref use_tree) => {
@@ -753,7 +753,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
753753
let ctor_res =
754754
Res::Def(DefKind::Ctor(CtorOf::Struct, ctor_kind), ctor_def_id.to_def_id());
755755
self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, sp, expansion));
756-
self.r.visibilities.insert(ctor_def_id, ctor_vis);
756+
self.r.feed_visibility(ctor_def_id, ctor_vis);
757757
// We need the field visibility spans also for the constructor for E0603.
758758
self.insert_field_visibilities_local(ctor_def_id.to_def_id(), vdata);
759759

@@ -899,7 +899,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
899899
let vis = self.resolve_visibility(&item.vis);
900900
let res = Res::Def(def_kind, def_id);
901901
self.r.define(parent, item.ident, ns, (res, vis, item.span, expansion));
902-
self.r.visibilities.insert(local_def_id, vis);
902+
self.r.feed_visibility(local_def_id, vis);
903903
}
904904

905905
fn build_reduced_graph_for_block(&mut self, block: &Block) {
@@ -1233,7 +1233,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
12331233
self.r.check_reserved_macro_name(ident, res);
12341234
self.insert_unused_macro(ident, def_id, item.id);
12351235
}
1236-
self.r.visibilities.insert(def_id, vis);
1236+
self.r.feed_visibility(def_id, vis);
12371237
let scope = self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding(
12381238
self.r.arenas.alloc_macro_rules_binding(MacroRulesBinding {
12391239
parent_macro_rules_scope: parent_scope.macro_rules,
@@ -1257,7 +1257,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
12571257
self.insert_unused_macro(ident, def_id, item.id);
12581258
}
12591259
self.r.define(module, ident, MacroNS, (res, vis, span, expansion));
1260-
self.r.visibilities.insert(def_id, vis);
1260+
self.r.feed_visibility(def_id, vis);
12611261
self.parent_scope.macro_rules
12621262
}
12631263
}
@@ -1359,7 +1359,7 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> {
13591359
// Trait impl item visibility is inherited from its trait when not specified
13601360
// explicitly. In that case we cannot determine it here in early resolve,
13611361
// so we leave a hole in the visibility table to be filled later.
1362-
self.r.visibilities.insert(local_def_id, vis);
1362+
self.r.feed_visibility(local_def_id, vis);
13631363
}
13641364

13651365
if ctxt == AssocCtxt::Trait {
@@ -1438,7 +1438,7 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> {
14381438
self.visit_invoc(sf.id);
14391439
} else {
14401440
let vis = self.resolve_visibility(&sf.vis);
1441-
self.r.visibilities.insert(self.r.local_def_id(sf.id), vis);
1441+
self.r.feed_visibility(self.r.local_def_id(sf.id), vis);
14421442
visit::walk_field_def(self, sf);
14431443
}
14441444
}
@@ -1460,7 +1460,7 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> {
14601460
let res = Res::Def(DefKind::Variant, def_id.to_def_id());
14611461
let vis = self.resolve_visibility(&variant.vis);
14621462
self.r.define(parent, ident, TypeNS, (res, vis, variant.span, expn_id));
1463-
self.r.visibilities.insert(def_id, vis);
1463+
self.r.feed_visibility(def_id, vis);
14641464

14651465
// If the variant is marked as non_exhaustive then lower the visibility to within the crate.
14661466
let ctor_vis =
@@ -1476,7 +1476,7 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> {
14761476
let ctor_res =
14771477
Res::Def(DefKind::Ctor(CtorOf::Variant, ctor_kind), ctor_def_id.to_def_id());
14781478
self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, variant.span, expn_id));
1479-
self.r.visibilities.insert(ctor_def_id, ctor_vis);
1479+
self.r.feed_visibility(ctor_def_id, ctor_vis);
14801480
}
14811481

14821482
// Record field names for error reporting.

‎compiler/rustc_resolve/src/effective_visibilities.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
186186
) -> Option<Option<Visibility>> {
187187
match parent_id {
188188
ParentId::Def(def_id) => (nominal_vis != self.current_private_vis
189-
&& self.r.visibilities[&def_id] != self.current_private_vis)
189+
&& self.r.tcx.local_visibility(def_id) != self.current_private_vis)
190190
.then_some(Some(self.current_private_vis)),
191191
ParentId::Import(_) => Some(None),
192192
}
@@ -222,7 +222,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
222222
}
223223

224224
fn update_field(&mut self, def_id: LocalDefId, parent_id: LocalDefId) {
225-
self.update_def(def_id, self.r.visibilities[&def_id], ParentId::Def(parent_id));
225+
self.update_def(def_id, self.r.tcx.local_visibility(def_id), ParentId::Def(parent_id));
226226
}
227227
}
228228

‎compiler/rustc_resolve/src/lib.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ mod imports;
8585
mod late;
8686
mod macros;
8787
pub mod rustdoc;
88-
8988
rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
9089

9190
#[derive(Debug)]
@@ -1007,8 +1006,6 @@ pub struct Resolver<'a, 'tcx> {
10071006

10081007
/// Maps glob imports to the names of items actually imported.
10091008
glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,
1010-
/// Visibilities in "lowered" form, for all entities that have them.
1011-
visibilities: FxHashMap<LocalDefId, ty::Visibility>,
10121009
used_imports: FxHashSet<NodeId>,
10131010
maybe_unused_trait_imports: FxIndexSet<LocalDefId>,
10141011

@@ -1295,9 +1292,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
12951292
&mut FxHashMap::default(),
12961293
);
12971294

1298-
let mut visibilities = FxHashMap::default();
1299-
visibilities.insert(CRATE_DEF_ID, ty::Visibility::Public);
1300-
13011295
let mut def_id_to_node_id = IndexVec::default();
13021296
assert_eq!(def_id_to_node_id.push(CRATE_NODE_ID), CRATE_DEF_ID);
13031297
let mut node_id_to_def_id = FxHashMap::default();
@@ -1363,7 +1357,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13631357
ast_transform_scopes: FxHashMap::default(),
13641358

13651359
glob_map: Default::default(),
1366-
visibilities,
13671360
used_imports: FxHashSet::default(),
13681361
maybe_unused_trait_imports: Default::default(),
13691362

@@ -1450,6 +1443,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
14501443

14511444
let root_parent_scope = ParentScope::module(graph_root, &resolver);
14521445
resolver.invocation_parent_scopes.insert(LocalExpnId::ROOT, root_parent_scope);
1446+
resolver.feed_visibility(CRATE_DEF_ID, ty::Visibility::Public);
14531447

14541448
resolver
14551449
}
@@ -1497,10 +1491,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
14971491
Default::default()
14981492
}
14991493

1494+
fn feed_visibility(&self, def_id: LocalDefId, vis: ty::Visibility) {
1495+
self.tcx.feed_local_def_id(def_id).visibility(vis.to_def_id());
1496+
}
1497+
15001498
pub fn into_outputs(self) -> ResolverOutputs {
15011499
let proc_macros = self.proc_macros.iter().map(|id| self.local_def_id(*id)).collect();
15021500
let expn_that_defined = self.expn_that_defined;
1503-
let visibilities = self.visibilities;
15041501
let extern_crate_map = self.extern_crate_map;
15051502
let maybe_unused_trait_imports = self.maybe_unused_trait_imports;
15061503
let glob_map = self.glob_map;
@@ -1517,7 +1514,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
15171514

15181515
let global_ctxt = ResolverGlobalCtxt {
15191516
expn_that_defined,
1520-
visibilities,
15211517
effective_visibilities,
15221518
extern_crate_map,
15231519
module_children: self.module_children,

0 commit comments

Comments
 (0)