From a31cbdd35fa7866cf186f80a64c63b2069facabe Mon Sep 17 00:00:00 2001 From: lcnr Date: Wed, 22 May 2024 16:56:10 +0000 Subject: [PATCH 1/2] drop region constraints for ambiguous goals --- .../src/solve/eval_ctxt/canonical.rs | 64 +++++++++++-------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs index 9590a82c0677f..06d98308426a2 100644 --- 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,30 +177,37 @@ 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, 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 @@ -201,7 +215,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> { 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 From f98196ffbbf28639258e8eeaca13f2d14f74e5d5 Mon Sep 17 00:00:00 2001 From: lcnr Date: Wed, 22 May 2024 17:01:23 +0000 Subject: [PATCH 2/2] add a regression test --- .../ambig-goal-infer-in-type-oulives.rs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/ui/traits/next-solver/normalize/ambig-goal-infer-in-type-oulives.rs 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 new file mode 100644 index 0000000000000..18dc34f7cc437 --- /dev/null +++ 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); +trait Extend { + fn extend>(iter: I); +} + +impl Extend for Foo { + fn extend>(_: I) { + todo!() + } +} + +impl<'a, T: 'a + Copy> Extend<&'a T> for Foo { + fn extend>(iter: I) { + >::extend(iter.into_iter().copied()) + } +} + +fn main() {}