-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -344,6 +344,48 @@ 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)) => { | ||
profq_msg!(tcx, ProfileQueriesMsg::CacheHit); | ||
tcx.dep_graph.read_index(dep_node_index); | ||
} | ||
Some(DepNodeColor::Red) => { | ||
let _ = tcx.$name(key); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment here saying something like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Just a moment. |
||
} | ||
None => { | ||
// Huh | ||
if !tcx.dep_graph.is_fully_enabled() { | ||
let _ = tcx.$name(key); | ||
return; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nikomatsakis Open question: Should the preceding 4 lines be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For reference, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather keep the explicit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll leave the code here as it is then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also expected those lines to be in There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)); | ||
profq_msg!(tcx, ProfileQueriesMsg::CacheHit); | ||
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) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't do any
profq_msg!
calls in this method for now. We'll have to talk to @matthewhammer at some point about how to properly integrate the new tracking system with the query profiler.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll pull them out. Thanks.