Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the hir().krate() method to a query and remove the Krate dep node #68889

Merged
merged 6 commits into from
Feb 7, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ macro_rules! arena_types {
[] tys: rustc::ty::TyS<$tcx>,

// HIR types
[few] hir_forest: rustc::hir::map::Forest<$tcx>,
[few] hir_krate: rustc_hir::Crate<$tcx>,
[] arm: rustc_hir::Arm<$tcx>,
[] attribute: syntax::ast::Attribute,
[] block: rustc_hir::Block<$tcx>,
Expand Down
15 changes: 1 addition & 14 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
//! "infer" some properties for each kind of `DepNode`:
//!
//! * Whether a `DepNode` of a given kind has any parameters at all. Some
//! `DepNode`s, like `Krate`, represent global concepts with only one value.
//! `DepNode`s, like `AllLocalTraitImpls`, represent global concepts with only one value.
//! * Whether it is possible, in principle, to reconstruct a query key from a
//! given `DepNode`. Many `DepKind`s only require a single `DefId` parameter,
//! in which case it is possible to map the node's fingerprint back to the
Expand Down Expand Up @@ -400,19 +400,6 @@ rustc_dep_node_append!([define_dep_nodes!][ <'tcx>
// We use this for most things when incr. comp. is turned off.
[] Null,

// Represents the `Krate` as a whole (the `hir::Krate` value) (as
// distinct from the krate module). This is basically a hash of
// the entire krate, so if you read from `Krate` (e.g., by calling
// `tcx.hir().krate()`), we will have to assume that any change
// means that you need to be recompiled. This is because the
// `Krate` value gives you access to all other items. To avoid
// this fate, do not call `tcx.hir().krate()`; instead, prefer
// wrappers like `tcx.visit_all_items_in_krate()`. If there is no
// suitable wrapper, you can use `tcx.dep_graph.ignore()` to gain
// access to the krate, but you must remember to add suitable
// edges yourself for the individual items that you read.
[eval_always] Krate,

// Represents the body of a function or method. The def-id is that of the
// function/method.
[eval_always] HirBody(DefId),
Expand Down
9 changes: 3 additions & 6 deletions src/librustc/hir/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,9 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
(commandline_args_hash, crate_disambiguator.to_fingerprint()),
);

let (_, crate_hash) = input_dep_node_and_hash(
self.dep_graph,
&mut self.hcx,
DepNode::new_no_params(DepKind::Krate),
crate_hash_input,
);
let mut stable_hasher = StableHasher::new();
crate_hash_input.hash_stable(&mut self.hcx, &mut stable_hasher);
let crate_hash: Fingerprint = stable_hasher.finish();

let svh = Svh::new(crate_hash.to_smaller_hash());
(self.map, svh)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/map/hir_id_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub fn check_crate(hir_map: &Map<'_>) {

let errors = Lock::new(Vec::new());

par_iter(&hir_map.krate().modules).for_each(|(module_id, _)| {
par_iter(&hir_map.krate.modules).for_each(|(module_id, _)| {
let local_def_id = hir_map.local_def_id(*module_id);
hir_map.visit_item_likes_in_module(
local_def_id,
Expand Down
98 changes: 31 additions & 67 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,30 +129,6 @@ impl<'hir> Entry<'hir> {
}
}

/// Stores a crate and any number of inlined items from other crates.
pub struct Forest<'hir> {
krate: Crate<'hir>,
pub dep_graph: DepGraph,
}

impl Forest<'hir> {
pub fn new(krate: Crate<'hir>, dep_graph: &DepGraph) -> Forest<'hir> {
Forest { krate, dep_graph: dep_graph.clone() }
}

pub fn krate(&self) -> &Crate<'hir> {
self.dep_graph.read(DepNode::new_no_params(DepKind::Krate));
&self.krate
}

/// This is used internally in the dependency tracking system.
/// Use the `krate` method to ensure your dependency on the
/// crate is tracked.
pub fn untracked_krate(&self) -> &Crate<'hir> {
&self.krate
}
}

/// This type is effectively a `HashMap<HirId, Entry<'hir>>`,
/// but it is implemented as 2 layers of arrays.
/// - first we have `A = IndexVec<DefIndex, B>` mapping `DefIndex`s to an inner value
Expand All @@ -162,11 +138,8 @@ pub(super) type HirEntryMap<'hir> = IndexVec<DefIndex, IndexVec<ItemLocalId, Opt
/// Represents a mapping from `NodeId`s to AST elements and their parent `NodeId`s.
#[derive(Clone)]
pub struct Map<'hir> {
/// The backing storage for all the AST nodes.
pub forest: &'hir Forest<'hir>,
pub krate: &'hir Crate<'hir>,

/// Same as the dep_graph in forest, just available with one fewer
/// deref. This is a gratuitous micro-optimization.
pub dep_graph: DepGraph,

/// The SVH of the local crate.
Expand Down Expand Up @@ -217,6 +190,13 @@ impl<'hir> Iterator for ParentHirIterator<'_, 'hir> {
}

impl<'hir> Map<'hir> {
/// This is used internally in the dependency tracking system.
/// Use the `krate` method to ensure your dependency on the
/// crate is tracked.
pub fn untracked_krate(&self) -> &Crate<'hir> {
&self.krate
}

#[inline]
fn lookup(&self, id: HirId) -> Option<&Entry<'hir>> {
let local_map = self.map.get(id.owner)?;
Expand Down Expand Up @@ -401,40 +381,36 @@ impl<'hir> Map<'hir> {
self.lookup(id).cloned()
}

pub fn krate(&self) -> &'hir Crate<'hir> {
self.forest.krate()
}

pub fn item(&self, id: HirId) -> &'hir Item<'hir> {
self.read(id);

// N.B., intentionally bypass `self.forest.krate()` so that we
// N.B., intentionally bypass `self.krate()` so that we
// do not trigger a read of the whole krate here
self.forest.krate.item(id)
self.krate.item(id)
}

pub fn trait_item(&self, id: TraitItemId) -> &'hir TraitItem<'hir> {
self.read(id.hir_id);

// N.B., intentionally bypass `self.forest.krate()` so that we
// N.B., intentionally bypass `self.krate()` so that we
// do not trigger a read of the whole krate here
self.forest.krate.trait_item(id)
self.krate.trait_item(id)
}

pub fn impl_item(&self, id: ImplItemId) -> &'hir ImplItem<'hir> {
self.read(id.hir_id);

// N.B., intentionally bypass `self.forest.krate()` so that we
// N.B., intentionally bypass `self.krate()` so that we
// do not trigger a read of the whole krate here
self.forest.krate.impl_item(id)
self.krate.impl_item(id)
}

pub fn body(&self, id: BodyId) -> &'hir Body<'hir> {
self.read(id.hir_id);

// N.B., intentionally bypass `self.forest.krate()` so that we
// N.B., intentionally bypass `self.krate()` so that we
// do not trigger a read of the whole krate here
self.forest.krate.body(id)
self.krate.body(id)
}

pub fn fn_decl_by_hir_id(&self, hir_id: HirId) -> Option<&'hir FnDecl<'hir>> {
Expand Down Expand Up @@ -530,9 +506,9 @@ impl<'hir> Map<'hir> {
pub fn trait_impls(&self, trait_did: DefId) -> &'hir [HirId] {
self.dep_graph.read(DepNode::new_no_params(DepKind::AllLocalTraitImpls));

// N.B., intentionally bypass `self.forest.krate()` so that we
// N.B., intentionally bypass `self.krate()` so that we
// do not trigger a read of the whole krate here
self.forest.krate.trait_impls.get(&trait_did).map_or(&[], |xs| &xs[..])
self.krate.trait_impls.get(&trait_did).map_or(&[], |xs| &xs[..])
}

/// Gets the attributes on the crate. This is preferable to
Expand All @@ -542,15 +518,15 @@ impl<'hir> Map<'hir> {
let def_path_hash = self.definitions.def_path_hash(CRATE_DEF_INDEX);

self.dep_graph.read(def_path_hash.to_dep_node(DepKind::Hir));
&self.forest.krate.attrs
&self.krate.attrs
}

pub fn get_module(&self, module: DefId) -> (&'hir Mod<'hir>, Span, HirId) {
let hir_id = self.as_local_hir_id(module).unwrap();
self.read(hir_id);
match self.find_entry(hir_id).unwrap().node {
Node::Item(&Item { span, kind: ItemKind::Mod(ref m), .. }) => (m, span, hir_id),
Node::Crate => (&self.forest.krate.module, self.forest.krate.span, hir_id),
Node::Crate => (&self.krate.module, self.krate.span, hir_id),
node => panic!("not a module: {:?}", node),
}
}
Expand All @@ -567,7 +543,7 @@ impl<'hir> Map<'hir> {
// in the expect_* calls the loops below
self.read(hir_id);

let module = &self.forest.krate.modules[&hir_id];
let module = &self.krate.modules[&hir_id];

for id in &module.items {
visitor.visit_item(self.expect_item(*id));
Expand Down Expand Up @@ -984,7 +960,7 @@ impl<'hir> Map<'hir> {
// Unit/tuple structs/variants take the attributes straight from
// the struct/variant definition.
Some(Node::Ctor(..)) => return self.attrs(self.get_parent_item(id)),
Some(Node::Crate) => Some(&self.forest.krate.attrs[..]),
Some(Node::Crate) => Some(&self.krate.attrs[..]),
_ => None,
};
attrs.unwrap_or(&[])
Expand Down Expand Up @@ -1063,7 +1039,7 @@ impl<'hir> Map<'hir> {
Some(Node::Visibility(v)) => bug!("unexpected Visibility {:?}", v),
Some(Node::Local(local)) => local.span,
Some(Node::MacroDef(macro_def)) => macro_def.span,
Some(Node::Crate) => self.forest.krate.span,
Some(Node::Crate) => self.krate.span,
None => bug!("hir::map::Map::span: id not in map: {:?}", hir_id),
}
}
Expand Down Expand Up @@ -1231,7 +1207,8 @@ impl Named for ImplItem<'_> {
pub fn map_crate<'hir>(
sess: &rustc_session::Session,
cstore: &CrateStoreDyn,
forest: &'hir Forest<'hir>,
krate: &'hir Crate<'hir>,
dep_graph: DepGraph,
definitions: Definitions,
) -> Map<'hir> {
let _prof_timer = sess.prof.generic_activity("build_hir_map");
Expand All @@ -1244,31 +1221,18 @@ pub fn map_crate<'hir>(
.collect();

let (map, crate_hash) = {
let hcx = crate::ich::StableHashingContext::new(sess, &forest.krate, &definitions, cstore);

let mut collector = NodeCollector::root(
sess,
&forest.krate,
&forest.dep_graph,
&definitions,
&hir_to_node_id,
hcx,
);
intravisit::walk_crate(&mut collector, &forest.krate);
let hcx = crate::ich::StableHashingContext::new(sess, krate, &definitions, cstore);

let mut collector =
NodeCollector::root(sess, krate, &dep_graph, &definitions, &hir_to_node_id, hcx);
intravisit::walk_crate(&mut collector, krate);

let crate_disambiguator = sess.local_crate_disambiguator();
let cmdline_args = sess.opts.dep_tracking_hash();
collector.finalize_and_compute_crate_hash(crate_disambiguator, cstore, cmdline_args)
};

let map = Map {
forest,
dep_graph: forest.dep_graph.clone(),
crate_hash,
map,
hir_to_node_id,
definitions,
};
let map = Map { krate, dep_graph, crate_hash, map, hir_to_node_id, definitions };

sess.time("validate_HIR_map", || {
hir_id_validator::check_crate(&map);
Expand Down
41 changes: 41 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,48 @@ pub mod exports;
pub mod map;

use crate::ty::query::Providers;
use crate::ty::TyCtxt;
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_hir::print;
use rustc_hir::Crate;
use std::ops::Deref;

/// A wrapper type which allows you to access HIR.
#[derive(Clone)]
pub struct Hir<'tcx> {
tcx: TyCtxt<'tcx>,
map: &'tcx map::Map<'tcx>,
}

impl<'tcx> Hir<'tcx> {
pub fn krate(&self) -> &'tcx Crate<'tcx> {
self.tcx.hir_crate(LOCAL_CRATE)
}
}

impl<'tcx> Deref for Hir<'tcx> {
eddyb marked this conversation as resolved.
Show resolved Hide resolved
type Target = &'tcx map::Map<'tcx>;

#[inline(always)]
fn deref(&self) -> &Self::Target {
&self.map
}
}

impl<'hir> print::PpAnn for Hir<'hir> {
fn nested(&self, state: &mut print::State<'_>, nested: print::Nested) {
self.map.nested(state, nested)
}
}

impl<'tcx> TyCtxt<'tcx> {
#[inline(always)]
pub fn hir(self) -> Hir<'tcx> {
Hir { tcx: self, map: &self.hir_map }
}
}

pub fn provide(providers: &mut Providers<'_>) {
providers.hir_crate = |tcx, _| tcx.hir_map.untracked_krate();
map::provide(providers);
}
12 changes: 12 additions & 0 deletions src/librustc/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ rustc_queries! {
}

Other {
// Represents crate as a whole (as distinct from the top-level crate module).
// If you call `hir_crate` (e.g., indirectly by calling `tcx.hir().krate()`),
// we will have to assume that any change means that you need to be recompiled.
// This is because the `hir_crate` query gives you access to all other items.
// To avoid this fate, do not call `tcx.hir().krate()`; instead,
// prefer wrappers like `tcx.visit_all_items_in_krate()`.
query hir_crate(key: CrateNum) -> &'tcx Crate<'tcx> {
eval_always
no_hash
desc { "get the crate HIR" }
}

/// Records the type of every item.
query type_of(key: DefId) -> Ty<'tcx> {
cache_on_disk_if { key.is_local() }
Expand Down
10 changes: 3 additions & 7 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,8 @@ pub struct GlobalCtxt<'tcx> {
/// Export map produced by name resolution.
export_map: FxHashMap<DefId, Vec<Export<hir::HirId>>>,

hir_map: hir_map::Map<'tcx>,
/// This should usually be accessed with the `tcx.hir()` method.
pub(crate) hir_map: hir_map::Map<'tcx>,
Zoxc marked this conversation as resolved.
Show resolved Hide resolved

/// A map from `DefPathHash` -> `DefId`. Includes `DefId`s from the local crate
/// as well as all upstream crates. Only populated in incremental mode.
Expand Down Expand Up @@ -1019,11 +1020,6 @@ pub struct GlobalCtxt<'tcx> {
}

impl<'tcx> TyCtxt<'tcx> {
#[inline(always)]
pub fn hir(self) -> &'tcx hir_map::Map<'tcx> {
&self.hir_map
}

pub fn alloc_steal_mir(self, mir: BodyAndCache<'tcx>) -> &'tcx Steal<BodyAndCache<'tcx>> {
self.arena.alloc(Steal::new(mir))
}
Expand Down Expand Up @@ -1328,7 +1324,7 @@ impl<'tcx> TyCtxt<'tcx> {

#[inline(always)]
pub fn create_stable_hashing_context(self) -> StableHashingContext<'tcx> {
let krate = self.gcx.hir_map.forest.untracked_krate();
let krate = self.gcx.hir_map.untracked_krate();

StableHashingContext::new(self.sess, krate, self.hir().definitions(), &*self.cstore)
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use rustc_data_structures::sync::Lrc;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, DefIndex};
use rustc_hir::{HirIdSet, ItemLocalId, TraitCandidate};
use rustc_hir::{Crate, HirIdSet, ItemLocalId, TraitCandidate};
use rustc_index::vec::IndexVec;
use rustc_target::spec::PanicStrategy;

Expand Down
1 change: 0 additions & 1 deletion src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,6 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool {
// These are inputs that are expected to be pre-allocated and that
// should therefore always be red or green already.
DepKind::AllLocalTraitImpls |
DepKind::Krate |
DepKind::CrateMetadata |
DepKind::HirBody |
DepKind::Hir |
Expand Down
Loading