Skip to content

Commit b759b2e

Browse files
committed
Auto merge of #99509 - lcnr:commit_unconditionally, r=jackh726
remove `commit_unconditionally` `commit_unconditionally` is a noop unless we somehow inspect the current state of our snapshot. The only thing which does that is the leak check which was only used in one place where `commit_if_ok` is probably at least as, or even more, correct. r? rust-lang/types
2 parents e4417cf + b81e74c commit b759b2e

File tree

4 files changed

+168
-199
lines changed

4 files changed

+168
-199
lines changed

compiler/rustc_infer/src/infer/mod.rs

-12
Original file line numberDiff line numberDiff line change
@@ -840,18 +840,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
840840
self.inner.borrow_mut().commit(undo_snapshot);
841841
}
842842

843-
/// Executes `f` and commit the bindings.
844-
#[instrument(skip(self, f), level = "debug")]
845-
pub fn commit_unconditionally<R, F>(&self, f: F) -> R
846-
where
847-
F: FnOnce(&CombinedSnapshot<'a, 'tcx>) -> R,
848-
{
849-
let snapshot = self.start_snapshot();
850-
let r = f(&snapshot);
851-
self.commit_from(snapshot);
852-
r
853-
}
854-
855843
/// Execute `f` and commit the bindings if closure `f` returns `Ok(_)`.
856844
#[instrument(skip(self, f), level = "debug")]
857845
pub fn commit_if_ok<T, E, F>(&self, f: F) -> Result<T, E>

compiler/rustc_trait_selection/src/traits/select/confirmation.rs

