Require coherence of supertrait associated item bounds for dyn-compability by theemathas · Pull Request #158915 · rust-lang/rust

theemathas · 2026-07-07T15:00:56Z

This PR is an alternative to #157710. However, unlike that PR, this PR does not address #150936.

In a dyn type, if multiple bounds are specified via supertraits for the same associated item, then we previously accepted them if the relevant trait's generics are different, even if the bounds conflict.

This was unsound, since those generics could end up being instantiated with identical concrete types, causing the dyn type to have two different "values" for the same bound.

Thus, if a trait has multiple supertrait bounds for the same associated item, we check whether those bounds are coherent, similarly to how we check for overlap between impls (i.e., we check if the generics could be instantiated to be the same while the "values" of the bounds are different). If the bounds are incoherent, then we consider the trait to be dyn-incompatible.

r? @fmease

theemathas · 2026-07-07T15:02:01Z

I'm unsure of what to do with the now-failing tests.

I'm also rather unsure of my code in general. Please be careful with the review to make sure I didn't do anything stupid. Thank you.

theemathas · 2026-07-21T12:58:50Z

r? types

lcnr · 2026-07-29T10:46:05Z

after that, we should do a crater run 😁

lcnr · 2026-07-29T10:46:29Z

@rustbot author

rustbot · 2026-07-29T10:46:35Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

theemathas · 2026-08-16T16:25:08Z

I decided to ask an LLM for assistance with understanding how the trait solver API works, and with double-checking my code for any glaring issues. I still wrote all code manually. In particular, the LLM did the following:

Explained the difference between InferCtxt and ObligationCtxt. (The documentation was unfortunately not very helpful to me here. I previously didn't realize that my previous InferCtxt usage generated nested obligations that weren't automatically solved for.)
Explained the bug in my initial implementation attempt (which incorrectly called fork_with_typing_mode in an attempt to "invert the polarity" between the two ObligationCtxts).
Pointed out the existence of plug_infer_with_placeholders, which I verified to have the behavior that I needed.
Pointed out that I needed to call resolve_regions. (I think that this is a rather unfortunate footgun that doesn't seem to be properly documented.)
Pointed out a potential problem due to me not instantiating the param_env. I don't understand what this problem is, so I'm deferring to you on what to do here. See the comment below.

I claim full responsibility of all code in this PR.

theemathas · 2026-08-16T16:27:22Z

+    // Now that we've constrained the two projections to be on the same thing,
+    // we check whether the two terms are necessarily equal to each other.
+    // If they are, then the two projections are coherent.
+    plug_infer_with_placeholders(&infcx, ty::UniverseIndex::ROOT, (proj_1, proj_2));
+    let ocx = ObligationCtxt::new(&infcx);
+    ocx.eq(&ObligationCause::dummy(), param_env, proj_1.term, proj_2.term).is_ok()
+        && ocx.evaluate_obligations_error_on_ambiguity().no_errors()
+        && ocx.resolve_regions(CRATE_DEF_ID, param_env, []).is_empty()
+}


The LLM, in reviewing my code, told me that I have to instantiate the param_env with trait_args here. However, I should not do this instantiation in the can_equate_generics computation. Is the LLM right here? I don't understand at all what this does.

View changes since the review

theemathas · 2026-08-16T16:28:57Z

This test is currently the only pre-existing test that this PR breaks. What should be done about this test?

View changes since the review

theemathas · 2026-08-16T16:30:43Z

Is this PR ready for a crater run?

@rustbot ready

theemathas · 2026-08-23T07:20:04Z

I just realized that the current implementation has a weird consequence: It is possible for a dyn-compatible trait to be a subtrait of a dyn-incompatible trait. For example:

trait Super<T> {
    type Assoc;
}
trait Sub<T, U>: Super<T, Assoc = i32> + Super<U, Assoc = i64> {}
trait Sub2: Sub<i32, i64> {}

In the above code, dyn Sub2 is legal, but dyn Sub<i32, i64> is not. Is this a problem? Does this cause problems with vtable generation?

rust-timer · 2026-09-07T16:59:38Z

