Skip to content

Commit

Permalink
Auto merge of #92179 - Aaron1011:incr-loaded-from-disk, r=michaelwoer…
Browse files Browse the repository at this point in the history
…ister

Add `#[rustc_clean(loaded_from_disk)]` to assert loading of query result

Currently, you can use `#[rustc_clean]` to assert to that a particular
query (technically, a `DepNode`) is green or red. However, a green
`DepNode` does not mean that the query result was actually deserialized
from disk - we might have never re-run a query that needed the result.

Some incremental tests are written as regression tests for ICEs that
occured during query result decoding. Using
`#[rustc_clean(loaded_from_disk="typeck")]`, you can now assert
that the result of a particular query (e.g. `typeck`) was actually
loaded from disk, in addition to being green.
  • Loading branch information
bors committed Jan 3, 2022
2 parents d367c34 + f1d6823 commit b5efe57
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 3 deletions.
42 changes: 40 additions & 2 deletions compiler/rustc_incremental/src/persist/dirty_clean.rs
Expand Up @@ -9,6 +9,13 @@
//! - `#[rustc_clean(cfg="rev2")]` same as above, except that the
//! fingerprints must be the SAME (along with all other fingerprints).
//!
//! - `#[rustc_clean(cfg="rev2", loaded_from_disk='typeck")]` asserts that
//! the query result for `DepNode::typeck(X)` was actually
//! loaded from disk (not just marked green). This can be useful
//! to ensure that a test is actually exercising the deserialization
//! logic for a particular query result. This can be combined with
//! `except`
//!
//! Errors are reported if we are in the suitable configuration but
//! the required condition is not met.

Expand All @@ -28,6 +35,7 @@ use rustc_span::Span;
use std::iter::FromIterator;
use std::vec::Vec;

const LOADED_FROM_DISK: Symbol = sym::loaded_from_disk;
const EXCEPT: Symbol = sym::except;
const CFG: Symbol = sym::cfg;

Expand Down Expand Up @@ -124,6 +132,7 @@ type Labels = FxHashSet<String>;
struct Assertion {
clean: Labels,
dirty: Labels,
loaded_from_disk: Labels,
}

