Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit d3c9082

Browse files
committed
Auto merge of #120006 - cjgillot:no-hir-owner, r=wesleywiser
Get rid of the hir_owner query. This query was meant as a firewall between `hir_owner_nodes` which is supposed to change often, and the queries that only depend on the item signature. That firewall was inefficient, leaking the contents of the HIR body through `HirId`s. `hir_owner` incurs a significant cost, as we need to hash HIR twice in multiple modes. This PR proposes to remove it, and simplify the hashing scheme. For the future, `def_kind`, `def_span`... are much more efficient for incremental decoupling, and should be preferred.
2 parents 25f8d01 + d59968b commit d3c9082

File tree

30 files changed

+581
-716
lines changed

30 files changed

+581
-716
lines changed

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -662,9 +662,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
662662
let (opt_hash_including_bodies, attrs_hash) = if self.tcx.needs_crate_hash() {
663663
self.tcx.with_stable_hashing_context(|mut hcx| {
664664
let mut stable_hasher = StableHasher::new();
665-
hcx.with_hir_bodies(node.def_id(),&bodies, |hcx| {
666-
node.hash_stable(hcx,&mut stable_hasher)
667-
});
665+
node.hash_stable(&muthcx,&mut stable_hasher);
666+
// Bodies are stored out of line, so we need to pull them explicitly in the hash.
667+
bodies.hash_stable(&mut hcx,&mut stable_hasher);
668668
let h1 = stable_hasher.finish();
669669

670670
let mut stable_hasher = StableHasher::new();

‎compiler/rustc_hir/src/hir.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -841,7 +841,7 @@ pub struct OwnerNodes<'tcx> {
841841
}
842842

843843
impl<'tcx> OwnerNodes<'tcx> {
844-
fn node(&self) -> OwnerNode<'tcx> {
844+
pubfn node(&self) -> OwnerNode<'tcx> {
845845
use rustc_index::Idx;
846846
let node = self.nodes[ItemLocalId::new(0)].as_ref().unwrap().node;
847847
let node = node.as_owner().unwrap(); // Indexing must ensure it is an OwnerNode.
@@ -1305,7 +1305,7 @@ pub enum UnsafeSource {
13051305
UserProvided,
13061306
}
13071307

1308-
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
1308+
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug,HashStable_Generic)]
13091309
pub struct BodyId {
13101310
pub hir_id: HirId,
13111311
}

‎compiler/rustc_hir/src/stable_hash_impls.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use rustc_span::def_id::DefPathHash;
1212
pub trait HashStableContext:
1313
rustc_ast::HashStableContext + rustc_target::HashStableContext
1414
{
15-
fn hash_body_id(&mut self, _: BodyId, hasher: &mut StableHasher);
1615
}
1716

1817
impl<HirCtx: crate::HashStableContext> ToStableHashKey<HirCtx> for HirId {
@@ -80,12 +79,6 @@ impl<HirCtx: crate::HashStableContext> ToStableHashKey<HirCtx> for ForeignItemId
8079
}
8180
}
8281

83-
impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for BodyId {
84-
fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
85-
hcx.hash_body_id(*self, hasher)
86-
}
87-
}
88-
8982
// The following implementations of HashStable for `ItemId`, `TraitItemId`, and
9083
// `ImplItemId` deserve special attention. Normally we do not hash `NodeId`s within
9184
// the HIR, since they just signify a HIR nodes own path. But `ItemId` et al

