From 3f9aea199ceefef9855850ab7843ef6935055eca Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Sep 2019 14:03:53 +1000 Subject: [PATCH 1/2] Clarify a comment in `RegionConstraintCollector::leak_check`. --- src/librustc/infer/region_constraints/leak_check.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc/infer/region_constraints/leak_check.rs b/src/librustc/infer/region_constraints/leak_check.rs index 0c83bbc1e53..3d069425685 100644 --- a/src/librustc/infer/region_constraints/leak_check.rs +++ b/src/librustc/infer/region_constraints/leak_check.rs @@ -14,9 +14,11 @@ impl<'tcx> RegionConstraintCollector<'tcx> { /// retain the older (arguably incorrect) behavior of the /// compiler. /// - /// NB. The use of snapshot here is mostly an efficiency thing -- - /// we could search *all* region constraints, but that'd be a - /// bigger set and the data structures are not setup for that. If + /// NB. Although `_snapshot` isn't used, it's passed in to prove + /// that we are in a snapshot, which guarantees that we can just + /// search the "undo log" for edges. This is mostly an efficiency + /// thing -- we could search *all* region constraints, but that'd be + /// a bigger set and the data structures are not setup for that. If /// we wind up keeping some form of this check long term, it would /// probably be better to remove the snapshot parameter and to /// refactor the constraint set. From 1b41f958c5d0d84b8dac385fb30bb7c7f12c13b8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Sep 2019 14:42:11 +1000 Subject: [PATCH 2/2] Remove `InferCtxt::in_snapshot()`. It's the same as `InferCtxt::commit_unconditionally()` except that it is passed a snapshot and has a worse name. The commit also changes `commit_unconditionally()` to receive a snapshot, for consistency with `commit_if_ok()` and `probe()`. --- src/librustc/infer/mod.rs | 22 +++++----------------- src/librustc/traits/select.rs | 35 ++++++++++++++++++----------------- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index ea9e931ad83..af74d135724 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -814,16 +814,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { /// Executes `f` and commit the bindings. pub fn commit_unconditionally(&self, f: F) -> R where - F: FnOnce() -> R, + F: FnOnce(&CombinedSnapshot<'a, 'tcx>) -> R, { - debug!("commit()"); + debug!("commit_unconditionally()"); let snapshot = self.start_snapshot(); - let r = f(); + let r = f(&snapshot); self.commit_from(snapshot); r } - /// Executes `f` and commit the bindings if closure `f` returns `Ok(_)`. + /// Execute `f` and commit the bindings if closure `f` returns `Ok(_)`. pub fn commit_if_ok(&self, f: F) -> Result where F: FnOnce(&CombinedSnapshot<'a, 'tcx>) -> Result, @@ -843,19 +843,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { r } - /// Execute `f` in a snapshot, and commit the bindings it creates. - pub fn in_snapshot(&self, f: F) -> T - where - F: FnOnce(&CombinedSnapshot<'a, 'tcx>) -> T, - { - debug!("in_snapshot()"); - let snapshot = self.start_snapshot(); - let r = f(&snapshot); - self.commit_from(snapshot); - r - } - - /// Executes `f` then unroll any bindings it creates. + /// Execute `f` then unroll any bindings it creates. pub fn probe(&self, f: F) -> R where F: FnOnce(&CombinedSnapshot<'a, 'tcx>) -> R, diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 4ee5700f43f..44d611ace77 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2819,7 +2819,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // binder moved -\ let ty: ty::Binder> = ty::Binder::bind(ty); // <----/ - self.infcx.in_snapshot(|_| { + self.infcx.commit_unconditionally(|_| { let (skol_ty, _) = self.infcx .replace_bound_vars_with_placeholders(&ty); let Normalized { @@ -2932,7 +2932,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } fn confirm_projection_candidate(&mut self, obligation: &TraitObligation<'tcx>) { - self.infcx.in_snapshot(|snapshot| { + self.infcx.commit_unconditionally(|snapshot| { let result = self.match_projection_obligation_against_definition_bounds( obligation, @@ -3054,19 +3054,20 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { nested, ); - let trait_obligations: Vec> = self.infcx.in_snapshot(|_| { - let poly_trait_ref = obligation.predicate.to_poly_trait_ref(); - let (trait_ref, _) = self.infcx - .replace_bound_vars_with_placeholders(&poly_trait_ref); - let cause = obligation.derived_cause(ImplDerivedObligation); - self.impl_or_trait_obligations( - cause, - obligation.recursion_depth + 1, - obligation.param_env, - trait_def_id, - &trait_ref.substs, - ) - }); + let trait_obligations: Vec> = + self.infcx.commit_unconditionally(|_| { + let poly_trait_ref = obligation.predicate.to_poly_trait_ref(); + let (trait_ref, _) = self.infcx + .replace_bound_vars_with_placeholders(&poly_trait_ref); + let cause = obligation.derived_cause(ImplDerivedObligation); + self.impl_or_trait_obligations( + cause, + obligation.recursion_depth + 1, + obligation.param_env, + trait_def_id, + &trait_ref.substs, + ) + }); // Adds the predicates from the trait. Note that this contains a `Self: Trait` // predicate as usual. It won't have any effect since auto traits are coinductive. @@ -3089,7 +3090,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // First, create the substitutions by matching the impl again, // this time not in a probe. - self.infcx.in_snapshot(|snapshot| { + self.infcx.commit_unconditionally(|snapshot| { let substs = self.rematch_impl(impl_def_id, obligation, snapshot); debug!("confirm_impl_candidate: substs={:?}", substs); let cause = obligation.derived_cause(ImplDerivedObligation); @@ -3253,7 +3254,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { obligation, alias_def_id ); - self.infcx.in_snapshot(|_| { + self.infcx.commit_unconditionally(|_| { let (predicate, _) = self.infcx() .replace_bound_vars_with_placeholders(&obligation.predicate); let trait_ref = predicate.trait_ref;