Skip to content

Commit

Permalink
Rollup merge of rust-lang#68889 - Zoxc:hir-krate, r=eddyb
Browse files Browse the repository at this point in the history
Move the `hir().krate()` method to a query and remove the `Krate` dep node

r? @eddyb cc @michaelwoerister
  • Loading branch information
Dylan-DPC authored Feb 7, 2020
2 parents b1aae7f + a575495 commit 32a3871
Show file tree
Hide file tree
Showing 26 changed files with 146 additions and 156 deletions.
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>,
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> {
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>,

/// 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

0 comments on commit 32a3871

Please sign in to comment.