Skip to content

Commit

Permalink
Rollup merge of #110618 - compiler-errors:eval-ctxt-tainted, r=BoxyUwU
Browse files Browse the repository at this point in the history
Track if EvalCtxt has been tainted, make sure it can't be used to make query responses after

Just some additional protection against missing probes or strange candidate assembly behavior in the new solver.

For background, we don't ever want to call `evaluate_added_goals_and_make_canonical_response` if a previous call to `try_evaluate_added_goals` has bailed with `NoSolution`, since our nested goals are left in an undefined state at that point. This most commonly suggests a missing `EvalCtxt::probe`, but could also signify some other shenanigans like dropping a `QueryResult` on the floor without properly `?`'ing it.

r? `@lcnr`
  • Loading branch information
matthiaskrgr authored Apr 21, 2023
2 parents ea01135 + 3206100 commit 77de5f0
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
15 changes: 15 additions & 0 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ pub struct EvalCtxt<'a, 'tcx> {
pub(super) search_graph: &'a mut SearchGraph<'tcx>,

pub(super) nested_goals: NestedGoals<'tcx>,

// Has this `EvalCtxt` errored out with `NoSolution` in `try_evaluate_added_goals`?
//
// If so, then it can no longer be used to make a canonical query response,
// since subsequent calls to `try_evaluate_added_goals` have possibly dropped
// ambiguous goals. Instead, a probe needs to be introduced somewhere in the
// evaluation code.
tainted: Result<(), NoSolution>,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -121,6 +129,7 @@ impl<'tcx> InferCtxtEvalExt<'tcx> for InferCtxt<'tcx> {
max_input_universe: ty::UniverseIndex::ROOT,
var_values: CanonicalVarValues::dummy(),
nested_goals: NestedGoals::new(),
tainted: Ok(()),
};
let result = ecx.evaluate_goal(IsNormalizesToHack::No, goal);

Expand Down Expand Up @@ -172,6 +181,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
max_input_universe: canonical_goal.max_universe,
search_graph,
nested_goals: NestedGoals::new(),
tainted: Ok(()),
};
ecx.compute_goal(goal)
})
Expand Down Expand Up @@ -391,6 +401,10 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
},
);

if response.is_err() {
self.tainted = Err(NoSolution);
}

self.nested_goals = goals;
response
}
Expand All @@ -404,6 +418,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
max_input_universe: self.max_input_universe,
search_graph: self.search_graph,
nested_goals: self.nested_goals.clone(),
tainted: self.tainted,
};
self.infcx.probe(|_| f(&mut ecx))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
certainty: Certainty,
) -> QueryResult<'tcx> {
let goals_certainty = self.try_evaluate_added_goals()?;
assert_eq!(
self.tainted,
Ok(()),
"EvalCtxt is tainted -- nested goals may have been dropped in a \
previous call to `try_evaluate_added_goals!`"
);

let certainty = certainty.unify_with(goals_certainty);

let external_constraints = self.compute_external_query_constraints()?;
Expand Down

0 comments on commit 77de5f0

Please sign in to comment.