diff --git a/src/librustc/dep_graph/README.md b/src/librustc/dep_graph/README.md index f16a9b386bb8a..48f5b7ea2595d 100644 --- a/src/librustc/dep_graph/README.md +++ b/src/librustc/dep_graph/README.md @@ -341,6 +341,8 @@ path is found (as demonstrated above). ### Debugging the dependency graph +#### Dumping the graph + The compiler is also capable of dumping the dependency graph for your debugging pleasure. To do so, pass the `-Z dump-dep-graph` flag. The graph will be dumped to `dep_graph.{txt,dot}` in the current @@ -392,6 +394,35 @@ This will dump out all the nodes that lead from `Hir(foo)` to `TypeckItemBody(bar)`, from which you can (hopefully) see the source of the erroneous edge. +#### Tracking down incorrect edges + +Sometimes, after you dump the dependency graph, you will find some +path that should not exist, but you will not be quite sure how it came +to be. **When the compiler is built with debug assertions,** it can +help you track that down. Simply set the `RUST_FORBID_DEP_GRAPH_EDGE` +environment variable to a filter. Every edge created in the dep-graph +will be tested against that filter -- if it matches, a `bug!` is +reported, so you can easily see the backtrace (`RUST_BACKTRACE=1`). + +The syntax for these filters is the same as described in the previous +section. However, note that this filter is applied to every **edge** +and doesn't handle longer paths in the graph, unlike the previous +section. + +Example: + +You find that there is a path from the `Hir` of `foo` to the type +check of `bar` and you don't think there should be. You dump the +dep-graph as described in the previous section and open `dep-graph.txt` +to see something like: + + Hir(foo) -> Collect(bar) + Collect(bar) -> TypeckItemBody(bar) + +That first edge looks suspicious to you. So you set +`RUST_FORBID_DEP_GRAPH_EDGE` to `Hir&foo -> Collect&bar`, re-run, and +then observe the backtrace. Voila, bug fixed! + ### Inlining of HIR nodes For the time being, at least, we still sometimes "inline" HIR nodes diff --git a/src/librustc/dep_graph/debug.rs b/src/librustc/dep_graph/debug.rs index 15b0380374c69..5b15c5e67174e 100644 --- a/src/librustc/dep_graph/debug.rs +++ b/src/librustc/dep_graph/debug.rs @@ -66,4 +66,11 @@ impl EdgeFilter { }) } } + + pub fn test(&self, + source: &DepNode, + target: &DepNode) + -> bool { + self.source.test(source) && self.target.test(target) + } } diff --git a/src/librustc/dep_graph/dep_tracking_map.rs b/src/librustc/dep_graph/dep_tracking_map.rs index 88cd1efd3459a..51f7890c7a2f4 100644 --- a/src/librustc/dep_graph/dep_tracking_map.rs +++ b/src/librustc/dep_graph/dep_tracking_map.rs @@ -80,6 +80,17 @@ impl DepTrackingMap { pub fn keys(&self) -> Vec { self.map.keys().cloned().collect() } + + /// Append `elem` to the vector stored for `k`, creating a new vector if needed. + /// This is considered a write to `k`. + pub fn push(&mut self, k: M::Key, elem: E) + where M: DepTrackingMapConfig> + { + self.write(&k); + self.map.entry(k) + .or_insert(Vec::new()) + .push(elem); + } } impl MemoizationMap for RefCell> { diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index bb027b11b45af..c42eeead69ec1 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/src/librustc/dep_graph/graph.rs @@ -46,7 +46,7 @@ impl DepGraph { data: Rc::new(DepGraphData { thread: DepGraphThreadData::new(enabled), previous_work_products: RefCell::new(FnvHashMap()), - work_products: RefCell::new(FnvHashMap()) + work_products: RefCell::new(FnvHashMap()), }) } } diff --git a/src/librustc/dep_graph/mod.rs b/src/librustc/dep_graph/mod.rs index a499cb10f2325..9c00e95c17e05 100644 --- a/src/librustc/dep_graph/mod.rs +++ b/src/librustc/dep_graph/mod.rs @@ -15,6 +15,7 @@ mod edges; mod graph; mod query; mod raii; +mod shadow; mod thread; mod visit; diff --git a/src/librustc/dep_graph/raii.rs b/src/librustc/dep_graph/raii.rs index c43d493d17675..e4f572902f9e5 100644 --- a/src/librustc/dep_graph/raii.rs +++ b/src/librustc/dep_graph/raii.rs @@ -47,3 +47,4 @@ impl<'graph> Drop for IgnoreTask<'graph> { self.data.enqueue(DepMessage::PopIgnore); } } + diff --git a/src/librustc/dep_graph/shadow.rs b/src/librustc/dep_graph/shadow.rs new file mode 100644 index 0000000000000..72a321425ef06 --- /dev/null +++ b/src/librustc/dep_graph/shadow.rs @@ -0,0 +1,145 @@ +// Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! The "Shadow Graph" is maintained on the main thread and which +//! tracks each message relating to the dep-graph and applies some +//! sanity checks as they go by. If an error results, it means you get +//! a nice stack-trace telling you precisely what caused the error. +//! +//! NOTE: This is a debugging facility which can potentially have non-trivial +//! runtime impact. Therefore, it is largely compiled out if +//! debug-assertions are not enabled. +//! +//! The basic sanity check, enabled if you have debug assertions +//! enabled, is that there is always a task (or ignore) on the stack +//! when you do read/write, and that the tasks are pushed/popped +//! according to a proper stack discipline. +//! +//! Optionally, if you specify RUST_FORBID_DEP_GRAPH_EDGE, you can +//! specify an edge filter to be applied to each edge as it is +//! created. See `./README.md` for details. + +use hir::def_id::DefId; +use std::cell::{BorrowState, RefCell}; +use std::env; + +use super::DepNode; +use super::thread::DepMessage; +use super::debug::EdgeFilter; + +pub struct ShadowGraph { + // if you push None onto the stack, that corresponds to an Ignore + stack: RefCell>>>, + forbidden_edge: Option, +} + +const ENABLED: bool = cfg!(debug_assertions); + +impl ShadowGraph { + pub fn new() -> Self { + let forbidden_edge = if !ENABLED { + None + } else { + match env::var("RUST_FORBID_DEP_GRAPH_EDGE") { + Ok(s) => { + match EdgeFilter::new(&s) { + Ok(f) => Some(f), + Err(err) => bug!("RUST_FORBID_DEP_GRAPH_EDGE invalid: {}", err), + } + } + Err(_) => None, + } + }; + + ShadowGraph { + stack: RefCell::new(vec![]), + forbidden_edge: forbidden_edge, + } + } + + pub fn enqueue(&self, message: &DepMessage) { + if ENABLED { + match self.stack.borrow_state() { + BorrowState::Unused => {} + _ => { + // When we apply edge filters, that invokes the + // Debug trait on DefIds, which in turn reads from + // various bits of state and creates reads! Ignore + // those recursive reads. + return; + } + } + + let mut stack = self.stack.borrow_mut(); + match *message { + DepMessage::Read(ref n) => self.check_edge(Some(Some(n)), top(&stack)), + DepMessage::Write(ref n) => self.check_edge(top(&stack), Some(Some(n))), + DepMessage::PushTask(ref n) => stack.push(Some(n.clone())), + DepMessage::PushIgnore => stack.push(None), + DepMessage::PopTask(ref n) => { + match stack.pop() { + Some(Some(m)) => { + if *n != m { + bug!("stack mismatch: found {:?} expected {:?}", m, n) + } + } + Some(None) => bug!("stack mismatch: found Ignore expected {:?}", n), + None => bug!("stack mismatch: found empty stack, expected {:?}", n), + } + } + DepMessage::PopIgnore => { + match stack.pop() { + Some(Some(m)) => bug!("stack mismatch: found {:?} expected ignore", m), + Some(None) => (), + None => bug!("stack mismatch: found empty stack, expected ignore"), + } + } + DepMessage::Query => (), + } + } + } + + fn check_edge(&self, + source: Option>>, + target: Option>>) { + assert!(ENABLED); + match (source, target) { + // cannot happen, one side is always Some(Some(_)) + (None, None) => unreachable!(), + + // nothing on top of the stack + (None, Some(n)) | (Some(n), None) => bug!("read/write of {:?} but no current task", n), + + // this corresponds to an Ignore being top of the stack + (Some(None), _) | (_, Some(None)) => (), + + // a task is on top of the stack + (Some(Some(source)), Some(Some(target))) => { + if let Some(ref forbidden_edge) = self.forbidden_edge { + if forbidden_edge.test(source, target) { + bug!("forbidden edge {:?} -> {:?} created", source, target) + } + } + } + } + } +} + +// Do a little juggling: we get back a reference to an option at the +// top of the stack, convert it to an optional reference. +fn top<'s>(stack: &'s Vec>>) -> Option>> { + stack.last() + .map(|n: &'s Option>| -> Option<&'s DepNode> { + // (*) + // (*) type annotation just there to clarify what would + // otherwise be some *really* obscure code + n.as_ref() + }) +} diff --git a/src/librustc/dep_graph/thread.rs b/src/librustc/dep_graph/thread.rs index 4e16fae187070..90c42d66b7adf 100644 --- a/src/librustc/dep_graph/thread.rs +++ b/src/librustc/dep_graph/thread.rs @@ -20,13 +20,13 @@ use hir::def_id::DefId; use rustc_data_structures::veccell::VecCell; -use std::cell::Cell; use std::sync::mpsc::{self, Sender, Receiver}; use std::thread; use super::DepGraphQuery; use super::DepNode; use super::edges::DepGraphEdges; +use super::shadow::ShadowGraph; #[derive(Debug)] pub enum DepMessage { @@ -42,12 +42,16 @@ pub enum DepMessage { pub struct DepGraphThreadData { enabled: bool, - // Local counter that just tracks how many tasks are pushed onto the - // stack, so that we still get an error in the case where one is - // missing. If dep-graph construction is enabled, we'd get the same - // error when processing tasks later on, but that's annoying because - // it lacks precision about the source of the error. - tasks_pushed: Cell, + // The "shadow graph" is a debugging aid. We give it each message + // in real time as it arrives and it checks for various errors + // (for example, a read/write when there is no current task; it + // can also apply user-defined filters; see `shadow` module for + // details). This only occurs if debug-assertions are enabled. + // + // Note that in some cases the same errors will occur when the + // data is processed off the main thread, but that's annoying + // because it lacks precision about the source of the error. + shadow_graph: ShadowGraph, // current buffer, where we accumulate messages messages: VecCell, @@ -76,7 +80,7 @@ impl DepGraphThreadData { DepGraphThreadData { enabled: enabled, - tasks_pushed: Cell::new(0), + shadow_graph: ShadowGraph::new(), messages: VecCell::with_capacity(INITIAL_CAPACITY), swap_in: rx2, swap_out: tx1, @@ -118,21 +122,7 @@ impl DepGraphThreadData { /// the buffer is full, this may swap.) #[inline] pub fn enqueue(&self, message: DepMessage) { - // Regardless of whether dep graph construction is enabled, we - // still want to check that we always have a valid task on the - // stack when a read/write/etc event occurs. - match message { - DepMessage::Read(_) | DepMessage::Write(_) => - if self.tasks_pushed.get() == 0 { - self.invalid_message("read/write but no current task") - }, - DepMessage::PushTask(_) | DepMessage::PushIgnore => - self.tasks_pushed.set(self.tasks_pushed.get() + 1), - DepMessage::PopTask(_) | DepMessage::PopIgnore => - self.tasks_pushed.set(self.tasks_pushed.get() - 1), - DepMessage::Query => - (), - } + self.shadow_graph.enqueue(&message); if self.enabled { self.enqueue_enabled(message); @@ -147,11 +137,6 @@ impl DepGraphThreadData { self.swap(); } } - - // Outline this too. - fn invalid_message(&self, string: &str) { - bug!("{}; see src/librustc/dep_graph/README.md for more information", string) - } } /// Definition of the depgraph thread. diff --git a/src/librustc/hir/def_id.rs b/src/librustc/hir/def_id.rs index a3b83ec5be4bd..16afa705e3919 100644 --- a/src/librustc/hir/def_id.rs +++ b/src/librustc/hir/def_id.rs @@ -58,19 +58,14 @@ impl fmt::Debug for DefId { write!(f, "DefId {{ krate: {:?}, node: {:?}", self.krate, self.index)?; - // Unfortunately, there seems to be no way to attempt to print - // a path for a def-id, so I'll just make a best effort for now - // and otherwise fallback to just printing the crate/node pair - if self.is_local() { // (1) - // (1) side-step fact that not all external things have paths at - // the moment, such as type parameters - ty::tls::with_opt(|opt_tcx| { - if let Some(tcx) = opt_tcx { - write!(f, " => {}", tcx.item_path_str(*self))?; + ty::tls::with_opt(|opt_tcx| { + if let Some(tcx) = opt_tcx { + if let Some(def_path) = tcx.opt_def_path(*self) { + write!(f, " => {}", def_path.to_string(tcx))?; } - Ok(()) - })?; - } + } + Ok(()) + })?; write!(f, " }}") } diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 3ffc95e64f5a7..5c302d927a718 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -927,6 +927,8 @@ pub fn map_decoded_item<'ast, F: FoldOps>(map: &Map<'ast>, ii: InlinedItem, fold_ops: F) -> &'ast InlinedItem { + let _ignore = map.forest.dep_graph.in_ignore(); + let mut fld = IdAndSpanUpdater::new(fold_ops); let ii = match ii { II::Item(d, i) => II::Item(fld.fold_ops.new_def_id(d), diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index f70349d0ee08b..d2a2f8a972d96 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -24,6 +24,7 @@ #![cfg_attr(not(stage0), deny(warnings))] #![feature(associated_consts)] +#![feature(borrow_state)] #![feature(box_patterns)] #![feature(box_syntax)] #![feature(collections)] diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs index b33bc520fe216..d1722ac6f2f7f 100644 --- a/src/librustc/middle/cstore.rs +++ b/src/librustc/middle/cstore.rs @@ -233,7 +233,7 @@ pub trait CrateStore<'tcx> { def: DefKey) -> Option; fn def_key(&self, def: DefId) -> hir_map::DefKey; - fn relative_def_path(&self, def: DefId) -> hir_map::DefPath; + fn relative_def_path(&self, def: DefId) -> Option; fn variant_kind(&self, def_id: DefId) -> Option; fn struct_ctor_def_id(&self, struct_def_id: DefId) -> Option; fn tuple_struct_definition_if_ctor(&self, did: DefId) -> Option; @@ -430,7 +430,9 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore { // resolve fn def_key(&self, def: DefId) -> hir_map::DefKey { bug!("def_key") } - fn relative_def_path(&self, def: DefId) -> hir_map::DefPath { bug!("relative_def_path") } + fn relative_def_path(&self, def: DefId) -> Option { + bug!("relative_def_path") + } fn variant_kind(&self, def_id: DefId) -> Option { bug!("variant_kind") } fn struct_ctor_def_id(&self, struct_def_id: DefId) -> Option { bug!("struct_ctor_def_id") } diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 0e8bea86178f3..5772d16c6d43d 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -39,7 +39,7 @@ dep_map_ty! { ImplTraitRefs: ItemSignature(DefId) -> Option> dep_map_ty! { TraitDefs: ItemSignature(DefId) -> &'tcx ty::TraitDef<'tcx> } dep_map_ty! { AdtDefs: ItemSignature(DefId) -> ty::AdtDefMaster<'tcx> } dep_map_ty! { ItemVariances: ItemSignature(DefId) -> Rc> } -dep_map_ty! { InherentImpls: InherentImpls(DefId) -> Rc> } +dep_map_ty! { InherentImpls: InherentImpls(DefId) -> Vec } dep_map_ty! { ImplItems: ImplItems(DefId) -> Vec } dep_map_ty! { TraitItems: TraitItems(DefId) -> Rc>> } dep_map_ty! { ReprHints: ReprHints(DefId) -> Rc> } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index dfe24d5627bf1..53838b0760a79 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2437,12 +2437,41 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } } - /// Returns the `DefPath` of an item. Note that if `id` is not - /// local to this crate -- or is inlined into this crate -- the - /// result will be a non-local `DefPath`. + /// Convert a `DefId` into its fully expanded `DefPath` (every + /// `DefId` is really just an interned def-path). + /// + /// Note that if `id` is not local to this crate -- or is + /// inlined into this crate -- the result will be a non-local + /// `DefPath`. + /// + /// This function is only safe to use when you are sure that the + /// full def-path is accessible. Examples that are known to be + /// safe are local def-ids or items; see `opt_def_path` for more + /// details. pub fn def_path(self, id: DefId) -> ast_map::DefPath { + self.opt_def_path(id).unwrap_or_else(|| { + bug!("could not load def-path for {:?}", id) + }) + } + + /// Convert a `DefId` into its fully expanded `DefPath` (every + /// `DefId` is really just an interned def-path). + /// + /// When going across crates, we do not save the full info for + /// every cross-crate def-id, and hence we may not always be able + /// to create a def-path. Therefore, this returns + /// `Option` to cover that possibility. It will always + /// return `Some` for local def-ids, however, as well as for + /// items. The problems arise with "minor" def-ids like those + /// associated with a pattern, `impl Trait`, or other internal + /// detail to a fn. + /// + /// Note that if `id` is not local to this crate -- or is + /// inlined into this crate -- the result will be a non-local + /// `DefPath`. + pub fn opt_def_path(self, id: DefId) -> Option { if id.is_local() { - self.map.def_path(id) + Some(self.map.def_path(id)) } else { self.sess.cstore.relative_def_path(id) } @@ -2665,7 +2694,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { self.impl_items.borrow_mut().insert(impl_def_id, impl_items); } - self.inherent_impls.borrow_mut().insert(type_id, Rc::new(inherent_impls)); + self.inherent_impls.borrow_mut().insert(type_id, inherent_impls); self.populated_external_types.borrow_mut().insert(type_id); } diff --git a/src/librustc_incremental/assert_dep_graph.rs b/src/librustc_incremental/assert_dep_graph.rs index bd96ae69ffbc8..b28454cddb247 100644 --- a/src/librustc_incremental/assert_dep_graph.rs +++ b/src/librustc_incremental/assert_dep_graph.rs @@ -26,19 +26,20 @@ //! used to check when paths exist or do not. //! //! The full form of the `rustc_if_this_changed` annotation is -//! `#[rustc_if_this_changed(id)]`. The `"id"` is optional and -//! defaults to `"id"` if omitted. +//! `#[rustc_if_this_changed("foo")]`, which will report a +//! source node of `foo(def_id)`. The `"foo"` is optional and +//! defaults to `"Hir"` if omitted. //! //! Example: //! //! ``` -//! #[rustc_if_this_changed] +//! #[rustc_if_this_changed(Hir)] //! fn foo() { } //! -//! #[rustc_then_this_would_need("trans")] //~ ERROR no path from `foo` +//! #[rustc_then_this_would_need(trans)] //~ ERROR no path from `foo` //! fn bar() { } //! -//! #[rustc_then_this_would_need("trans")] //~ ERROR OK +//! #[rustc_then_this_would_need(trans)] //~ ERROR OK //! fn baz() { foo(); } //! ``` @@ -47,7 +48,7 @@ use rustc::dep_graph::{DepGraphQuery, DepNode}; use rustc::dep_graph::debug::{DepNodeFilter, EdgeFilter}; use rustc::hir::def_id::DefId; use rustc::ty::TyCtxt; -use rustc_data_structures::fnv::{FnvHashMap, FnvHashSet}; +use rustc_data_structures::fnv::FnvHashSet; use rustc_data_structures::graph::{Direction, INCOMING, OUTGOING, NodeIndex}; use rustc::hir; use rustc::hir::intravisit::Visitor; @@ -61,7 +62,6 @@ use syntax_pos::Span; const IF_THIS_CHANGED: &'static str = "rustc_if_this_changed"; const THEN_THIS_WOULD_NEED: &'static str = "rustc_then_this_would_need"; -const ID: &'static str = "id"; pub fn assert_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let _ignore = tcx.dep_graph.in_ignore(); @@ -80,8 +80,9 @@ pub fn assert_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { // Find annotations supplied by user (if any). let (if_this_changed, then_this_would_need) = { let mut visitor = IfThisChanged { tcx: tcx, - if_this_changed: FnvHashMap(), - then_this_would_need: FnvHashMap() }; + if_this_changed: vec![], + then_this_would_need: vec![] }; + visitor.process_attrs(ast::CRATE_NODE_ID, &tcx.map.krate().attrs); tcx.map.krate().visit_all_items(&mut visitor); (visitor.if_this_changed, visitor.then_this_would_need) }; @@ -97,58 +98,51 @@ pub fn assert_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { check_paths(tcx, &if_this_changed, &then_this_would_need); } -type SourceHashMap = - FnvHashMap)>>; -type TargetHashMap = - FnvHashMap)>>; +type Sources = Vec<(Span, DefId, DepNode)>; +type Targets = Vec<(Span, InternedString, ast::NodeId, DepNode)>; struct IfThisChanged<'a, 'tcx:'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, - if_this_changed: SourceHashMap, - then_this_would_need: TargetHashMap, + if_this_changed: Sources, + then_this_would_need: Targets, } impl<'a, 'tcx> IfThisChanged<'a, 'tcx> { - fn process_attrs(&mut self, node_id: ast::NodeId, def_id: DefId) { - for attr in self.tcx.get_attrs(def_id).iter() { + fn argument(&self, attr: &ast::Attribute) -> Option { + let mut value = None; + for list_item in attr.meta_item_list().unwrap_or_default() { + match list_item.word() { + Some(word) if value.is_none() => + value = Some(word.name().clone()), + _ => + // FIXME better-encapsulate meta_item (don't directly access `node`) + span_bug!(list_item.span(), "unexpected meta-item {:?}", list_item.node), + } + } + value + } + + fn process_attrs(&mut self, node_id: ast::NodeId, attrs: &[ast::Attribute]) { + let def_id = self.tcx.map.local_def_id(node_id); + for attr in attrs { if attr.check_name(IF_THIS_CHANGED) { - let mut id = None; - for list_item in attr.meta_item_list().unwrap_or_default() { - match list_item.word() { - Some(word) if id.is_none() => { - id = Some(word.name().clone()) - }, - _ => { - // FIXME better-encapsulate meta_item (don't directly access `node`) - span_bug!(list_item.span(), "unexpected list-item {:?}", list_item.node) + let dep_node_interned = self.argument(attr); + let dep_node = match dep_node_interned { + None => DepNode::Hir(def_id), + Some(ref n) => { + match DepNode::from_label_string(&n[..], def_id) { + Ok(n) => n, + Err(()) => { + self.tcx.sess.span_fatal( + attr.span, + &format!("unrecognized DepNode variant {:?}", n)); + } } } - } - - let id = id.unwrap_or(InternedString::new(ID)); - self.if_this_changed.entry(id) - .or_insert(FnvHashSet()) - .insert((attr.span, def_id, DepNode::Hir(def_id))); + }; + self.if_this_changed.push((attr.span, def_id, dep_node)); } else if attr.check_name(THEN_THIS_WOULD_NEED) { - let mut dep_node_interned = None; - let mut id = None; - for list_item in attr.meta_item_list().unwrap_or_default() { - match list_item.word() { - Some(word) if dep_node_interned.is_none() => { - dep_node_interned = Some(word.name().clone()); - }, - Some(word) if id.is_none() => { - id = Some(word.name().clone()) - }, - _ => { - // FIXME better-encapsulate meta_item (don't directly access `node`) - span_bug!(list_item.span(), "unexpected meta-item {:?}", list_item.node) - } - } - } - + let dep_node_interned = self.argument(attr); let dep_node = match dep_node_interned { Some(ref n) => { match DepNode::from_label_string(&n[..], def_id) { @@ -166,11 +160,10 @@ impl<'a, 'tcx> IfThisChanged<'a, 'tcx> { &format!("missing DepNode variant")); } }; - let id = id.unwrap_or(InternedString::new(ID)); - self.then_this_would_need - .entry(id) - .or_insert(FnvHashSet()) - .insert((attr.span, dep_node_interned.clone().unwrap(), node_id, dep_node)); + self.then_this_would_need.push((attr.span, + dep_node_interned.clone().unwrap(), + node_id, + dep_node)); } } } @@ -178,47 +171,38 @@ impl<'a, 'tcx> IfThisChanged<'a, 'tcx> { impl<'a, 'tcx> Visitor<'tcx> for IfThisChanged<'a, 'tcx> { fn visit_item(&mut self, item: &'tcx hir::Item) { - let def_id = self.tcx.map.local_def_id(item.id); - self.process_attrs(item.id, def_id); + self.process_attrs(item.id, &item.attrs); } } fn check_paths<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - if_this_changed: &SourceHashMap, - then_this_would_need: &TargetHashMap) + if_this_changed: &Sources, + then_this_would_need: &Targets) { // Return early here so as not to construct the query, which is not cheap. if if_this_changed.is_empty() { + for &(target_span, _, _, _) in then_this_would_need { + tcx.sess.span_err( + target_span, + &format!("no #[rustc_if_this_changed] annotation detected")); + + } return; } let query = tcx.dep_graph.query(); - for (id, sources) in if_this_changed { - let targets = match then_this_would_need.get(id) { - Some(targets) => targets, - None => { - for &(source_span, ..) in sources.iter().take(1) { - tcx.sess.span_err( - source_span, - &format!("no targets for id `{}`", id)); - } - continue; - } - }; - - for &(_, source_def_id, ref source_dep_node) in sources { - let dependents = query.transitive_successors(source_dep_node); - for &(target_span, ref target_pass, _, ref target_dep_node) in targets { - if !dependents.contains(&target_dep_node) { - tcx.sess.span_err( - target_span, - &format!("no path from `{}` to `{}`", - tcx.item_path_str(source_def_id), - target_pass)); - } else { - tcx.sess.span_err( - target_span, - &format!("OK")); - } + for &(_, source_def_id, ref source_dep_node) in if_this_changed { + let dependents = query.transitive_successors(source_dep_node); + for &(target_span, ref target_pass, _, ref target_dep_node) in then_this_would_need { + if !dependents.contains(&target_dep_node) { + tcx.sess.span_err( + target_span, + &format!("no path from `{}` to `{}`", + tcx.item_path_str(source_def_id), + target_pass)); + } else { + tcx.sess.span_err( + target_span, + &format!("OK")); } } } diff --git a/src/librustc_metadata/common.rs b/src/librustc_metadata/common.rs index 1e6c74bef8da5..29b9cc0d1d923 100644 --- a/src/librustc_metadata/common.rs +++ b/src/librustc_metadata/common.rs @@ -149,9 +149,9 @@ pub const tag_items_data_item_visibility: usize = 0x78; pub const tag_items_data_item_inherent_impl: usize = 0x79; // GAP 0x7a pub const tag_mod_child: usize = 0x7b; -pub const tag_misc_info: usize = 0x108; // top-level only -pub const tag_misc_info_crate_items: usize = 0x7c; +// GAP 0x7c +// GAP 0x108 pub const tag_impls: usize = 0x109; // top-level only pub const tag_impls_trait: usize = 0x7d; pub const tag_impls_trait_impl: usize = 0x7e; diff --git a/src/librustc_metadata/csearch.rs b/src/librustc_metadata/csearch.rs index 0fd7b683067b7..d7ca93235fddb 100644 --- a/src/librustc_metadata/csearch.rs +++ b/src/librustc_metadata/csearch.rs @@ -425,13 +425,21 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore { /// parent `DefId` as well as some idea of what kind of data the /// `DefId` refers to. fn def_key(&self, def: DefId) -> hir_map::DefKey { - self.dep_graph.read(DepNode::MetaData(def)); + // Note: loading the def-key (or def-path) for a def-id is not + // a *read* of its metadata. This is because the def-id is + // really just an interned shorthand for a def-path, which is the + // canonical name for an item. + // + // self.dep_graph.read(DepNode::MetaData(def)); let cdata = self.get_crate_data(def.krate); decoder::def_key(&cdata, def.index) } - fn relative_def_path(&self, def: DefId) -> hir_map::DefPath { - self.dep_graph.read(DepNode::MetaData(def)); + fn relative_def_path(&self, def: DefId) -> Option { + // See `Note` above in `def_key()` for why this read is + // commented out: + // + // self.dep_graph.read(DepNode::MetaData(def)); let cdata = self.get_crate_data(def.krate); decoder::def_path(&cdata, def.index) } diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 6b48b4dfabcfd..5bad89f1a593f 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -23,6 +23,7 @@ use index; use tls_context; use tydecode::TyDecoder; +use rustc::hir::def_id::CRATE_DEF_INDEX; use rustc::hir::svh::Svh; use rustc::hir::map as hir_map; use rustc::hir::map::DefKey; @@ -732,15 +733,7 @@ pub fn each_top_level_item_of_crate(cdata: Cmd, get_crate_data: G, callbac where F: FnMut(DefLike, ast::Name, ty::Visibility), G: FnMut(ast::CrateNum) -> Rc, { - let root_doc = rbml::Doc::new(cdata.data()); - let misc_info_doc = reader::get_doc(root_doc, tag_misc_info); - let crate_items_doc = reader::get_doc(misc_info_doc, - tag_misc_info_crate_items); - - each_child_of_item_or_crate(cdata, - crate_items_doc, - get_crate_data, - callback) + each_child_of_item(cdata, CRATE_DEF_INDEX, get_crate_data, callback) } pub fn get_item_name(cdata: Cmd, id: DefIndex) -> ast::Name { @@ -766,7 +759,7 @@ pub fn maybe_get_item_ast<'a, 'tcx>(cdata: Cmd, tcx: TyCtxt<'a, 'tcx, 'tcx>, id: krate: cdata.cnum, index: def_key(cdata, id).parent.unwrap() }; - let mut parent_def_path = def_path(cdata, id); + let mut parent_def_path = def_path(cdata, id).unwrap(); parent_def_path.data.pop(); if let Some(ast_doc) = reader::maybe_get_doc(item_doc, tag_ast as usize) { let ii = decode_inlined_item(cdata, @@ -1633,9 +1626,16 @@ fn item_def_key(item_doc: rbml::Doc) -> hir_map::DefKey { } } -pub fn def_path(cdata: Cmd, id: DefIndex) -> hir_map::DefPath { +// Returns the path leading to the thing with this `id`. Note that +// some def-ids don't wind up in the metadata, so `def_path` sometimes +// returns `None` +pub fn def_path(cdata: Cmd, id: DefIndex) -> Option { debug!("def_path(id={:?})", id); - hir_map::DefPath::make(cdata.cnum, id, |parent| def_key(cdata, parent)) + if cdata.get_item(id).is_some() { + Some(hir_map::DefPath::make(cdata.cnum, id, |parent| def_key(cdata, parent))) + } else { + None + } } pub fn get_panic_strategy(data: &[u8]) -> PanicStrategy { diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 23398a0400c51..603b7a483b90c 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -1693,30 +1693,6 @@ fn encode_impls<'a>(ecx: &'a EncodeContext, rbml_w.end_tag(); } -fn encode_misc_info(ecx: &EncodeContext, - krate: &hir::Crate, - rbml_w: &mut Encoder) { - rbml_w.start_tag(tag_misc_info); - rbml_w.start_tag(tag_misc_info_crate_items); - for item_id in &krate.module.item_ids { - rbml_w.wr_tagged_u64(tag_mod_child, - def_to_u64(ecx.tcx.map.local_def_id(item_id.id))); - - let item = ecx.tcx.map.expect_item(item_id.id); - each_auxiliary_node_id(item, |auxiliary_node_id| { - rbml_w.wr_tagged_u64(tag_mod_child, - def_to_u64(ecx.tcx.map.local_def_id(auxiliary_node_id))); - true - }); - } - - // Encode reexports for the root module. - encode_reexports(ecx, rbml_w, 0); - - rbml_w.end_tag(); - rbml_w.end_tag(); -} - // Encodes all reachable symbols in this crate into the metadata. // // This pass is seeded off the reachability list calculated in the @@ -1861,7 +1837,7 @@ fn encode_metadata_inner(rbml_w: &mut Encoder, codemap_bytes: u64, macro_defs_bytes: u64, impl_bytes: u64, - misc_bytes: u64, + reachable_bytes: u64, item_bytes: u64, index_bytes: u64, xref_bytes: u64, @@ -1877,7 +1853,7 @@ fn encode_metadata_inner(rbml_w: &mut Encoder, codemap_bytes: 0, macro_defs_bytes: 0, impl_bytes: 0, - misc_bytes: 0, + reachable_bytes: 0, item_bytes: 0, index_bytes: 0, xref_bytes: 0, @@ -1931,11 +1907,10 @@ fn encode_metadata_inner(rbml_w: &mut Encoder, encode_impls(&ecx, krate, rbml_w); stats.impl_bytes = rbml_w.writer.seek(SeekFrom::Current(0)).unwrap() - i; - // Encode miscellaneous info. + // Encode reachability info. i = rbml_w.writer.seek(SeekFrom::Current(0)).unwrap(); - encode_misc_info(&ecx, krate, rbml_w); encode_reachable(&ecx, rbml_w); - stats.misc_bytes = rbml_w.writer.seek(SeekFrom::Current(0)).unwrap() - i; + stats.reachable_bytes = rbml_w.writer.seek(SeekFrom::Current(0)).unwrap() - i; // Encode and index the items. rbml_w.start_tag(tag_items); @@ -1972,7 +1947,7 @@ fn encode_metadata_inner(rbml_w: &mut Encoder, println!(" codemap bytes: {}", stats.codemap_bytes); println!(" macro def bytes: {}", stats.macro_defs_bytes); println!(" impl bytes: {}", stats.impl_bytes); - println!(" misc bytes: {}", stats.misc_bytes); + println!(" reachable bytes: {}", stats.reachable_bytes); println!(" item bytes: {}", stats.item_bytes); println!(" index bytes: {}", stats.index_bytes); println!(" xref bytes: {}", stats.xref_bytes); diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index d2b7f07b9ce6c..57d3c46af9306 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -32,10 +32,7 @@ use rustc::ty::util::CopyImplementationError; use middle::free_region::FreeRegionMap; use CrateCtxt; use rustc::infer::{self, InferCtxt, TypeOrigin}; -use std::cell::RefCell; -use std::rc::Rc; use syntax_pos::Span; -use util::nodemap::{DefIdMap, FnvHashMap}; use rustc::dep_graph::DepNode; use rustc::hir::map as hir_map; use rustc::hir::intravisit; @@ -49,7 +46,6 @@ mod unsafety; struct CoherenceChecker<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { crate_context: &'a CrateCtxt<'a, 'gcx>, inference_context: InferCtxt<'a, 'gcx, 'tcx>, - inherent_impls: RefCell>>>>, } struct CoherenceCheckVisitor<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { @@ -109,15 +105,6 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { DepNode::CoherenceCheckImpl, &mut CoherenceCheckVisitor { cc: self }); - // Copy over the inherent impls we gathered up during the walk into - // the tcx. - let mut tcx_inherent_impls = - self.crate_context.tcx.inherent_impls.borrow_mut(); - for (k, v) in self.inherent_impls.borrow().iter() { - tcx_inherent_impls.insert((*k).clone(), - Rc::new((*v.borrow()).clone())); - } - // Populate the table of destructors. It might seem a bit strange to // do this here, but it's actually the most convenient place, since // the coherence tables contain the trait -> type mappings. @@ -175,14 +162,8 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { } fn add_inherent_impl(&self, base_def_id: DefId, impl_def_id: DefId) { - if let Some(implementation_list) = self.inherent_impls.borrow().get(&base_def_id) { - implementation_list.borrow_mut().push(impl_def_id); - return; - } - - self.inherent_impls.borrow_mut().insert( - base_def_id, - Rc::new(RefCell::new(vec!(impl_def_id)))); + let tcx = self.crate_context.tcx; + tcx.inherent_impls.borrow_mut().push(base_def_id, impl_def_id); } fn add_trait_impl(&self, impl_trait_ref: ty::TraitRef<'gcx>, impl_def_id: DefId) { @@ -556,7 +537,6 @@ pub fn check_coherence(ccx: &CrateCtxt) { CoherenceChecker { crate_context: ccx, inference_context: infcx, - inherent_impls: RefCell::new(FnvHashMap()), }.check(); }); unsafety::check(ccx.tcx); diff --git a/src/test/incremental/krate-inherent.rs b/src/test/incremental/krate-inherent.rs new file mode 100644 index 0000000000000..ac6cc3e9826f1 --- /dev/null +++ b/src/test/incremental/krate-inherent.rs @@ -0,0 +1,34 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// revisions: rpass1 rpass2 +// compile-flags: -Z query-dep-graph + +#![allow(warnings)] +#![feature(rustc_attrs)] +#![rustc_partition_reused(module="krate_inherent-x", cfg="rpass2")] + +fn main() { } + +mod x { + struct Foo; + impl Foo { + fn foo(&self) { } + } + + fn method() { + let x: Foo = Foo; + x.foo(); // inherent methods used to add an edge from Krate + } +} + +#[cfg(rpass1)] +fn bar() { } // remove this unrelated fn in rpass2, which should not affect `x::method` + diff --git a/src/test/incremental/krate-inlined.rs b/src/test/incremental/krate-inlined.rs new file mode 100644 index 0000000000000..ba32b41983fc2 --- /dev/null +++ b/src/test/incremental/krate-inlined.rs @@ -0,0 +1,31 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Regr. test that using HIR inlined from another krate does *not* add +// a dependency from the local Krate node. + +// revisions: cfail1 +// compile-flags: -Z query-dep-graph + +#![allow(warnings)] +#![feature(rustc_attrs)] + +#![rustc_if_this_changed(Krate)] + +fn main() { } + +mod x { + #[rustc_then_this_would_need(TransCrateItem)] //[cfail1]~ ERROR no path + fn method() { + // use some methods that require inlining HIR from another crate: + let mut v = vec![]; + v.push(1); + } +} diff --git a/src/test/incremental/remove-private-item-cross-crate/auxiliary/a.rs b/src/test/incremental/remove-private-item-cross-crate/auxiliary/a.rs new file mode 100644 index 0000000000000..4d84e844dedbb --- /dev/null +++ b/src/test/incremental/remove-private-item-cross-crate/auxiliary/a.rs @@ -0,0 +1,18 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(warnings)] +#![crate_name = "a"] +#![crate_type = "rlib"] + +pub fn foo(b: u8) -> u32 { b as u32 } + +#[cfg(rpass1)] +fn bar() { } diff --git a/src/test/incremental/remove-private-item-cross-crate/main.rs b/src/test/incremental/remove-private-item-cross-crate/main.rs new file mode 100644 index 0000000000000..582ee905d7ca4 --- /dev/null +++ b/src/test/incremental/remove-private-item-cross-crate/main.rs @@ -0,0 +1,28 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we are able to reuse `main` even though a private +// item was removed from the root module of crate`a`. + +// revisions:rpass1 rpass2 +// aux-build:a.rs + +#![feature(rustc_attrs)] +#![crate_type = "bin"] +#![rustc_partition_reused(module="main", cfg="rpass2")] + +extern crate a; + +pub fn main() { + let vec: Vec = vec![0, 1, 2, 3]; + for &b in &vec { + println!("{}", a::foo(b)); + } +}