allow testing DepNode::Krate edges directly

Author: nikomatsakis

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<InternedString,
-               FnvHashSet<(Span, DefId, DepNode<DefId>)>>;
-type TargetHashMap =
-    FnvHashMap<InternedString,
-               FnvHashSet<(Span, InternedString, ast::NodeId, DepNode<DefId>)>>;
+type Sources = Vec<(Span, DefId, DepNode<DefId>)>;
+type Targets = Vec<(Span, InternedString, ast::NodeId, DepNode<DefId>)>;
 
 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<InternedString> {
+        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,59 +160,49 @@ 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));
             }
         }
     }
 }
 
 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"));
             }
         }
     }

src/test/incremental/krate-inlined.rs

@@ -8,22 +8,24 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// revisions: rpass1 rpass2
+// 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_partition_reused(module="krate_inlined-x", cfg="rpass2")]
+
+#![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);
     }
 }
-
-#[cfg(rpass1)]
-fn bar() { } // remove this unrelated fn in rpass2, which should not affect `x::method`