Fix ICE when multiple supertrait substitutions need assoc but only one is provided

[compiler-errors]

Dyn traits must have all of their associated types constrained either by:

1. writing them in the dyn trait itself as an associated type bound, like dyn Iterator<Item = u32>,
2. A supertrait bound, like trait ConstrainedIterator: Iterator<Item = u32> {}, then you may write dyn ConstrainedIterator which doesn't need to mention Item.

However, the object type lowering code did not consider the fact that there may be multiple supertraits with different substitutions, so it just used the associated type's def id as a key for keeping track of which associated types are missing:

rust/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs

Line 131 in 1fc691e

let mut associated_types: FxIndexMap<Span, FxIndexSet<DefId>> = FxIndexMap::default();

This means that we can have missing associated types when there are mutliple supertraits with different substitutions and only one of them is constrained, like:

trait Sup<T> {
    type Assoc: Default;
}

impl<T: Default> Sup<T> for () {
    type Assoc = T;
}
impl<T: Default, U: Default> Dyn<T, U> for () {}

trait Dyn<A, B>: Sup<A, Assoc = A> + Sup<B> {}

The above example allows you to name <dyn Dyn<i32, u32> as Sup<u32>>::Assoc even though it is not possible to project since it's neither constrained by a manually written projection bound or a supertrait bound. This successfully type-checks, but leads to a codegen ICE since we are not able to project the associated type.

This PR fixes the validation for checking that a dyn trait mentions all of its associated type bounds. This is theoretically a breaking change, since you could technically use that dyn Dyn<A, B> type mentionedin the example above without actually projecting to the bad associated type, but I don't expect it to ever be relevant to a user since it's almost certainly a bug. This is corroborated with the crater results¹, which show no failures².

Crater: #133392 (comment)

Fixes #133388

Footnotes

1. I cratered this originally with Fix dyn incompleteness with multiple supertraits with different substitutions #133397, which is a PR that is stacked on top, then re-ran crater with just the failures from that PR. ↩

2. If you look at the crater results, it shows all of the passes as "unknown". I believe this is a crater bug, since looking at the results manually shows them as passes. ↩

[rustbot]

changes to the core type system

cc @compiler-errors, @lcnr

HIR ty lowering was modified

cc @fmease

[compiler-errors]

@bors try

[bors]

⌛ Trying commit 8a10f68 with merge c7377f6...

[bors]

☀️ Try build successful - checks-actions
Build commit: c7377f6 (c7377f633f2c785ef968ab6517e5218297777ae5)

[compiler-errors]

@craterbot check

[craterbot]

👌 Experiment pr-133392 created and queued.
🤖 Automatically detected try build c7377f6
🔍 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

[compiler-errors]

@craterbot check crates=https://crater-reports.s3.amazonaws.com/pr-133397/retry-regressed-list.txt p=1

[craterbot]

👌 Experiment pr-133392-1 created and queued.
🤖 Automatically detected try build c7377f6
⚠️ Try build based on commit 8a10f68, but latest commit is 2a0ede8. Did you forget to make a new try build?
🔍 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

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[lcnr]

@bors r+ rollup

[bors]

📌 Commit 24da053 has been approved by lcnr

It is now in the queue for this repository.

[lcnr]

oh lel, i thought it finished the fcp period 😅

@bors r-

[BoxyUwU]

Kind of funny that you just cant specify the associated type equality bounds here

[compiler-errors]

Yeah, that's a fundamental limitation of Rust atm. I can't be arsed to improve the diagnostic, though I can open an issue for the diagnostic being confusing if you care.

[bors]

☔ The latest upstream changes (presumably #134164) made this pull request unmergeable. Please resolve the merge conflicts.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[compiler-errors]

lcnr already approved it previously, so now that this is done with FCP

@bors r=lcnr

[bors]

📌 Commit 43e2fd5 has been approved by lcnr

It is now in the queue for this repository.