Skip to content

Commit

Permalink
Auto merge of rust-lang#133828 - compiler-errors:incr-sad, r=lcnr
Browse files Browse the repository at this point in the history
Make sure to record deps from cached task in new solver on first run

We weren't actually performing a read of the dep node in `with_cached_task` in the new solver, which meant that all queries that computed a goal for the first time were just not recording the query dependencies that we call in that query.

In the incremental test, the typeck query for `fn poll` isn't being marked red even tho it's invalidated due to its writeback results changing. This happens b/c we normalize `Self::Error` into `Error`, which should call `type_of` which is a red query (since `ty::Adt` contains an `AdtDef`, and that `AdtDef`'s stable hash changes since it's ). However, since we weren't tracking deps in that normalize query, the typeck result was remaining green, and we were trying to decode a def id that no longer exists (the field that got removed).

r? lcnr
  • Loading branch information
bors committed Dec 5, 2024
2 parents acabb52 + 988f28d commit 5a0a5e6
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 10 deletions.
17 changes: 15 additions & 2 deletions compiler/rustc_query_system/src/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,11 @@ impl<D: Deps> DepGraph<D> {
OP: FnOnce() -> R,
{
match self.data() {
Some(data) => data.with_anon_task(cx, dep_kind, op),
Some(data) => {
let (result, index) = data.with_anon_task_inner(cx, dep_kind, op);
self.read_index(index);
(result, index)
}
None => (op(), self.next_virtual_depnode_index()),
}
}
Expand Down Expand Up @@ -397,7 +401,16 @@ impl<D: Deps> DepGraphData<D> {

/// Executes something within an "anonymous" task, that is, a task the
/// `DepNode` of which is determined by the list of inputs it read from.
pub(crate) fn with_anon_task<Tcx: DepContext<Deps = D>, OP, R>(
///
/// NOTE: this does not actually count as a read of the DepNode here.
/// Using the result of this task without reading the DepNode will result
/// in untracked dependencies which may lead to ICEs as nodes are
/// incorrectly marked green.
///
/// FIXME: This could perhaps return a `WithDepNode` to ensure that the
/// user of this function actually performs the read; we'll have to see
/// how to make that work with `anon` in `execute_job_incr`, though.
pub(crate) fn with_anon_task_inner<Tcx: DepContext<Deps = D>, OP, R>(
&self,
cx: Tcx,
dep_kind: DepKind,
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_query_system/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,9 +520,11 @@ where
let (result, dep_node_index) =
qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), || {
if query.anon() {
return dep_graph_data.with_anon_task(*qcx.dep_context(), query.dep_kind(), || {
query.compute(qcx, key)
});
return dep_graph_data.with_anon_task_inner(
*qcx.dep_context(),
query.dep_kind(),
|| query.compute(qcx, key),
);
}

// `to_dep_node` is expensive for some `DepKind`s.
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1400,10 +1400,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
where
OP: FnOnce(&mut Self) -> R,
{
let (result, dep_node) =
self.tcx().dep_graph.with_anon_task(self.tcx(), dep_kinds::TraitSelect, || op(self));
self.tcx().dep_graph.read_index(dep_node);
(result, dep_node)
self.tcx().dep_graph.with_anon_task(self.tcx(), dep_kinds::TraitSelect, || op(self))
}

/// filter_impls filters candidates that have a positive impl for a negative
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_type_ir/src/search_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {

// This is for global caching, so we properly track query dependencies.
// Everything that affects the `result` should be performed within this
// `with_anon_task` closure. If computing this goal depends on something
// `with_cached_task` closure. If computing this goal depends on something
// not tracked by the cache key and from outside of this anon task, it
// must not be added to the global cache. Notably, this is the case for
// trait solver cycles participants.
Expand Down
25 changes: 25 additions & 0 deletions tests/incremental/track-deps-in-new-solver.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//@ revisions: cfail1 cfail2

//@ compile-flags: -Znext-solver
//@ check-pass

pub trait Future {
type Error;
fn poll() -> Self::Error;
}

struct S;
impl Future for S {
type Error = Error;
fn poll() -> Self::Error {
todo!()
}
}

#[cfg(cfail1)]
pub struct Error(());

#[cfg(cfail2)]
pub struct Error();

fn main() {}

0 comments on commit 5a0a5e6

Please sign in to comment.