+145-162
Original file line numberDiff line numberDiff line change
@@ -144,67 +144,65 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
144144
obligation: &TraitObligation<'tcx>,
145145
idx: usize,
146146
) -> Result<Vec<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
147-
self.infcx.commit_unconditionally(|_| {
148-
let tcx = self.tcx();
149-
150-
let trait_predicate = self.infcx.shallow_resolve(obligation.predicate);
151-
let placeholder_trait_predicate =
152-
self.infcx().replace_bound_vars_with_placeholders(trait_predicate).trait_ref;
153-
let placeholder_self_ty = placeholder_trait_predicate.self_ty();
154-
let placeholder_trait_predicate = ty::Binder::dummy(placeholder_trait_predicate);
155-
let (def_id, substs) = match *placeholder_self_ty.kind() {
156-
ty::Projection(proj) => (proj.item_def_id, proj.substs),
157-
ty::Opaque(def_id, substs) => (def_id, substs),
158-
_ => bug!("projection candidate for unexpected type: {:?}", placeholder_self_ty),
159-
};
147+
let tcx = self.tcx();
160148

161-
let candidate_predicate =
162-
tcx.bound_item_bounds(def_id).map_bound(|i| i[idx]).subst(tcx, substs);
163-
let candidate = candidate_predicate
164-
.to_opt_poly_trait_pred()
165-
.expect("projection candidate is not a trait predicate")
166-
.map_bound(|t| t.trait_ref);
167-
let mut obligations = Vec::new();
168-
let candidate = normalize_with_depth_to(
169-
self,
170-
obligation.param_env,
171-
obligation.cause.clone(),
172-
obligation.recursion_depth + 1,
173-
candidate,
174-
&mut obligations,
175-
);
149+
let trait_predicate = self.infcx.shallow_resolve(obligation.predicate);
150+
let placeholder_trait_predicate =
151+
self.infcx().replace_bound_vars_with_placeholders(trait_predicate).trait_ref;
152+
let placeholder_self_ty = placeholder_trait_predicate.self_ty();
153+
let placeholder_trait_predicate = ty::Binder::dummy(placeholder_trait_predicate);
154+
let (def_id, substs) = match *placeholder_self_ty.kind() {
155+
ty::Projection(proj) => (proj.item_def_id, proj.substs),
156+
ty::Opaque(def_id, substs) => (def_id, substs),
157+
_ => bug!("projection candidate for unexpected type: {:?}", placeholder_self_ty),
158+
};
176159

177-
obligations.extend(self.infcx.commit_if_ok(|_| {
178-
self.infcx
179-
.at(&obligation.cause, obligation.param_env)
180-
.sup(placeholder_trait_predicate, candidate)
181-
.map(|InferOk { obligations, .. }| obligations)
182-
.map_err(|_| Unimplemented)
183-
})?);
184-
185-
if let ty::Projection(..) = placeholder_self_ty.kind() {
186-
let predicates = tcx.predicates_of(def_id).instantiate_own(tcx, substs).predicates;
187-
debug!(?predicates, "projection predicates");
188-
for predicate in predicates {
189-
let normalized = normalize_with_depth_to(
190-
self,
191-
obligation.param_env,
192-
obligation.cause.clone(),
193-
obligation.recursion_depth + 1,
194-
predicate,
195-
&mut obligations,
196-
);
197-
obligations.push(Obligation::with_depth(
198-
obligation.cause.clone(),
199-
obligation.recursion_depth + 1,
200-
obligation.param_env,
201-
normalized,
202-
));
203-
}
160+
let candidate_predicate =
161+
tcx.bound_item_bounds(def_id).map_bound(|i| i[idx]).subst(tcx, substs);
162+
let candidate = candidate_predicate
163+
.to_opt_poly_trait_pred()
164+
.expect("projection candidate is not a trait predicate")
165+
.map_bound(|t| t.trait_ref);
166+
let mut obligations = Vec::new();
167+
let candidate = normalize_with_depth_to(
168+
self,
169+
obligation.param_env,
170+
obligation.cause.clone(),
171+
obligation.recursion_depth + 1,
172+
candidate,
173+
&mut obligations,
174+
);
175+
176+
obligations.extend(self.infcx.commit_if_ok(|_| {
177+
self.infcx
178+
.at(&obligation.cause, obligation.param_env)
179+
.sup(placeholder_trait_predicate, candidate)
180+
.map(|InferOk { obligations, .. }| obligations)
181+
.map_err(|_| Unimplemented)
182+
})?);
183+
184+
if let ty::Projection(..) = placeholder_self_ty.kind() {
185+
let predicates = tcx.predicates_of(def_id).instantiate_own(tcx, substs).predicates;
186+
debug!(?predicates, "projection predicates");
187+
for predicate in predicates {
188+
let normalized = normalize_with_depth_to(
189+
self,
190+
obligation.param_env,
191+
obligation.cause.clone(),
192+
obligation.recursion_depth + 1,
193+
predicate,
194+
&mut obligations,
195+
);
196+
obligations.push(Obligation::with_depth(
197+
obligation.cause.clone(),
198+
obligation.recursion_depth + 1,
199+
obligation.param_env,
200+
normalized,
201+
));
204202
}
203+
}
205204

206-
Ok(obligations)
207-
})
205+
Ok(obligations)
208206
}
209207

