Auto merge of #125413 - lcnr:ambig-drop-region-constraints, r=<try>

bors · bors · commit 870a2db6a840 · 2024-05-24T14:17:27.000Z
drop region constraints for ambiguous goals See the comment in `compute_external_query_constraints`. While the underlying issue is preexisting, this fixes a bug introduced by #125343. It slightly weakens the leak chec, even if we didn't have any test which was affected. I want to write such a test before merging this PR. r? `@compiler-errors`
diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs
@@ -99,6 +99,13 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
             previous call to `try_evaluate_added_goals!`"
         );
 
+        // We only check for leaks from universes which were entered inside
+        // of the query.
+        self.infcx.leak_check(self.max_input_universe, None).map_err(|e| {
+            trace!(?e, "failed the leak check");
+            NoSolution
+        })?;
+
         // When normalizing, we've replaced the expected term with an unconstrained
         // inference variable. This means that we dropped information which could
         // have been important. We handle this by instead returning the nested goals
@@ -121,7 +128,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
         };
 
         let external_constraints =
-            self.compute_external_query_constraints(normalization_nested_goals)?;
+            self.compute_external_query_constraints(certainty, normalization_nested_goals);
         let (var_values, mut external_constraints) =
             (self.var_values, external_constraints).fold_with(&mut EagerResolver::new(self.infcx));
         // Remove any trivial region constraints once we've resolved regions
@@ -170,38 +177,45 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
     #[instrument(level = "trace", skip(self), ret)]
     fn compute_external_query_constraints(
         &self,
+        certainty: Certainty,
         normalization_nested_goals: NestedNormalizationGoals<'tcx>,
-    ) -> Result<ExternalConstraintsData<'tcx>, NoSolution> {
-        // We only check for leaks from universes which were entered inside
-        // of the query.
-        self.infcx.leak_check(self.max_input_universe, None).map_err(|e| {
-            trace!(?e, "failed the leak check");
-            NoSolution
-        })?;
-
-        // Cannot use `take_registered_region_obligations` as we may compute the response
-        // inside of a `probe` whenever we have multiple choices inside of the solver.
-        let region_obligations = self.infcx.inner.borrow().region_obligations().to_owned();
-        let mut region_constraints = self.infcx.with_region_constraints(|region_constraints| {
-            make_query_region_constraints(
-                self.tcx(),
-                region_obligations
-                    .iter()
-                    .map(|r_o| (r_o.sup_type, r_o.sub_region, r_o.origin.to_constraint_category())),
-                region_constraints,
-            )
-        });
-
-        let mut seen = FxHashSet::default();
-        region_constraints.outlives.retain(|outlives| seen.insert(*outlives));
+    ) -> ExternalConstraintsData<'tcx> {
+        // We only return region constraints once the certainty is `Yes`.
+        // This is necessary as we may drop nested goals on ambiguity, which
+        // may result in unconstrained inference variables in the region
+        // constraints. It also prevents us from emitting duplicate region
+        // constraints, avoiding some unnecessary work.
+        //
+        // This slightly weakens the leak check in case it uses region constraints
+        // from an ambiguous nested goal. TODO link test
+        let region_constraints = if certainty == Certainty::Yes {
+            // Cannot use `take_registered_region_obligations` as we may compute the response
+            // inside of a `probe` whenever we have multiple choices inside of the solver.
+            let region_obligations = self.infcx.inner.borrow().region_obligations().to_owned();
+            let mut region_constraints = self.infcx.with_region_constraints(|region_constraints| {
+                make_query_region_constraints(
+                    self.tcx(),
+                    region_obligations.iter().map(|r_o| {
+                        (r_o.sup_type, r_o.sub_region, r_o.origin.to_constraint_category())
+                    }),
+                    region_constraints,
+                )
+            });
+
+            let mut seen = FxHashSet::default();
+            region_constraints.outlives.retain(|outlives| seen.insert(*outlives));
+            region_constraints
+        } else {
+            Default::default()
+        };
 
         let mut opaque_types = self.infcx.clone_opaque_types_for_query_response();
         // Only return opaque type keys for newly-defined opaques
         opaque_types.retain(|(a, _)| {
             self.predefined_opaques_in_body.opaque_types.iter().all(|(pa, _)| pa != a)
         });
 
-        Ok(ExternalConstraintsData { region_constraints, opaque_types, normalization_nested_goals })
+        ExternalConstraintsData { region_constraints, opaque_types, normalization_nested_goals }
     }
 
     /// After calling a canonical query, we apply the constraints returned
diff --git a/tests/ui/traits/next-solver/normalize/ambig-goal-infer-in-type-oulives.rs b/tests/ui/traits/next-solver/normalize/ambig-goal-infer-in-type-oulives.rs
@@ -0,0 +1,29 @@
+//@ check-pass
+//@ compile-flags: -Znext-solver
+//@ ignore-compare-mode-next-solver (explicitly enabled)
+
+// Regression test for an ICE when trying to bootstrap rustc
+// with #125343. An ambiguous goal returned a `TypeOutlives`
+// constraint referencing an inference variable. This inference
+// variable was created inside of the goal, causing it to be
+// unconstrained in the caller. This then caused an ICE in MIR
+// borrowck.
+
+struct Foo<T>(T);
+trait Extend<T> {
+    fn extend<I: IntoIterator<Item = T>>(iter: I);
+}
+
+impl<T> Extend<T> for Foo<T> {
+    fn extend<I: IntoIterator<Item = T>>(_: I) {
+        todo!()
+    }
+}
+
+impl<'a, T: 'a + Copy> Extend<&'a T> for Foo<T> {
+    fn extend<I: IntoIterator<Item = &'a T>>(iter: I) {
+        <Self as Extend<T>>::extend(iter.into_iter().copied())
+    }
+}
+
+fn main() {}
