Rollup merge of rust-lang#121002 - lcnr:cleanup-commit_if_ok, r=oli-obk

remove unnecessary calls to `commit_if_ok` we propagate the error outwards, so anything which wants to discard the error should do so itself. r? types
matthiaskrgr · Feb 13, 2024 · ee19276 · ee19276
2 parents 458d5c3 + ae92334
commit ee19276
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 70 deletions.
diff --git a/compiler/rustc_infer/src/infer/at.rs b/compiler/rustc_infer/src/infer/at.rs
@@ -285,13 +285,11 @@ impl<'a, 'tcx> Trace<'a, 'tcx> {
         T: Relate<'tcx>,
     {
         let Trace { at, trace, a_is_expected } = self;
-        at.infcx.commit_if_ok(|_| {
-            let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
-            fields
-                .sub(a_is_expected)
-                .relate(a, b)
-                .map(move |_| InferOk { value: (), obligations: fields.obligations })
-        })
+        let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
+        fields
+            .sub(a_is_expected)
+            .relate(a, b)
+            .map(move |_| InferOk { value: (), obligations: fields.obligations })
     }
 
     /// Makes `a == b`; the expectation is set by the call to
@@ -302,13 +300,11 @@ impl<'a, 'tcx> Trace<'a, 'tcx> {
         T: Relate<'tcx>,
     {
         let Trace { at, trace, a_is_expected } = self;
-        at.infcx.commit_if_ok(|_| {
-            let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
-            fields
-                .equate(a_is_expected)
-                .relate(a, b)
-                .map(move |_| InferOk { value: (), obligations: fields.obligations })
-        })
+        let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
+        fields
+            .equate(a_is_expected)
+            .relate(a, b)
+            .map(move |_| InferOk { value: (), obligations: fields.obligations })
     }
 
     #[instrument(skip(self), level = "debug")]
@@ -317,13 +313,11 @@ impl<'a, 'tcx> Trace<'a, 'tcx> {
         T: Relate<'tcx>,
     {
         let Trace { at, trace, a_is_expected } = self;
-        at.infcx.commit_if_ok(|_| {
-            let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
-            fields
-                .lub(a_is_expected)
-                .relate(a, b)
-                .map(move |t| InferOk { value: t, obligations: fields.obligations })
-        })
+        let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
+        fields
+            .lub(a_is_expected)
+            .relate(a, b)
+            .map(move |t| InferOk { value: t, obligations: fields.obligations })
     }
 
     #[instrument(skip(self), level = "debug")]
@@ -332,13 +326,11 @@ impl<'a, 'tcx> Trace<'a, 'tcx> {
         T: Relate<'tcx>,
     {
         let Trace { at, trace, a_is_expected } = self;
-        at.infcx.commit_if_ok(|_| {
-            let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
-            fields
-                .glb(a_is_expected)
-                .relate(a, b)
-                .map(move |t| InferOk { value: t, obligations: fields.obligations })
-        })
+        let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
+        fields
+            .glb(a_is_expected)
+            .relate(a, b)
+            .map(move |t| InferOk { value: t, obligations: fields.obligations })
     }
 }
 

diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs
@@ -620,42 +620,40 @@ impl<'tcx> InferCtxt<'tcx> {
         variables1: &OriginalQueryValues<'tcx>,
         variables2: impl Fn(BoundVar) -> GenericArg<'tcx>,
     ) -> InferResult<'tcx, ()> {
-        self.commit_if_ok(|_| {
-            let mut obligations = vec![];
-            for (index, value1) in variables1.var_values.iter().enumerate() {
-                let value2 = variables2(BoundVar::new(index));
-
-                match (value1.unpack(), value2.unpack()) {
-                    (GenericArgKind::Type(v1), GenericArgKind::Type(v2)) => {
-                        obligations.extend(
-                            self.at(cause, param_env)
-                                .eq(DefineOpaqueTypes::Yes, v1, v2)?
-                                .into_obligations(),
-                        );
-                    }
-                    (GenericArgKind::Lifetime(re1), GenericArgKind::Lifetime(re2))
-                        if re1.is_erased() && re2.is_erased() =>
-                    {
-                        // no action needed
-                    }
-                    (GenericArgKind::Lifetime(v1), GenericArgKind::Lifetime(v2)) => {
-                        obligations.extend(
-                            self.at(cause, param_env)
-                                .eq(DefineOpaqueTypes::Yes, v1, v2)?
-                                .into_obligations(),
-                        );
-                    }
-                    (GenericArgKind::Const(v1), GenericArgKind::Const(v2)) => {
-                        let ok = self.at(cause, param_env).eq(DefineOpaqueTypes::Yes, v1, v2)?;
-                        obligations.extend(ok.into_obligations());
-                    }
-                    _ => {
-                        bug!("kind mismatch, cannot unify {:?} and {:?}", value1, value2,);
-                    }
+        let mut obligations = vec![];
+        for (index, value1) in variables1.var_values.iter().enumerate() {
+            let value2 = variables2(BoundVar::new(index));
+
+            match (value1.unpack(), value2.unpack()) {
+                (GenericArgKind::Type(v1), GenericArgKind::Type(v2)) => {
+                    obligations.extend(
+                        self.at(cause, param_env)
+                            .eq(DefineOpaqueTypes::Yes, v1, v2)?
+                            .into_obligations(),
+                    );
+                }
+                (GenericArgKind::Lifetime(re1), GenericArgKind::Lifetime(re2))
+                    if re1.is_erased() && re2.is_erased() =>
+                {
+                    // no action needed
+                }
+                (GenericArgKind::Lifetime(v1), GenericArgKind::Lifetime(v2)) => {
+                    obligations.extend(
+                        self.at(cause, param_env)
+                            .eq(DefineOpaqueTypes::Yes, v1, v2)?
+                            .into_obligations(),
+                    );
+                }
+                (GenericArgKind::Const(v1), GenericArgKind::Const(v2)) => {
+                    let ok = self.at(cause, param_env).eq(DefineOpaqueTypes::Yes, v1, v2)?;
+                    obligations.extend(ok.into_obligations());
+                }
+                _ => {
+                    bug!("kind mismatch, cannot unify {:?} and {:?}", value1, value2,);
                 }
             }
-            Ok(InferOk { value: (), obligations })
-        })
+        }
+        Ok(InferOk { value: (), obligations })
     }
 }
 

diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
@@ -192,13 +192,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             &mut obligations,
         );
 
-        obligations.extend(self.infcx.commit_if_ok(|_| {
+        obligations.extend(
             self.infcx
                 .at(&obligation.cause, obligation.param_env)
                 .sup(DefineOpaqueTypes::No, placeholder_trait_predicate, candidate)
                 .map(|InferOk { obligations, .. }| obligations)
-                .map_err(|_| Unimplemented)
-        })?);
+                .map_err(|_| Unimplemented)?,
+        );
 
         // FIXME(compiler-errors): I don't think this is needed.
         if let ty::Alias(ty::Projection, alias_ty) = placeholder_self_ty.kind() {
@@ -532,13 +532,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             &mut nested,
         );
 
-        nested.extend(self.infcx.commit_if_ok(|_| {
+        nested.extend(
             self.infcx
                 .at(&obligation.cause, obligation.param_env)
                 .sup(DefineOpaqueTypes::No, obligation_trait_ref, upcast_trait_ref)
                 .map(|InferOk { obligations, .. }| obligations)
-                .map_err(|_| Unimplemented)
-        })?);
+                .map_err(|_| Unimplemented)?,
+        );
 
         // Check supertraits hold. This is so that their associated type bounds
         // will be checked in the code below.

diff --git a/tests/ui/closure-expected-type/expect-infer-var-appearing-twice.stderr b/tests/ui/closure-expected-type/expect-infer-var-appearing-twice.stderr
@@ -9,8 +9,8 @@ LL | |
 LL | |     });
    | |______^ expected due to this
    |
-   = note: expected closure signature `fn(_, _) -> _`
-              found closure signature `fn(u32, i32) -> _`
+   = note: expected closure signature `fn(_, u32) -> _`
+              found closure signature `fn(_, i32) -> _`
 note: required by a bound in `with_closure`
   --> $DIR/expect-infer-var-appearing-twice.rs:2:14
    |