From 83e6da0be5e0e7cec8326ad5c4ba463299d32eea Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 9 May 2024 17:11:51 +0000 Subject: [PATCH 1/2] analyse visitor: build proof tree in probe --- .../src/solve/eval_ctxt/canonical.rs | 1 + .../src/solve/inspect/analyse.rs | 44 ++++++++++++++----- .../ambiguity-causes-canonical-state-ice-1.rs | 43 ++++++++++++++++++ .../ambiguity-causes-canonical-state-ice-2.rs | 19 ++++++++ ...iguity-causes-canonical-state-ice-2.stderr | 11 +++++ .../incompleteness-unstable-result.rs | 2 +- .../incompleteness-unstable-result.stderr | 13 +++--- 7 files changed, 113 insertions(+), 20 deletions(-) create mode 100644 tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-1.rs create mode 100644 tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-2.rs create mode 100644 tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-2.stderr 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 5f73432750d10..41b304ec53852 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs @@ -409,6 +409,7 @@ pub(in crate::solve) fn make_canonical_state<'tcx, T: TypeFoldable> /// This currently assumes that unifying the var values trivially succeeds. /// Adding any inference constraints which weren't present when originally /// computing the canonical query can result in bugs. +#[instrument(level = "debug", skip(infcx, span, param_env))] pub(in crate::solve) fn instantiate_canonical_state<'tcx, T: TypeFoldable>>( infcx: &InferCtxt<'tcx>, span: Span, diff --git a/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs b/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs index fa4323a3a944d..71ae410a03f98 100644 --- a/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs +++ b/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs @@ -146,6 +146,11 @@ impl<'a, 'tcx> InspectCandidate<'a, 'tcx> { /// inference constraints, and optionally the args of an impl if this candidate /// came from a `CandidateSource::Impl`. This function modifies the state of the /// `infcx`. + #[instrument( + level = "debug", + skip_all, + fields(goal = ?self.goal.goal, nested_goals = ?self.nested_goals) + )] pub fn instantiate_nested_goals_and_opt_impl_args( &self, span: Span, @@ -213,10 +218,23 @@ impl<'a, 'tcx> InspectCandidate<'a, 'tcx> { }; let goal = goal.with(infcx.tcx, ty::NormalizesTo { alias, term: unconstrained_term }); - let proof_tree = EvalCtxt::enter_root(infcx, GenerateProofTree::Yes, |ecx| { - ecx.evaluate_goal_raw(GoalEvaluationKind::Root, GoalSource::Misc, goal) - }) - .1; + // We have to use a `probe` here as evaluating a `NormalizesTo` can constrain the + // expected term. This means that candidates which only fail due to nested goals + // and which normalize to a different term then the final result could ICE: when + // building their proof tree, the expected term was unconstrained, but when + // instantiating the candidate it is already constrained to the result of another + // candidate. + let proof_tree = infcx + .probe(|_| { + EvalCtxt::enter_root(infcx, GenerateProofTree::Yes, |ecx| { + ecx.evaluate_goal_raw( + GoalEvaluationKind::Root, + GoalSource::Misc, + goal, + ) + }) + }) + .1; InspectGoal::new( infcx, self.goal.depth + 1, @@ -225,13 +243,17 @@ impl<'a, 'tcx> InspectCandidate<'a, 'tcx> { source, ) } - _ => InspectGoal::new( - infcx, - self.goal.depth + 1, - infcx.evaluate_root_goal(goal, GenerateProofTree::Yes).1.unwrap(), - None, - source, - ), + _ => { + // We're using a probe here as evaluating a goal could constrain + // inference variables by choosing one candidate. If we then recurse + // into another candidate who ends up with different inference + // constraints, we get an ICE if we already applied the constraints + // from the chosen candidate. + let proof_tree = infcx + .probe(|_| infcx.evaluate_root_goal(goal, GenerateProofTree::Yes).1) + .unwrap(); + InspectGoal::new(infcx, self.goal.depth + 1, proof_tree, None, source) + } }) .collect(); diff --git a/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-1.rs b/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-1.rs new file mode 100644 index 0000000000000..151c3b226c114 --- /dev/null +++ b/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-1.rs @@ -0,0 +1,43 @@ +//@ compile-flags: -Znext-solver=coherence +//@ check-pass + +// A regression test for #124791. Computing ambiguity causes +// for the overlap of the `ToString` impls caused an ICE. +#![crate_type = "lib"] +#![feature(min_specialization)] +trait Display {} + +trait ToOwned { + type Owned; +} + +impl ToOwned for T { + type Owned = T; +} + +struct Cow(B); + +impl Display for Cow +where + B: ToOwned, + B::Owned: Display, +{ +} + +impl Display for () {} + +trait ToString { + fn to_string(); +} + +impl ToString for T { + default fn to_string() {} +} + +impl ToString for Cow { + fn to_string() {} +} + +impl ToOwned for str { + type Owned = (); +} diff --git a/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-2.rs b/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-2.rs new file mode 100644 index 0000000000000..b472499cb0bf7 --- /dev/null +++ b/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-2.rs @@ -0,0 +1,19 @@ +//@ compile-flags: -Znext-solver=coherence + +// A regression test for #124791. Computing ambiguity causes +// for the overlap of the `ToString` impls caused an ICE. +#![crate_type = "lib"] +trait ToOwned { + type Owned; +} +impl ToOwned for T { + type Owned = u8; +} +impl ToOwned for str { + type Owned = i8; +} + +trait Overlap {} +impl + ?Sized> Overlap for T {} +impl Overlap for str {} +//~^ ERROR conflicting implementations of trait `Overlap` diff --git a/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-2.stderr b/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-2.stderr new file mode 100644 index 0000000000000..469f7a909b141 --- /dev/null +++ b/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice-2.stderr @@ -0,0 +1,11 @@ +error[E0119]: conflicting implementations of trait `Overlap` for type `str` + --> $DIR/ambiguity-causes-canonical-state-ice-2.rs:18:1 + | +LL | impl + ?Sized> Overlap for T {} + | --------------------------------------------------- first implementation here +LL | impl Overlap for str {} + | ^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `str` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0119`. diff --git a/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.rs b/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.rs index 8bb4ff469076c..7eea81ce03c66 100644 --- a/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.rs +++ b/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.rs @@ -61,7 +61,7 @@ where // entering the cycle from `A` fails, but would work if we were to use the cache // result of `B`. impls_trait::, _, _, _>(); - //~^ ERROR the trait bound `A: Trait` is not satisfied + //~^ ERROR the trait bound `A: Trait<_, _, _>` is not satisfied } fn main() { diff --git a/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr b/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr index cdb4ff4588f75..ffa3f29e4bd6f 100644 --- a/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr +++ b/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr @@ -1,10 +1,11 @@ -error[E0277]: the trait bound `A: Trait` is not satisfied +error[E0277]: the trait bound `A: Trait<_, _, _>` is not satisfied --> $DIR/incompleteness-unstable-result.rs:63:19 | LL | impls_trait::, _, _, _>(); - | ^^^^ the trait `Trait` is not implemented for `A`, which is required by `A: Trait<_, _, _>` + | ^^^^ the trait `Trait<_, _, _>` is not implemented for `A`, which is required by `A: Trait<_, _, _>` | -note: required for `A` to implement `Trait` + = help: the trait `Trait` is implemented for `A` +note: required for `A` to implement `Trait<_, _, _>` --> $DIR/incompleteness-unstable-result.rs:32:50 | LL | impl Trait for A @@ -13,16 +14,12 @@ LL | impl Trait for A LL | A: Trait, | -------------- unsatisfied trait bound introduced here = note: 8 redundant requirements hidden - = note: required for `A` to implement `Trait` + = note: required for `A` to implement `Trait<_, _, _>` note: required by a bound in `impls_trait` --> $DIR/incompleteness-unstable-result.rs:51:28 | LL | fn impls_trait, U: ?Sized, V: ?Sized, D: ?Sized>() {} | ^^^^^^^^^^^^^^ required by this bound in `impls_trait` -help: consider extending the `where` clause, but there might be an alternative better way to express this requirement - | -LL | X: IncompleteGuidance, A: Trait - | ~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to 1 previous error From feff7520dfcc8d52bef92b89a5b7ed970255f89f Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 9 May 2024 17:51:05 +0000 Subject: [PATCH 2/2] update crashes --- tests/crashes/124702.rs | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 tests/crashes/124702.rs diff --git a/tests/crashes/124702.rs b/tests/crashes/124702.rs deleted file mode 100644 index e3767dec4036c..0000000000000 --- a/tests/crashes/124702.rs +++ /dev/null @@ -1,14 +0,0 @@ -//@ known-bug: rust-lang/rust#124702 -//@ compile-flags: -Znext-solver=coherence -trait X {} - -trait Z { - type Assoc: Y; -} -struct A(T); - -impl Z for A { - type Assoc = T; -} - -impl From<> as Z>::Assoc> for T {}