Finished benchmarking commit (e3607d5): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 1.9%, secondary 4.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.9%	[0.7%, 2.9%]	5
Regressions ❌ (secondary)	4.0%	[2.1%, 7.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.9%	[0.7%, 2.9%]	5

Cycles

Results (primary -1.6%, secondary 2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.6%, 0.8%]	2
Regressions ❌ (secondary)	2.8%	[2.7%, 2.9%]	2
Improvements ✅ (primary)	-2.7%	[-5.3%, -0.5%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.6%	[-5.3%, 0.8%]	6

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 478.991s -> 480.616s (0.34%)
Artifact size: 403.45 MiB -> 403.52 MiB (0.02%)

theemathas · 2026-09-14T11:40:35Z

Pushed a fix. I'll finagle with the commit splitting later. Let's rerun crater.

@bors try

Require coherence of supertrait associated item bounds for dyn-compability

rust-bors · 2026-09-14T12:44:18Z

☀️ Try build successful (CI)
Build commit: 0335fc7 (0335fc7ee9e33d92e03d99f3e2fb0fe6119f3cc0)
Base parent: ed77b7b (ed77b7b8699e342c2dc842c83cb5fe4025252ee8)

theemathas · 2026-09-14T12:52:53Z

@craterbot check

craterbot · 2026-09-14T12:53:37Z

👌 Experiment pr-158915-1 created and queued.
🤖 Automatically detected try build 0335fc7
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2026-09-21T08:07:00Z

🚧 Experiment pr-158915-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2026-09-23T18:15:26Z

🎉 Experiment pr-158915-1 is completed!
📊 1 regressed and 0 fixed (1116832 total)
📊 5994 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-158915-1/retry-regressed-list.txt ↩

theemathas · 2026-09-24T00:10:16Z

Crater is clean.

…ics. See rust-lang#154662 These tests will be fixed in a subsequent commit.

…ility Fixes rust-lang#154662 In a `dyn` type, if multiple bounds are specified via supertraits for the same associated item, then we previously accepted them if the relevant trait's generics are different, even if the bounds conflict. This was unsound, since those generics could end up being instantiated with identical concrete types, causing the `dyn` type to have two different "values" for the same bound. Thus, if a trait has multiple supertrait bounds for the same associated item, we check whether those bounds are coherent, similarly to how we check for overlap between impls (i.e., we check if the generics could be instantiated to be the same while the "values" of the bounds are different). If the bounds are incoherent, then we consider the trait to be dyn-incompatible.

rustbot · 2026-09-24T14:41:24Z

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.

theemathas · 2026-09-24T14:41:41Z

Squashed into 2 commits, and rebased. Ready for review, and hopefully ready for FCP.

rustbot assigned fmease Jul 7, 2026

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 Jul 7, 2026

theemathas added needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. labels Jul 7, 2026

theemathas force-pushed the coherent-dyn-compat branch from 4d67fa1 to 256670e Compare July 7, 2026 15:07

rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Jul 21, 2026

rustbot assigned lcnr and unassigned fmease Jul 21, 2026

lcnr reviewed Jul 29, 2026

View reviewed changes

Comment thread compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs Outdated

Comment thread compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs Outdated

Comment thread compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs Outdated

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 29, 2026

This comment has been minimized.

Sign in to view

theemathas force-pushed the coherent-dyn-compat branch from 256670e to 4adf718 Compare August 16, 2026 09:25

This comment has been minimized.

Sign in to view

theemathas force-pushed the coherent-dyn-compat branch from 4adf718 to 97592bc Compare August 16, 2026 16:10

theemathas commented Aug 16, 2026

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 16, 2026

theemathas force-pushed the coherent-dyn-compat branch from 97592bc to 8546c21 Compare August 18, 2026 13:09

craterbot added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Sep 7, 2026

This comment was marked as outdated.

Sign in to view

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Sep 13, 2026

This comment was marked as outdated.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 14, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Sep 14, 2026

Auto merge of #158915 - theemathas:coherent-dyn-compat, r=<try>

0335fc7

Require coherence of supertrait associated item bounds for dyn-compability

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 14, 2026

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Sep 23, 2026

This comment has been minimized.

Sign in to view

theemathas force-pushed the coherent-dyn-compat branch from 5a1c765 to 2c711a8 Compare September 24, 2026 14:34

This comment has been minimized.

Sign in to view

theemathas added 2 commits September 24, 2026 21:35

Add tests for conflicting associated type bounds with different gener…

a56a70d

…ics. See rust-lang#154662 These tests will be fixed in a subsequent commit.

theemathas force-pushed the coherent-dyn-compat branch from 2c711a8 to 20cb1f3 Compare September 24, 2026 14:41

Uh oh!

Conversation

theemathas commented Jul 7, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theemathas commented Jul 7, 2026

Uh oh!

theemathas commented Jul 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lcnr commented Jul 29, 2026

Uh oh!

lcnr commented Jul 29, 2026

Uh oh!

rustbot commented Jul 29, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

theemathas commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theemathas Aug 16, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theemathas Aug 16, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theemathas commented Aug 16, 2026

Uh oh!

theemathas commented Aug 23, 2026

Uh oh!

rust-timer commented Sep 7, 2026

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

theemathas commented Sep 14, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Sep 14, 2026

Uh oh!

theemathas commented Sep 14, 2026

Uh oh!

craterbot commented Sep 14, 2026

Uh oh!

craterbot commented Sep 21, 2026

Uh oh!

craterbot commented Sep 23, 2026

Footnotes

Uh oh!

This comment has been minimized.

theemathas commented Sep 24, 2026

Uh oh!

This comment has been minimized.

rustbot commented Sep 24, 2026

Uh oh!

theemathas commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

theemathas commented Jul 7, 2026 •

edited by rustbot

Loading

theemathas commented Aug 16, 2026 •

edited

Loading

theemathas Aug 16, 2026 •

edited by rustbot

Loading

theemathas Aug 16, 2026 •

edited by rustbot

Loading

theemathas commented Sep 24, 2026 •

edited

Loading