pub fn check_dirty_clean_annotations(tcx: TyCtxt<'_>) {
Expand Down Expand Up @@ -174,6 +183,7 @@ impl<'tcx> DirtyCleanVisitor<'tcx> {
fn assertion_auto(&mut self, item_id: LocalDefId, attr: &Attribute) -> Assertion {
let (name, mut auto) = self.auto_labels(item_id, attr);
let except = self.except(attr);
let loaded_from_disk = self.loaded_from_disk(attr);
for e in except.iter() {
if !auto.remove(e) {
let msg = format!(
Expand All @@ -183,7 +193,19 @@ impl<'tcx> DirtyCleanVisitor<'tcx> {
self.tcx.sess.span_fatal(attr.span, &msg);
}
}
Assertion { clean: auto, dirty: except }
Assertion { clean: auto, dirty: except, loaded_from_disk }
}

/// `loaded_from_disk=` attribute value
fn loaded_from_disk(&self, attr: &Attribute) -> Labels {
for item in attr.meta_item_list().unwrap_or_else(Vec::new) {
if item.has_name(LOADED_FROM_DISK) {
let value = expect_associated_value(self.tcx, &item);
return self.resolve_labels(&item, value);
}
}
// If `loaded_from_disk=` is not specified, don't assert anything
Labels::default()
}

/// `except=` attribute value
Expand Down Expand Up @@ -332,6 +354,18 @@ impl<'tcx> DirtyCleanVisitor<'tcx> {
}
}

fn assert_loaded_from_disk(&self, item_span: Span, dep_node: DepNode) {
debug!("assert_loaded_from_disk({:?})", dep_node);

if !self.tcx.dep_graph.debug_was_loaded_from_disk(dep_node) {
let dep_node_str = self.dep_node_str(&dep_node);
self.tcx.sess.span_err(
item_span,
&format!("`{}` should have been loaded from disk but it was not", dep_node_str),
);
}
}

fn check_item(&mut self, item_id: LocalDefId, item_span: Span) {
let def_path_hash = self.tcx.def_path_hash(item_id.to_def_id());
for attr in self.tcx.get_attrs(item_id.to_def_id()).iter() {
Expand All @@ -348,6 +382,10 @@ impl<'tcx> DirtyCleanVisitor<'tcx> {
let dep_node = DepNode::from_label_string(self.tcx, &label, def_path_hash).unwrap();
self.assert_dirty(item_span, dep_node);
}
for label in assertion.loaded_from_disk {
let dep_node = DepNode::from_label_string(self.tcx, &label, def_path_hash).unwrap();
self.assert_loaded_from_disk(item_span, dep_node);
}
}
}
}
Expand Down Expand Up @@ -382,7 +420,7 @@ fn check_config(tcx: TyCtxt<'_>, attr: &Attribute) -> bool {
let value = expect_associated_value(tcx, &item);
debug!("check_config: searching for cfg {:?}", value);
cfg = Some(config.contains(&(value, None)));
} else if !item.has_name(EXCEPT) {
} else if !(item.has_name(EXCEPT) || item.has_name(LOADED_FROM_DISK)) {
tcx.sess.span_err(attr.span, &format!("unknown item `{}`", item.name_or_empty()));
}
}
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_query_system/src/dep_graph/graph.rs
Expand Up @@ -88,6 +88,11 @@ struct DepGraphData<K: DepKind> {
previous_work_products: FxHashMap<WorkProductId, WorkProduct>,

dep_node_debug: Lock<FxHashMap<DepNode<K>, String>>,

/// Used by incremental compilation tests to assert that
/// a particular query result was decoded from disk
/// (not just marked green)
debug_loaded_from_disk: Lock<FxHashSet<DepNode<K>>>,
}

pub fn hash_result<R>(hcx: &mut StableHashingContext<'_>, result: &R) -> Fingerprint
Expand Down Expand Up @@ -135,6 +140,7 @@ impl<K: DepKind> DepGraph<K> {
processed_side_effects: Default::default(),
previous: prev_graph,
colors: DepNodeColorMap::new(prev_graph_node_count),
debug_loaded_from_disk: Default::default(),
})),
virtual_dep_node_index: Lrc::new(AtomicU32::new(0)),
}
Expand Down Expand Up @@ -438,6 +444,14 @@ impl<K: DepKind> DepGraph<K> {
&self.data.as_ref().unwrap().previous_work_products
}

pub fn mark_debug_loaded_from_disk(&self, dep_node: DepNode<K>) {
self.data.as_ref().unwrap().debug_loaded_from_disk.lock().insert(dep_node);
}

pub fn debug_was_loaded_from_disk(&self, dep_node: DepNode<K>) -> bool {
self.data.as_ref().unwrap().debug_loaded_from_disk.lock().contains(&dep_node)
}

#[inline(always)]
pub fn register_dep_node_debug_str<F>(&self, dep_node: DepNode<K>, debug_str_gen: F)
where
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_query_system/src/query/plumbing.rs
Expand Up @@ -519,6 +519,10 @@ where
prof_timer.finish_with_query_invocation_id(dep_node_index.into());

if let Some(result) = result {
if unlikely!(tcx.dep_context().sess().opts.debugging_opts.query_dep_graph) {
dep_graph.mark_debug_loaded_from_disk(*dep_node)
}

let prev_fingerprint = tcx
.dep_context()
.dep_graph()
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Expand Up @@ -788,6 +788,7 @@ symbols! {
literal,
llvm_asm,
load,
loaded_from_disk,
local,
local_inner_macros,
log10f32,
Expand Down
6 changes: 5 additions & 1 deletion src/test/incremental/change_private_fn/struct_point.rs
Expand Up @@ -51,7 +51,11 @@ pub mod point {
pub mod fn_calls_methods_in_same_impl {
use point::Point;

#[rustc_clean(cfg="cfail2")]
// The cached result should actually be loaded from disk
// (not just marked green) - for example, `DeadVisitor`
// always runs during compilation as a "pass", and loads
// the typeck results for bodies.
#[rustc_clean(cfg="cfail2", loaded_from_disk="typeck")]
pub fn check() {
let x = Point { x: 2.0, y: 2.0 };
x.distance_from_origin();
Expand Down

0 comments on commit b5efe57

Please sign in to comment.