Unnamed repository; edit this file 'description' to name the repository.
Do not save the `FulfillmentCtxt` in snapshots
This is what rustc does. It's more performant, but we couldn't do that so far because old code relied on also restoring the obligations, i.e. it was registering maybe-incorrect obligations in the `InferenceTable` without first checking them in a separate `ObligationCtxt`. That code is now gone.
| -rw-r--r-- | crates/hir-ty/src/infer/coerce.rs | 6 | ||||
| -rw-r--r-- | crates/hir-ty/src/infer/unify.rs | 22 | ||||
| -rw-r--r-- | crates/hir-ty/src/next_solver/fulfill.rs | 19 | ||||
| -rw-r--r-- | crates/hir-ty/src/next_solver/infer/snapshot/mod.rs | 6 |
4 files changed, 17 insertions, 36 deletions
diff --git a/crates/hir-ty/src/infer/coerce.rs b/crates/hir-ty/src/infer/coerce.rs index db912c0eb6..962fc752a5 100644 --- a/crates/hir-ty/src/infer/coerce.rs +++ b/crates/hir-ty/src/infer/coerce.rs @@ -152,10 +152,8 @@ where let snapshot = self.infcx().start_snapshot(); let result = f(self); match result { - Ok(_) => {} - Err(_) => { - self.infcx().rollback_to(snapshot); - } + Ok(_) => self.infcx().commit_from(snapshot), + Err(_) => self.infcx().rollback_to(snapshot), } result } diff --git a/crates/hir-ty/src/infer/unify.rs b/crates/hir-ty/src/infer/unify.rs index 4df198e002..c28fea0ff1 100644 --- a/crates/hir-ty/src/infer/unify.rs +++ b/crates/hir-ty/src/infer/unify.rs @@ -130,11 +130,6 @@ pub(crate) struct InferenceTable<'db> { pub(super) diverging_type_vars: FxHashSet<Ty<'db>>, } -pub(crate) struct InferenceTableSnapshot<'db> { - ctxt_snapshot: CombinedSnapshot, - obligations: FulfillmentCtxt<'db>, -} - impl<'db> InferenceTable<'db> { /// Inside hir-ty you should use this for inference only, and always pass `owner`. /// Outside it, always pass `owner = None`. @@ -353,16 +348,13 @@ impl<'db> InferenceTable<'db> { // FIXME: Err if it still contain infer vars. } - pub(crate) fn snapshot(&mut self) -> InferenceTableSnapshot<'db> { - let ctxt_snapshot = self.infer_ctxt.start_snapshot(); - let obligations = self.fulfillment_cx.clone(); - InferenceTableSnapshot { ctxt_snapshot, obligations } + pub(crate) fn snapshot(&mut self) -> CombinedSnapshot { + self.infer_ctxt.start_snapshot() } #[tracing::instrument(skip_all)] - pub(crate) fn rollback_to(&mut self, snapshot: InferenceTableSnapshot<'db>) { - self.infer_ctxt.rollback_to(snapshot.ctxt_snapshot); - self.fulfillment_cx = snapshot.obligations; + pub(crate) fn rollback_to(&mut self, snapshot: CombinedSnapshot) { + self.infer_ctxt.rollback_to(snapshot); } pub(crate) fn commit_if_ok<T, E>( @@ -372,10 +364,8 @@ impl<'db> InferenceTable<'db> { let snapshot = self.snapshot(); let result = f(self); match result { - Ok(_) => {} - Err(_) => { - self.rollback_to(snapshot); - } + Ok(_) => self.infer_ctxt.commit_from(snapshot), + Err(_) => self.rollback_to(snapshot), } result } diff --git a/crates/hir-ty/src/next_solver/fulfill.rs b/crates/hir-ty/src/next_solver/fulfill.rs index 6739795a00..cd0cb59760 100644 --- a/crates/hir-ty/src/next_solver/fulfill.rs +++ b/crates/hir-ty/src/next_solver/fulfill.rs @@ -44,7 +44,6 @@ pub struct FulfillmentCtxt<'db> { /// outside of this snapshot leads to subtle bugs if the snapshot /// gets rolled back. Because of this we explicitly check that we only /// use the context in exactly this snapshot. - #[expect(unused)] usable_in_snapshot: usize, try_evaluate_obligations_scratch: PendingObligations<'db>, } @@ -120,24 +119,22 @@ impl<'db> FulfillmentCtxt<'db> { } impl<'db> FulfillmentCtxt<'db> { - #[tracing::instrument(level = "trace", skip(self, _infcx))] + #[tracing::instrument(level = "trace", skip(self, infcx))] pub(crate) fn register_predicate_obligation( &mut self, - _infcx: &InferCtxt<'db>, + infcx: &InferCtxt<'db>, obligation: PredicateObligation<'db>, ) { - // FIXME: See the comment in `try_evaluate_obligations()`. - // assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots()); + assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots()); self.obligations.register(obligation, None); } pub(crate) fn register_predicate_obligations( &mut self, - _infcx: &InferCtxt<'db>, + infcx: &InferCtxt<'db>, obligations: impl IntoIterator<Item = PredicateObligation<'db>>, ) { - // FIXME: See the comment in `try_evaluate_obligations()`. - // assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots()); + assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots()); obligations.into_iter().for_each(|obligation| self.obligations.register(obligation, None)); } @@ -157,11 +154,7 @@ impl<'db> FulfillmentCtxt<'db> { &mut self, infcx: &InferCtxt<'db>, ) -> Vec<NextSolverError<'db>> { - // FIXME(next-solver): We should bring this assertion back. Currently it panics because - // there are places which use `InferenceTable` and open a snapshot and register obligations - // and select. They should use a different `ObligationCtxt` instead. Then we'll be also able - // to not put the obligations queue in `InferenceTable`'s snapshots. - // assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots()); + assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots()); self.try_evaluate_obligations_scratch.clear(); let mut errors = Vec::new(); loop { diff --git a/crates/hir-ty/src/next_solver/infer/snapshot/mod.rs b/crates/hir-ty/src/next_solver/infer/snapshot/mod.rs index 705aa43fb1..39c8a37adb 100644 --- a/crates/hir-ty/src/next_solver/infer/snapshot/mod.rs +++ b/crates/hir-ty/src/next_solver/infer/snapshot/mod.rs @@ -47,7 +47,7 @@ impl<'db> InferCtxt<'db> { UndoLogs::<UndoLog<'db>>::num_open_snapshots(&self.inner.borrow_mut().undo_log) } - pub(crate) fn start_snapshot(&self) -> CombinedSnapshot { + pub fn start_snapshot(&self) -> CombinedSnapshot { debug!("start_snapshot()"); let mut inner = self.inner.borrow_mut(); @@ -60,7 +60,7 @@ impl<'db> InferCtxt<'db> { } #[instrument(skip(self, snapshot), level = "debug")] - pub(crate) fn rollback_to(&self, snapshot: CombinedSnapshot) { + pub fn rollback_to(&self, snapshot: CombinedSnapshot) { let CombinedSnapshot { undo_snapshot, region_constraints_snapshot, universe } = snapshot; self.universe.set(universe); @@ -71,7 +71,7 @@ impl<'db> InferCtxt<'db> { } #[instrument(skip(self, snapshot), level = "debug")] - fn commit_from(&self, snapshot: CombinedSnapshot) { + pub fn commit_from(&self, snapshot: CombinedSnapshot) { let CombinedSnapshot { undo_snapshot, region_constraints_snapshot: _, universe: _ } = snapshot; |