210208
fn confirm_param_candidate(
@@ -347,19 +345,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
347345
ensure_sufficient_stack(|| {
348346
let cause = obligation.derived_cause(BuiltinDerivedObligation);
349347

350-
let trait_obligations: Vec<PredicateObligation<'_>> =
351-
self.infcx.commit_unconditionally(|_| {
352-
let poly_trait_ref = obligation.predicate.to_poly_trait_ref();
353-
let trait_ref = self.infcx.replace_bound_vars_with_placeholders(poly_trait_ref);
354-
self.impl_or_trait_obligations(
355-
&cause,
356-
obligation.recursion_depth + 1,
357-
obligation.param_env,
358-
trait_def_id,
359-
&trait_ref.substs,
360-
obligation.predicate,
361-
)
362-
});
348+
let poly_trait_ref = obligation.predicate.to_poly_trait_ref();
349+
let trait_ref = self.infcx.replace_bound_vars_with_placeholders(poly_trait_ref);
350+
let trait_obligations: Vec<PredicateObligation<'_>> = self.impl_or_trait_obligations(
351+
&cause,
352+
obligation.recursion_depth + 1,
353+
obligation.param_env,
354+
trait_def_id,
355+
&trait_ref.substs,
356+
obligation.predicate,
357+
);
363358

364359
let mut obligations = self.collect_predicates_for_types(
365360
obligation.param_env,
@@ -388,19 +383,17 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
388383

389384
// First, create the substitutions by matching the impl again,
390385
// this time not in a probe.
391-
self.infcx.commit_unconditionally(|_| {
392-
let substs = self.rematch_impl(impl_def_id, obligation);
393-
debug!(?substs, "impl substs");
394-
ensure_sufficient_stack(|| {
395-
self.vtable_impl(
396-
impl_def_id,
397-
substs,
398-
&obligation.cause,
399-
obligation.recursion_depth + 1,
400-
obligation.param_env,
401-
obligation.predicate,
402-
)
403-
})
386+
let substs = self.rematch_impl(impl_def_id, obligation);
387+
debug!(?substs, "impl substs");
388+
ensure_sufficient_stack(|| {
389+
self.vtable_impl(
390+
impl_def_id,
391+
substs,
392+
&obligation.cause,
393+
obligation.recursion_depth + 1,
394+
obligation.param_env,
395+
obligation.predicate,
396+
)
404397
})
405398
}
406399

@@ -647,25 +640,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
647640
) -> ImplSourceTraitAliasData<'tcx, PredicateObligation<'tcx>> {
648641
debug!(?obligation, ?alias_def_id, "confirm_trait_alias_candidate");
649642

650-
self.infcx.commit_unconditionally(|_| {
651-
let predicate = self.infcx().replace_bound_vars_with_placeholders(obligation.predicate);
652-
let trait_ref = predicate.trait_ref;
653-
let trait_def_id = trait_ref.def_id;
654-
let substs = trait_ref.substs;
643+
let predicate = self.infcx().replace_bound_vars_with_placeholders(obligation.predicate);
644+
let trait_ref = predicate.trait_ref;
645+
let trait_def_id = trait_ref.def_id;
646+
let substs = trait_ref.substs;
655647

656-
let trait_obligations = self.impl_or_trait_obligations(
657-
&obligation.cause,
658-
obligation.recursion_depth,
659-
obligation.param_env,
660-
trait_def_id,
661-
&substs,
662-
obligation.predicate,
663-
);
648+
let trait_obligations = self.impl_or_trait_obligations(
649+
&obligation.cause,
650+
obligation.recursion_depth,
651+
obligation.param_env,
652+
trait_def_id,
653+
&substs,
654+
obligation.predicate,
655+
);
664656

665-
debug!(?trait_def_id, ?trait_obligations, "trait alias obligations");
657+
debug!(?trait_def_id, ?trait_obligations, "trait alias obligations");
666658

667-
ImplSourceTraitAliasData { alias_def_id, substs, nested: trait_obligations }
668-
})
659+
ImplSourceTraitAliasData { alias_def_id, substs, nested: trait_obligations }
669660
}
670661

671662
fn confirm_generator_candidate(
@@ -763,15 +754,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
763754
// Normalize the obligation and expected trait refs together, because why not
764755
let Normalized { obligations: nested, value: (obligation_trait_ref, expected_trait_ref) } =
765756
ensure_sufficient_stack(|| {
766-
self.infcx.commit_unconditionally(|_| {
767-
normalize_with_depth(
768-
self,
769-
obligation.param_env,
770-
obligation.cause.clone(),
771-
obligation.recursion_depth + 1,
772-
(obligation_trait_ref, expected_trait_ref),
773-
)
774-
})
757+
normalize_with_depth(
758+
self,
759+
obligation.param_env,
760+
obligation.cause.clone(),
761+
obligation.recursion_depth + 1,
762+
(obligation_trait_ref, expected_trait_ref),
763+
)
775764
});
776765

777766
self.infcx
@@ -1147,32 +1136,30 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
11471136
// first check it like a regular impl candidate.
11481137
// This is copied from confirm_impl_candidate but remaps the predicate to `~const Drop` beforehand.
11491138
if let Some(impl_def_id) = impl_def_id {
1150-
let obligations = self.infcx.commit_unconditionally(|_| {
1151-
let mut new_obligation = obligation.clone();
1152-
new_obligation.predicate = new_obligation.predicate.map_bound(|mut trait_pred| {
1153-
trait_pred.trait_ref.def_id = drop_trait;
1154-
trait_pred
1155-
});
1156-
let substs = self.rematch_impl(impl_def_id, &new_obligation);
1157-
debug!(?substs, "impl substs");
1158-
1159-
let cause = obligation.derived_cause(|derived| {
1160-
ImplDerivedObligation(Box::new(ImplDerivedObligationCause {
1161-
derived,
1162-
impl_def_id,
1163-
span: obligation.cause.span,
1164-
}))
1165-
});
1166-
ensure_sufficient_stack(|| {
1167-
self.vtable_impl(
1168-
impl_def_id,
1169-
substs,
1170-
&cause,
1171-
new_obligation.recursion_depth + 1,
1172-
new_obligation.param_env,
1173-
obligation.predicate,
1174-
)
1175-
})
1139+
let mut new_obligation = obligation.clone();
1140+
new_obligation.predicate = new_obligation.predicate.map_bound(|mut trait_pred| {
1141+
trait_pred.trait_ref.def_id = drop_trait;
1142+
trait_pred
1143+
});
1144+
let substs = self.rematch_impl(impl_def_id, &new_obligation);
1145+
debug!(?substs, "impl substs");
1146+
1147+
let cause = obligation.derived_cause(|derived| {
1148+
ImplDerivedObligation(Box::new(ImplDerivedObligationCause {
1149+
derived,
1150+
impl_def_id,
1151+
span: obligation.cause.span,
1152+
}))
1153+
});
1154+
let obligations = ensure_sufficient_stack(|| {
1155+
self.vtable_impl(
1156+
impl_def_id,
1157+
substs,
1158+
&cause,
1159+
new_obligation.recursion_depth + 1,
1160+
new_obligation.param_env,
1161+
obligation.predicate,
1162+
)
11761163
});
11771164
nested.extend(obligations.nested);
11781165
}
@@ -1223,34 +1210,30 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
12231210
// If we have a projection type, make sure to normalize it so we replace it
12241211
// with a fresh infer variable
12251212
ty::Projection(..) => {
1226-
self.infcx.commit_unconditionally(|_| {
1227-
let predicate = normalize_with_depth_to(
1228-
self,
1229-
obligation.param_env,
1230-
cause.clone(),
1231-
obligation.recursion_depth + 1,
1232-
self_ty
1233-
.rebind(ty::TraitPredicate {
1234-
trait_ref: ty::TraitRef {
1235-
def_id: self
1236-
.tcx()
1237-
.require_lang_item(LangItem::Destruct, None),
1238-
substs: self.tcx().mk_substs_trait(nested_ty, &[]),
1239-
},
1240-
constness: ty::BoundConstness::ConstIfConst,
1241-
polarity: ty::ImplPolarity::Positive,
1242-
})
1243-
.to_predicate(tcx),
1244-
&mut nested,
1245-
);
1246-
1247-
nested.push(Obligation::with_depth(
1248-
cause.clone(),
1249-
obligation.recursion_depth + 1,
1250-
obligation.param_env,
1251-
predicate,
1252-
));
1253-
});
1213+
let predicate = normalize_with_depth_to(
1214+
self,
1215+
obligation.param_env,
1216+
cause.clone(),
1217+
obligation.recursion_depth + 1,
1218+
self_ty
1219+
.rebind(ty::TraitPredicate {
1220+
trait_ref: ty::TraitRef {
1221+
def_id: self.tcx().require_lang_item(LangItem::Destruct, None),
1222+
substs: self.tcx().mk_substs_trait(nested_ty, &[]),
1223+
},
1224+
constness: ty::BoundConstness::ConstIfConst,
1225+
polarity: ty::ImplPolarity::Positive,
1226+
})
1227+
.to_predicate(tcx),
1228+
&mut nested,
1229+
);
1230+
1231+
nested.push(Obligation::with_depth(
1232+
cause.clone(),
1233+
obligation.recursion_depth + 1,
1234+
obligation.param_env,
1235+
predicate,
1236+
));
12541237
}
12551238

12561239
// If we have any other type (e.g. an ADT), just register a nested obligation

0 commit comments

Comments
 (0)