‎compiler/rustc_incremental/src/assert_dep_graph.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,11 @@ impl<'tcx> IfThisChanged<'tcx> {
128128
if attr.has_name(sym::rustc_if_this_changed) {
129129
let dep_node_interned = self.argument(attr);
130130
let dep_node = match dep_node_interned {
131-
None => {
132-
DepNode::from_def_path_hash(self.tcx, def_path_hash, dep_kinds::hir_owner)
133-
}
131+
None => DepNode::from_def_path_hash(
132+
self.tcx,
133+
def_path_hash,
134+
dep_kinds::hir_owner_nodes,
135+
),
134136
Some(n) => {
135137
match DepNode::from_label_string(self.tcx, n.as_str(), def_path_hash) {
136138
Ok(n) => n,

‎compiler/rustc_incremental/src/persist/dirty_clean.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ const BASE_FN: &[&str] = &[
5757

5858
/// DepNodes for Hir, which is pretty much everything
5959
const BASE_HIR: &[&str] = &[
60-
// hir_owner and hir_owner_nodes should be computed for all nodes
61-
label_strs::hir_owner,
60+
// hir_owner_nodes should be computed for all nodes
6261
label_strs::hir_owner_nodes,
6362
];
6463

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

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::hir::{ModuleItems,Owner};
1+
use crate::hir::ModuleItems;
22
use crate::middle::debugger_visualizer::DebuggerVisualizerFile;
33
use crate::query::LocalCrate;
44
use crate::ty::TyCtxt;
@@ -108,7 +108,7 @@ impl<'hir> Iterator for ParentOwnerIterator<'hir> {
108108
if self.current_id.local_id.index() != 0 {
109109
self.current_id.local_id = ItemLocalId::new(0);
110110
if let Some(node) = self.map.tcx.hir_owner(self.current_id.owner) {
111-
return Some((self.current_id.owner, node.node));
111+
return Some((self.current_id.owner, node));
112112
}
113113
}
114114
if self.current_id == CRATE_HIR_ID {
@@ -126,23 +126,23 @@ impl<'hir> Iterator for ParentOwnerIterator<'hir> {
126126

127127
// If this `HirId` doesn't have an entry, skip it and look for its `parent_id`.
128128
if let Some(node) = self.map.tcx.hir_owner(self.current_id.owner) {
129-
return Some((self.current_id.owner, node.node));
129+
return Some((self.current_id.owner, node));
130130
}
131131
}
132132
}
133133
}
134134

135135
impl<'tcx> TyCtxt<'tcx> {
136+
#[inline]
137+
fn hir_owner(self, owner: OwnerId) -> Option<OwnerNode<'tcx>> {
138+
Some(self.hir_owner_nodes(owner).as_owner()?.node())
139+
}
140+
136141
/// Retrieves the `hir::Node` corresponding to `id`, returning `None` if cannot be found.
137142
pub fn opt_hir_node(self, id: HirId) -> Option<Node<'tcx>> {
138-
if id.local_id == ItemLocalId::from_u32(0) {
139-
let owner = self.hir_owner(id.owner)?;
140-
Some(owner.node.into())
141-
} else {
142-
let owner = self.hir_owner_nodes(id.owner).as_owner()?;
143-
let node = owner.nodes[id.local_id].as_ref()?;
144-
Some(node.node)
145-
}
143+
let owner = self.hir_owner_nodes(id.owner).as_owner()?;
144+
let node = owner.nodes[id.local_id].as_ref()?;
145+
Some(node.node)
146146
}
147147

148148
/// Retrieves the `hir::Node` corresponding to `id`, returning `None` if cannot be found.
@@ -174,7 +174,7 @@ impl<'hir> Map<'hir> {
174174

175175
#[inline]
176176
pub fn root_module(self) -> &'hir Mod<'hir> {
177-
match self.tcx.hir_owner(CRATE_OWNER_ID).map(|o| o.node) {
177+
match self.tcx.hir_owner(CRATE_OWNER_ID) {
178178
Some(OwnerNode::Crate(item)) => item,
179179
_ => bug!(),
180180
}
@@ -242,27 +242,27 @@ impl<'hir> Map<'hir> {
242242

243243
pub fn get_generics(self, id: LocalDefId) -> Option<&'hir Generics<'hir>> {
244244
let node = self.tcx.hir_owner(OwnerId { def_id: id })?;
245-
node.node.generics()
245+
node.generics()
246246
}
247247

248248
pub fn owner(self, id: OwnerId) -> OwnerNode<'hir> {
249-
self.tcx.hir_owner(id).unwrap_or_else(|| bug!("expected owner for {:?}", id)).node
249+
self.tcx.hir_owner(id).unwrap_or_else(|| bug!("expected owner for {:?}", id))
250250
}
251251

252252
pub fn item(self, id: ItemId) -> &'hir Item<'hir> {
253-
self.tcx.hir_owner(id.owner_id).unwrap().node.expect_item()
253+
self.tcx.hir_owner(id.owner_id).unwrap().expect_item()
254254
}
255255

256256
pub fn trait_item(self, id: TraitItemId) -> &'hir TraitItem<'hir> {
257-
self.tcx.hir_owner(id.owner_id).unwrap().node.expect_trait_item()
257+
self.tcx.hir_owner(id.owner_id).unwrap().expect_trait_item()
258258
}
259259

260260
pub fn impl_item(self, id: ImplItemId) -> &'hir ImplItem<'hir> {
261-
self.tcx.hir_owner(id.owner_id).unwrap().node.expect_impl_item()
261+
self.tcx.hir_owner(id.owner_id).unwrap().expect_impl_item()
262262
}
263263

264264
pub fn foreign_item(self, id: ForeignItemId) -> &'hir ForeignItem<'hir> {
265-
self.tcx.hir_owner(id.owner_id).unwrap().node.expect_foreign_item()
265+
self.tcx.hir_owner(id.owner_id).unwrap().expect_foreign_item()
266266
}
267267

268268
pub fn body(self, id: BodyId) -> &'hir Body<'hir> {
@@ -436,7 +436,7 @@ impl<'hir> Map<'hir> {
436436

437437
pub fn get_module(self, module: LocalModDefId) -> (&'hir Mod<'hir>, Span, HirId) {
438438
let hir_id = HirId::make_owner(module.to_local_def_id());
439-
match self.tcx.hir_owner(hir_id.owner).map(|o| o.node) {
439+
match self.tcx.hir_owner(hir_id.owner) {
440440
Some(OwnerNode::Item(&Item { span, kind: ItemKind::Mod(m), .. })) => (m, span, hir_id),
441441
Some(OwnerNode::Crate(item)) => (item, item.spans.inner_span, hir_id),
442442
node => panic!("not a module: {node:?}"),
@@ -726,11 +726,10 @@ impl<'hir> Map<'hir> {
726726

727727
pub fn get_foreign_abi(self, hir_id: HirId) -> Abi {
728728
let parent = self.get_parent_item(hir_id);
729-
if let Some(node) = self.tcx.hir_owner(parent) {
730-
if let OwnerNode::Item(Item { kind: ItemKind::ForeignMod { abi, .. }, .. }) = node.node
731-
{
732-
return *abi;
733-
}
729+
if let Some(node) = self.tcx.hir_owner(parent)
730+
&& let OwnerNode::Item(Item { kind: ItemKind::ForeignMod { abi, .. }, .. }) = node
731+
{
732+
return *abi;
734733
}
735734
bug!(
736735
"expected foreign mod or inlined parent, found {}",
@@ -742,33 +741,32 @@ impl<'hir> Map<'hir> {
742741
self.tcx
743742
.hir_owner(OwnerId { def_id })
744743
.unwrap_or_else(|| bug!("expected owner for {:?}", def_id))
745-
.node
746744
}
747745

748746
pub fn expect_item(self, id: LocalDefId) -> &'hir Item<'hir> {
749747
match self.tcx.hir_owner(OwnerId { def_id: id }) {
750-
Some(Owner{node:OwnerNode::Item(item), .. }) => item,
748+
Some(OwnerNode::Item(item)) => item,
751749
_ => bug!("expected item, found {}", self.node_to_string(HirId::make_owner(id))),
752750
}
753751
}
754752

755753
pub fn expect_impl_item(self, id: LocalDefId) -> &'hir ImplItem<'hir> {
756754
match self.tcx.hir_owner(OwnerId { def_id: id }) {
757-
Some(Owner{node:OwnerNode::ImplItem(item), .. }) => item,
755+
Some(OwnerNode::ImplItem(item)) => item,
758756
_ => bug!("expected impl item, found {}", self.node_to_string(HirId::make_owner(id))),
759757
}
760758
}
761759

762760
pub fn expect_trait_item(self, id: LocalDefId) -> &'hir TraitItem<'hir> {
763761
match self.tcx.hir_owner(OwnerId { def_id: id }) {
764-
Some(Owner{node:OwnerNode::TraitItem(item), .. }) => item,
762+
Some(OwnerNode::TraitItem(item)) => item,
765763
_ => bug!("expected trait item, found {}", self.node_to_string(HirId::make_owner(id))),
766764
}
767765
}
768766

769767
pub fn get_fn_output(self, def_id: LocalDefId) -> Option<&'hir FnRetTy<'hir>> {
770768
match self.tcx.hir_owner(OwnerId { def_id }) {
771-
Some(Owner{node, .. }) => node.fn_decl().map(|fn_decl| &fn_decl.output),
769+
Some(node) => node.fn_decl().map(|fn_decl| &fn_decl.output),
772770
_ => None,
773771
}
774772
}
@@ -782,7 +780,7 @@ impl<'hir> Map<'hir> {
782780

783781
pub fn expect_foreign_item(self, id: OwnerId) -> &'hir ForeignItem<'hir> {
784782
match self.tcx.hir_owner(id) {
785-
Some(Owner{node:OwnerNode::ForeignItem(item), .. }) => item,
783+
Some(OwnerNode::ForeignItem(item)) => item,
786784
_ => {
787785
bug!(
788786
"expected foreign item, found {}",

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

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,34 +8,12 @@ pub mod place;
88

99
use crate::query::Providers;
1010
use crate::ty::{EarlyBinder, ImplSubject, TyCtxt};
11-
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
1211
use rustc_data_structures::sync::{try_par_for_each_in, DynSend, DynSync};
1312
use rustc_hir::def::DefKind;
1413
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
1514
use rustc_hir::*;
16-
use rustc_query_system::ich::StableHashingContext;
1715
use rustc_span::{ErrorGuaranteed, ExpnId, DUMMY_SP};
1816

19-
/// Top-level HIR node for current owner. This only contains the node for which
20-
/// `HirId::local_id == 0`, and excludes bodies.
21-
///
22-
/// This struct exists to encapsulate all access to the hir_owner query in this module, and to
23-
/// implement HashStable without hashing bodies.
24-
#[derive(Copy, Clone, Debug)]
25-
pub struct Owner<'tcx> {
26-
node: OwnerNode<'tcx>,
27-
}
28-
29-
impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for Owner<'tcx> {
30-
#[inline]
31-
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
32-
// Perform a shallow hash instead using the deep hash saved in `OwnerNodes`. This lets us
33-
// differentiate queries that depend on the full HIR tree from those that only depend on
34-
// the item signature.
35-
hcx.without_hir_bodies(|hcx| self.node.hash_stable(hcx, hasher));
36-
}
37-
}
38-
3917
/// Gather the LocalDefId for each item-like within a module, including items contained within
4018
/// bodies. The Ids are in visitor order. This is used to partition a pass between modules.
4119
#[derive(Debug, HashStable, Encodable, Decodable)]
@@ -149,11 +127,6 @@ pub fn provide(providers: &mut Providers) {
149127
providers.hir_crate_items = map::hir_crate_items;
150128
providers.crate_hash = map::crate_hash;
151129
providers.hir_module_items = map::hir_module_items;
152-
providers.hir_owner = |tcx, id| {
153-
let owner = tcx.hir_crate(()).owners.get(id.def_id)?.as_owner()?;
154-
let node = owner.node();
155-
Some(Owner { node })
156-
};
157130
providers.opt_local_def_id_to_hir_id = |tcx, id| {
158131
let owner = tcx.hir_crate(()).owners[id].map(|_| ());
159132
Some(match owner {
@@ -162,7 +135,13 @@ pub fn provide(providers: &mut Providers) {
162135
MaybeOwner::NonOwner(hir_id) => hir_id,
163136
})
164137
};
165-
providers.hir_owner_nodes = |tcx, id| tcx.hir_crate(()).owners[id.def_id].map(|i| &i.nodes);
138+
providers.hir_owner_nodes = |tcx, id| {
139+
if let Some(i) = tcx.hir_crate(()).owners.get(id.def_id) {
140+
i.map(|i| &i.nodes)
141+
} else {
142+
MaybeOwner::Phantom
143+
}
144+
};
166145
providers.hir_owner_parent = |tcx, id| {
167146
// Accessing the local_parent is ok since its value is hashed as part of `id`'s DefPathHash.
168147
tcx.opt_local_parent(id.def_id).map_or(CRATE_HIR_ID, |parent| {

‎compiler/rustc_middle/src/query/erase.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,6 @@ impl<T> EraseType for Option<&'_ [T]> {
164164
type Result = [u8; size_of::<Option<&'static [()]>>()];
165165
}
166166

167-
impl EraseType for Option<rustc_middle::hir::Owner<'_>> {
168-
type Result = [u8; size_of::<Option<rustc_middle::hir::Owner<'static>>>()];
169-
}
170-
171167
impl EraseType for Option<mir::DestructuredConstant<'_>> {
172168
type Result = [u8; size_of::<Option<mir::DestructuredConstant<'static>>>()];
173169
}
@@ -324,7 +320,6 @@ macro_rules! tcx_lifetime {
324320
}
325321

326322
tcx_lifetime! {
327-
rustc_middle::hir::Owner,
328323
rustc_middle::middle::exported_symbols::ExportedSymbol,
329324
rustc_middle::mir::Const,
330325
rustc_middle::mir::DestructuredConstant,

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,6 @@ rustc_queries! {
174174
cache_on_disk_if { true }
175175
}
176176

177-
/// Gives access to the HIR node for the HIR owner `key`.
178-
///
179-
/// This can be conveniently accessed by methods on `tcx.hir()`.
180-
/// Avoid calling this query directly.
181-
query hir_owner(key: hir::OwnerId) -> Option<crate::hir::Owner<'tcx>> {
182-
desc { |tcx| "getting HIR owner of `{}`", tcx.def_path_str(key) }
183-
}
184-
185177
/// Gives access to the HIR ID for the given `LocalDefId` owner `key` if any.
186178
///
187179
/// Definitions that were generated with no HIR, would be fed to return `None`.

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /