Skip to content

Commit 14191ee

Browse files
michaelwoeristermichaelwoerister
committed
Let the ICH testing framework check that all #[rustc_dirty] attrs have been actually checked.
1 parent aed6410 commit 14191ee

File tree

7 files changed

+300
-58
lines changed

7 files changed

+300
-58
lines changed

‎src/librustc/hir/intravisit.rs

+1
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,7 @@ pub fn walk_decl<'v, V: Visitor<'v>>(visitor: &mut V, declaration: &'v Decl) {
914914

915915
pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
916916
visitor.visit_id(expression.id);
917+
walk_list!(visitor, visit_attribute, expression.attrs.iter());
917918
match expression.node {
918919
ExprBox(ref subexpression) => {
919920
visitor.visit_expr(subexpression)

‎src/librustc_incremental/persist/dirty_clean.rs

+116-31
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ use rustc::dep_graph::{DepGraphQuery, DepNode};
4646
use rustc::hir;
4747
use rustc::hir::def_id::DefId;
4848
use rustc::hir::itemlikevisit::ItemLikeVisitor;
49+
use rustc::hir::intravisit;
4950
use syntax::ast::{self, Attribute, NestedMetaItem};
5051
use rustc_data_structures::fx::{FxHashSet, FxHashMap};
5152
use syntax_pos::Span;
@@ -73,17 +74,29 @@ pub fn check_dirty_clean_annotations<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
7374
let query = tcx.dep_graph.query();
7475
debug!("query-nodes: {:?}", query.nodes());
7576
let krate = tcx.hir.krate();
76-
krate.visit_all_item_likes(&mut DirtyCleanVisitor {
77+
let mut dirty_clean_visitor = DirtyCleanVisitor {
7778
tcx: tcx,
7879
query: &query,
7980
dirty_inputs: dirty_inputs,
80-
});
81+
checked_attrs: FxHashSet(),
82+
};
83+
krate.visit_all_item_likes(&mut dirty_clean_visitor);
84+
85+
let mut all_attrs = FindAllAttrs {
86+
tcx: tcx,
87+
attr_names: vec![ATTR_DIRTY, ATTR_CLEAN],
88+
found_attrs: vec![],
89+
};
90+
intravisit::walk_crate(&mut all_attrs, krate);
91+
92+
all_attrs.report_unchecked_attrs(&dirty_clean_visitor.checked_attrs);
8193
}
8294

8395
pub struct DirtyCleanVisitor<'a, 'tcx:'a> {
8496
tcx: TyCtxt<'a, 'tcx, 'tcx>,
8597
query: &'a DepGraphQuery<DefId>,
8698
dirty_inputs: FxHashSet<DepNode<DefId>>,
99+
checked_attrs: FxHashSet<ast::AttrId>,
87100
}
88101

89102
impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
@@ -109,7 +122,7 @@ impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
109122
dep_node.map_def(|&def_id| Some(self.tcx.item_path_str(def_id))).unwrap()
110123
}
111124

112-
fn assert_dirty(&self, item: &hir::Item, dep_node: DepNode<DefId>) {
125+
fn assert_dirty(&self, item_span: Span, dep_node: DepNode<DefId>) {
113126
debug!("assert_dirty({:?})", dep_node);
114127

115128
match dep_node {
@@ -121,7 +134,7 @@ impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
121134
if !self.dirty_inputs.contains(&dep_node) {
122135
let dep_node_str = self.dep_node_str(&dep_node);
123136
self.tcx.sess.span_err(
124-
item.span,
137+
item_span,
125138
&format!("`{:?}` not found in dirty set, but should be dirty",
126139
dep_node_str));
127140
}
@@ -132,14 +145,14 @@ impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
132145
if self.query.contains_node(&dep_node) {
133146
let dep_node_str = self.dep_node_str(&dep_node);
134147
self.tcx.sess.span_err(
135-
item.span,
148+
item_span,
136149
&format!("`{:?}` found in dep graph, but should be dirty", dep_node_str));
137150
}
138151
}
139152
}
140153
}
141154

142-
fn assert_clean(&self, item: &hir::Item, dep_node: DepNode<DefId>) {
155+
fn assert_clean(&self, item_span: Span, dep_node: DepNode<DefId>) {
143156
debug!("assert_clean({:?})", dep_node);
144157

145158
match dep_node {
@@ -150,7 +163,7 @@ impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
150163
if self.dirty_inputs.contains(&dep_node) {
151164
let dep_node_str = self.dep_node_str(&dep_node);
152165
self.tcx.sess.span_err(
153-
item.span,
166+
item_span,
154167
&format!("`{:?}` found in dirty-node set, but should be clean",
155168
dep_node_str));
156169
}
@@ -160,35 +173,43 @@ impl<'a, 'tcx> DirtyCleanVisitor<'a, 'tcx> {
160173
if !self.query.contains_node(&dep_node) {
161174
let dep_node_str = self.dep_node_str(&dep_node);
162175
self.tcx.sess.span_err(
163-
item.span,
176+
item_span,
164177
&format!("`{:?}` not found in dep graph, but should be clean",
165178
dep_node_str));
166179
}
167180
}
168181
}
169182
}
170-
}
171183

172-
impl<'a, 'tcx> ItemLikeVisitor<'tcx> for DirtyCleanVisitor<'a, 'tcx> {
173-
fn visit_item(&mut self, item: &'tcx hir::Item) {
174-
let def_id = self.tcx.hir.local_def_id(item.id);
184+
fn check_item(&mut self, item_id: ast::NodeId, item_span: Span) {
185+
let def_id = self.tcx.hir.local_def_id(item_id);
175186
for attr in self.tcx.get_attrs(def_id).iter() {
176187
if attr.check_name(ATTR_DIRTY) {
177188
if check_config(self.tcx, attr) {
178-
self.assert_dirty(item, self.dep_node(attr, def_id));
189+
self.checked_attrs.insert(attr.id);
190+
self.assert_dirty(item_span, self.dep_node(attr, def_id));
179191
}
180192
} else if attr.check_name(ATTR_CLEAN) {
181193
if check_config(self.tcx, attr) {
182-
self.assert_clean(item, self.dep_node(attr, def_id));
194+
self.checked_attrs.insert(attr.id);
195+
self.assert_clean(item_span, self.dep_node(attr, def_id));
183196
}
184197
}
185198
}
186199
}
200+
}
187201

188-
fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem) {
202+
impl<'a, 'tcx> ItemLikeVisitor<'tcx> for DirtyCleanVisitor<'a, 'tcx> {
203+
fn visit_item(&mut self, item: &'tcx hir::Item) {
204+
self.check_item(item.id, item.span);
189205
}
190206

191-
fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) {
207+
fn visit_trait_item(&mut self, item: &hir::TraitItem) {
208+
self.check_item(item.id, item.span);
209+
}
210+
211+
fn visit_impl_item(&mut self, item: &hir::ImplItem) {
212+
self.check_item(item.id, item.span);
192213
}
193214
}
194215

@@ -201,46 +222,66 @@ pub fn check_dirty_clean_metadata<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
201222

202223
tcx.dep_graph.with_ignore(||{
203224
let krate = tcx.hir.krate();
204-
krate.visit_all_item_likes(&mut DirtyCleanMetadataVisitor {
225+
let mut dirty_clean_visitor = DirtyCleanMetadataVisitor {
205226
tcx: tcx,
206227
prev_metadata_hashes: prev_metadata_hashes,
207228
current_metadata_hashes: current_metadata_hashes,
208-
});
229+
checked_attrs: FxHashSet(),
230+
};
231+
krate.visit_all_item_likes(&mut dirty_clean_visitor);
232+
233+
let mut all_attrs = FindAllAttrs {
234+
tcx: tcx,
235+
attr_names: vec![ATTR_DIRTY_METADATA, ATTR_CLEAN_METADATA],
236+
found_attrs: vec![],
237+
};
238+
intravisit::walk_crate(&mut all_attrs, krate);
239+
240+
all_attrs.report_unchecked_attrs(&dirty_clean_visitor.checked_attrs);
209241
});
210242
}
211243

212244
pub struct DirtyCleanMetadataVisitor<'a, 'tcx:'a, 'm> {
213245
tcx: TyCtxt<'a, 'tcx, 'tcx>,
214246
prev_metadata_hashes: &'m FxHashMap<DefId, Fingerprint>,
215247
current_metadata_hashes: &'m FxHashMap<DefId, Fingerprint>,
248+
checked_attrs: FxHashSet<ast::AttrId>,
216249
}
217250

218251
impl<'a, 'tcx, 'm> ItemLikeVisitor<'tcx> for DirtyCleanMetadataVisitor<'a, 'tcx, 'm> {
219252
fn visit_item(&mut self, item: &'tcx hir::Item) {
220-
let def_id = self.tcx.hir.local_def_id(item.id);
253+
self.check_item(item.id, item.span);
254+
}
255+
256+
fn visit_trait_item(&mut self, item: &hir::TraitItem) {
257+
self.check_item(item.id, item.span);
258+
}
259+
260+
fn visit_impl_item(&mut self, item: &hir::ImplItem) {
261+
self.check_item(item.id, item.span);
262+
}
263+
}
264+
265+
impl<'a, 'tcx, 'm> DirtyCleanMetadataVisitor<'a, 'tcx, 'm> {
266+
267+
fn check_item(&mut self, item_id: ast::NodeId, item_span: Span) {
268+
let def_id = self.tcx.hir.local_def_id(item_id);
221269

222270
for attr in self.tcx.get_attrs(def_id).iter() {
223271
if attr.check_name(ATTR_DIRTY_METADATA) {
224272
if check_config(self.tcx, attr) {
225-
self.assert_state(false, def_id, item.span);
273+
self.checked_attrs.insert(attr.id);
274+
self.assert_state(false, def_id, item_span);
226275
}
227276
} else if attr.check_name(ATTR_CLEAN_METADATA) {
228277
if check_config(self.tcx, attr) {
229-
self.assert_state(true, def_id, item.span);
278+
self.checked_attrs.insert(attr.id);
279+
self.assert_state(true, def_id, item_span);
230280
}
231281
}
232282
}
233283
}
234284

235-
fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem) {
236-
}
237-
238-
fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) {
239-
}
240-
}
241-
242-
impl<'a, 'tcx, 'm> DirtyCleanMetadataVisitor<'a, 'tcx, 'm> {
243-
244285
fn assert_state(&self, should_be_clean: bool, def_id: DefId, span: Span) {
245286
let item_path = self.tcx.item_path_str(def_id);
246287
debug!("assert_state({})", item_path);
@@ -274,7 +315,7 @@ impl<'a, 'tcx, 'm> DirtyCleanMetadataVisitor<'a, 'tcx, 'm> {
274315
/// Given a `#[rustc_dirty]` or `#[rustc_clean]` attribute, scan
275316
/// for a `cfg="foo"` attribute and check whether we have a cfg
276317
/// flag called `foo`.
277-
fn check_config(tcx: TyCtxt, attr: &ast::Attribute) -> bool {
318+
fn check_config(tcx: TyCtxt, attr: &Attribute) -> bool {
278319
debug!("check_config(attr={:?})", attr);
279320
let config = &tcx.sess.parse_sess.config;
280321
debug!("check_config: config={:?}", config);
@@ -304,3 +345,47 @@ fn expect_associated_value(tcx: TyCtxt, item: &NestedMetaItem) -> ast::Name {
304345
tcx.sess.span_fatal(item.span, &msg);
305346
}
306347
}
348+
349+
350+
// A visitor that collects all #[rustc_dirty]/#[rustc_clean] attributes from
351+
// the HIR. It is used to verfiy that we really ran checks for all annotated
352+
// nodes.
353+
pub struct FindAllAttrs<'a, 'tcx:'a> {
354+
tcx: TyCtxt<'a, 'tcx, 'tcx>,
355+
attr_names: Vec<&'static str>,
356+
found_attrs: Vec<&'tcx Attribute>,
357+
}
358+
359+
impl<'a, 'tcx> FindAllAttrs<'a, 'tcx> {
360+
361+
fn is_active_attr(&mut self, attr: &Attribute) -> bool {
362+
for attr_name in &self.attr_names {
363+
if attr.check_name(attr_name) && check_config(self.tcx, attr) {
364+
return true;
365+
}
366+
}
367+
368+
false
369+
}
370+
371+
fn report_unchecked_attrs(&self, checked_attrs: &FxHashSet<ast::AttrId>) {
372+
for attr in &self.found_attrs {
373+
if !checked_attrs.contains(&attr.id) {
374+
self.tcx.sess.span_err(attr.span, &format!("found unchecked \
375+
#[rustc_dirty]/#[rustc_clean] attribute"));
376+
}
377+
}
378+
}
379+
}
380+
381+
impl<'a, 'tcx> intravisit::Visitor<'tcx> for FindAllAttrs<'a, 'tcx> {
382+
fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> {
383+
intravisit::NestedVisitorMap::All(&self.tcx.hir)
384+
}
385+
386+
fn visit_attribute(&mut self, attr: &'tcx Attribute) {
387+
if self.is_active_attr(attr) {
388+
self.found_attrs.push(attr);
389+
}
390+
}
391+
}

0 commit comments

Comments
 (0)