don't ignore ambiguity during MIR type check by lcnr · Pull Request #164055 · rust-lang/rust

lcnr · 2026-10-09T20:46:26Z

this was previously fixed in #116530 and later reverted in #118736.

It impacts the new solver more than the old one as the new solver depends significantly more on region identity, see the relevant stabilization report section.

The old solver has significantly less region identity dependence. It does rely on region identity when merging candidates during normalization:

rust/compiler/rustc_trait_selection/src/traits/project.rs

Lines 103 to 107 in 69bccf0

    
           // Duplicates can happen inside ParamEnv. In the case, we 
        
           // perform a lazy deduplication. 
        
           if current == &candidate { 
        
               return false; 
        
           }

This check results in additional ICE with the old solver while instead triggering the potentially region dependent goals check with the new one

rust/compiler/rustc_trait_selection/src/traits/query/type_op/custom.rs

Lines 94 to 95 in 14f0018

    
           let value = op(&ocx).map_err(|_| { 
        
               infcx.tcx.check_potentially_region_dependent_goals(root_def_id).err().unwrap_or_else(

r? types

rustbot · 2026-10-09T20:46:51Z

🤖 A dedicated #t-types/nominated topic has been opened for humans to discuss this issue 🤖

rustbot · 2026-10-28T14:44:02Z

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

RalfJung · 2026-10-28T16:35:34Z

@@ -1,7 +1,10 @@
 // Test for <https://github.com/rust-lang/rust/issues/135316>.


Is a fail test the best test we can have now for #135316 ? It means we're not even hitting vtable generation any more.

View changes since the review

jackh726

Several of these tests I wouldn't expect to error because I'm not understand what region constraints we're generating.

Seems like the (undesired, probably) fallback is basically all around dyn Trait? We don't get similar fallout from non-dyn multiple super traits?

Have we done crater with these changes yet? The breakage to the MIRI test has me worried about whether we may see this pop up often.

If there is undesired breakage here, I think we should mark those tests as known-bug.

View changes since this review

jackh726 · 2026-11-01T00:06:13Z

-    (&() as &'static dyn for<'a> Trait<&'static (), &'a ()>
-        as &'static dyn Trait<&'static (), &'static ()>)
-        .say_hello(&0);
+    // We can't have `dyn Trait<&'static (), &'static ()>` directly here
+    // as that results in an ambiguity error in MIR borrowck.
+    fn generic<T: 'static>() {
+        (&() as &'static dyn for<'a> Trait<T, &'a ()> as &'static dyn Trait<T, &'static ()>)
+            .say_hello(&0);
+    }
+
+    generic::<&'static ()>();


Hmm. Breaking MIRI does not bode well for the fallout here.

proving dyn Trait<&'x (), &'y ()>: Supertrait<&'z ()> is ambiguous as it either requires 'z = 'x or 'z = 'y. The reason this previously compiled with -Znext-solver is that.

Your last sentence cut off I think.

Proving dyn Trait<&'x (), &'y ()>: Supertrait<&'z ()> is ambiguous as it either requires 'z = 'x or 'z = 'y. The reason this previously compiled with -Znext-solver is that during HIR typeck succeeds as it has Trait<&'static (), &'static ()> while MIR borrowck proves it for Trait<&'x (), &'y ()> and the resulting ambiguity got ignored.

The explanation here is in #164055 (comment)

jackh726 · 2026-11-01T00:13:52Z

    (&() as &dyn D<&(), &()>).f()
+    //[next]~^ ERROR: type annotations needed: cannot satisfy `dyn D<&(), &()>: B<&()>`


So...this on first glance this really doesn't seem to me like it should fail. I guess from a technical perspective, I can probably see why it does (lack of OR constraints) but even that I'm unsure about, because I would expect that D<C, E>: B<C> and D<C, E>: B<E> to be able to be solved separately without region constraints.

Given that I would expect that this test be marked as known-bug under the next-solver.

This one is odd because it should also just be ambiguous during HIR typeck already.

For dyn D<&'a (), &'b ()> HIR typeck explicitly proves B<&'a ()> and B<&'b ()>, both of which work as one of the candidates has no region constraints. The MIR body has the function item <dyn D<&'a (), &'b ()> as D<&'c (), &'d ()>>::f, proving dyn D<&'a (), &'b ()>: D<&'c (), &'d ()> for that one is ambiguous.

jackh726 · 2026-11-01T00:15:09Z


 fn bar<'x, 'y>() {
    impls_trait::<'y, _>(foo::<'x, 'y>());
+    //[next]~^ ERROR: type annotations needed: cannot satisfy `impl Trait<'_> + Trait<'_>: Trait<'_>`


Similar point to above.

jackh726 · 2026-11-01T00:16:13Z

+// Need the ambiguous `T: Sup<'a, (), Assoc = &'static str>` bound to only be
+// used inside of the trait impl, as otherwise we also encounter ambiguity
+// during normalization, which ICEs instead of ignoring it.
+impl<'a, T: Sup<'a, (), Assoc = &'static str> + ?Sized> HideMe for T {


So, this one clearly should not pass.

jackh726 · 2026-11-01T00:18:34Z

+trait D<'a, 'b>: Tr<'a> + Tr<'b> {}
+//[current]~^ ERROR: type annotations needed: cannot satisfy `Self: Tr<'a>`
+
+impl<'t> Tr<'t> for &'t str { fn get(&self) -> &'t str { *self } }


This one I can see failing, because this does register a region constraint.

lcnr · 2026-11-02T08:20:47Z

so in general, we tend to have dyn Trait<'a, 'b>: Sup<'a> or dyn Trait<'a, 'a>: Sup<'b> during HIR typeck, where the two trait Trait<'a, 'b>: Sup<'a> + Sup<'b> candidates have the same constraints, and then in MIR borrowck we then have to prove dyn Trait<'x, 'y>: Sup<'z>, where the two candidates have different constraints.

Seems like the (undesired, probably) fallback is basically all around dyn Trait? We don't get similar fallout from non-dyn multiple super traits?

This is effectively only a problem for the builtin dyn Trait candidates if there are two equal-modulo regions super trait bounds, or similarly, you could have the same issue with item bounds for <T as Trait>::Assoc<'a, 'a> with type Assoc<'a, 'b>: Tr<'a> + Tr<'b>

Have we done crater with these changes yet? The breakage to the MIRI test has me worried about whether we may see this pop up often.

This PR reverts the behavior back to the "correct" behavior pre #160982. We tested that behavior as part of full next-solver crater runs in #133502. Most notably the crater run in #133502 (comment). We didn't encounter any meaningful breakage in that crater run because of this.

This affects cases which previously ICEd and were tracked in rust-lang/trait-system-refactor-initiative#27. Even back when the cases which now error resulted in ICE, this was something we didn't really encounter.

I would personally just merge this.

lcnr · 2026-11-02T08:23:51Z

If there is undesired breakage here, I think we should mark those tests as known-bug.

This breakage does feel... fairly desired to me. Or well, it is intended - but undesirable - breakage and for this correctly work, we do need OR region constraints.

rustbot · 2026-11-10T15:01:52Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

jackh726 · 2026-11-10T16:32:45Z

@rfcbot merge types

rust-rfcbot · 2026-11-10T16:32:50Z

@jackh726 has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

lcnr · 2026-11-12T21:14:08Z

so in general, we tend to have dyn Trait<'a, 'b>: Sup<'a> or dyn Trait<'a, 'a>: Sup<'b> during HIR typeck, where the two trait Trait<'a, 'b>: Sup<'a> + Sup<'b> candidates have the same constraints, and then in MIR borrowck we then have to prove dyn Trait<'x, 'y>: Sup<'z>, where the two candidates have different constraints.

Seems like the (undesired, probably) fallback is basically all around dyn Trait? We don't get similar fallout from non-dyn multiple super traits?

This is effectively only a problem for the builtin dyn Trait candidates if there are two equal-modulo regions super trait bounds, or similarly, you could have the same issue with item bounds for <T as Trait>::Assoc<'a, 'a> with type Assoc<'a, 'b>: Tr<'a> + Tr<'b>

Have we done crater with these changes yet? The breakage to the MIRI test has me worried about whether we may see this pop up often.

This PR reverts the behavior back to the "correct" behavior pre #160982. We tested that behavior as part of full next-solver crater runs in #133502. Most notably the crater run in #133502 (comment). We didn't encounter any meaningful breakage in that crater run because of this.

This affects cases which previously ICEd and were tracked in rust-lang/trait-system-refactor-initiative#27. Even back when the cases which now error resulted in ICE, this was something we didn't really encounter.

I would personally just merge this.

@rfcbot reviewed

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Oct 9, 2026

rustbot assigned jackh726 Oct 9, 2026

lcnr added the I-types-nominated Nominated for discussion during a types team meeting. label Oct 9, 2026