Skip to content

incr.comp.: Introduce ensure and ensure typeck_tables_of #45228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/librustc/ty/maps/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,52 @@ macro_rules! define_maps {
}
}

/// Ensure that either this query has all green inputs or been executed.
/// Executing query::ensure(D) is considered a read of the dep-node D.
///
/// This function is particularly useful when executing passes for their
/// side-effects -- e.g., in order to report errors for erroneous programs.
///
/// Note: The optimization is only available during incr. comp.
pub fn ensure(tcx: TyCtxt<'a, $tcx, 'lcx>, key: $K) -> () {
let dep_node = Self::to_dep_node(tcx, &key);

// Ensuring an "input" or anonymous query makes no sense
assert!(!dep_node.kind.is_anon());
assert!(!dep_node.kind.is_input());
use dep_graph::DepNodeColor;
match tcx.dep_graph.node_color(&dep_node) {
Some(DepNodeColor::Green(dep_node_index)) => {
tcx.dep_graph.read_index(dep_node_index);
}
Some(DepNodeColor::Red) => {
// A DepNodeColor::Red DepNode means that this query was executed
// before. We can not call `dep_graph.read()` here as we don't have
// the DepNodeIndex. Instead, We call the query again to issue the
// appropriate `dep_graph.read()` call. The performance cost this
// introduces should be negligible as we'll immediately hit the
// in-memory cache.
let _ = tcx.$name(key);
}
None => {
// Huh
if !tcx.dep_graph.is_fully_enabled() {
let _ = tcx.$name(key);
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikomatsakis Open question: Should the preceding 4 lines be in try_mark_green?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, try_mark_green will unwrap a None here if tcx.dep_graph.is_fully_enabled() would return False.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep the explicit panic in try_mark_green, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave the code here as it is then.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also expected those lines to be in try_mark_green, but I trust @michaelwoerister's intuitions here. =) That said, I think we could consider trying to extract some of the common code between this function and try_get_with. I had thought it might make sense to extract a helper function that returns Some(dep_node_index) if either: the node color is green or can be made green, and None otherwise. This function here could invoke tcx.$name(key) if None is returned, and try_get_with can do whatever it does now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! I'll implement that refactor as a follow on PR, unless this round of testing does not go well and I have to fix something.

match tcx.dep_graph.try_mark_green(tcx, &dep_node) {
Some(dep_node_index) => {
debug_assert!(tcx.dep_graph.is_green(dep_node_index));
tcx.dep_graph.read_index(dep_node_index);
}
None => {
let _ = tcx.$name(key);
}
}
}
}
}

fn compute_result(tcx: TyCtxt<'a, $tcx, 'lcx>, key: $K) -> $V {
let provider = tcx.maps.providers[key.map_crate()].$name;
provider(tcx.global_tcx(), key)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ fn typeck_item_bodies<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum
debug_assert!(crate_num == LOCAL_CRATE);
Ok(tcx.sess.track_errors(|| {
for body_owner_def_id in tcx.body_owners() {
tcx.typeck_tables_of(body_owner_def_id);
ty::maps::queries::typeck_tables_of::ensure(tcx, body_owner_def_id);
}
})?)
}
Expand Down