From 2a84449169b3c882e101a68eb156800fe8ff24c3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 24 Aug 2016 11:00:55 -0400 Subject: [PATCH] allow testing DepNode::Krate edges directly --- src/librustc_incremental/assert_dep_graph.rs | 158 +++++++++---------- src/test/incremental/krate-inlined.rs | 12 +- 2 files changed, 78 insertions(+), 92 deletions(-) diff --git a/src/librustc_incremental/assert_dep_graph.rs b/src/librustc_incremental/assert_dep_graph.rs index bd96ae69ffbc..b28454cddb24 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/test/incremental/krate-inlined.rs b/src/test/incremental/krate-inlined.rs index 368065898479..ba32b41983fc 100644 --- a/src/test/incremental/krate-inlined.rs +++ b/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`