Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coherence with object types with overlapping supertrait projections is incomplete #133361

Open
compiler-errors opened this issue Nov 23, 2024 · 3 comments · May be fixed by #133397
Open

Coherence with object types with overlapping supertrait projections is incomplete #133361

compiler-errors opened this issue Nov 23, 2024 · 3 comments · May be fixed by #133397
Assignees
Labels
A-coherence Area: Coherence A-trait-objects Area: trait objects, vtable layout C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@compiler-errors
Copy link
Member

compiler-errors commented Nov 23, 2024

I tried this code:

trait Sup<T> {
    type Assoc;
}

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

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

trait Trait {
    type Assoc;
}
impl Trait for dyn Dyn<(), ()> {
    type Assoc = &'static str;
}
impl<A, B> Trait for dyn Dyn<A, B> {
    type Assoc = usize;
}

fn call<A, B>(x: usize) -> <dyn Dyn<A, B> as Trait>::Assoc {
    x
}

fn main() {
    let x: &'static str = call::<(), ()>(0xDEADBEEF);
    println!("{x}");
}

I expected to see this happen: It does not work.

Instead, this happened: Segfault

Meta

rustc --version --verbose:

rustc 1.85.0-nightly (a47555110 2024-11-22)
@compiler-errors compiler-errors added A-coherence Area: Coherence A-trait-objects Area: trait objects, vtable layout C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-types Relevant to the types team, which will review and decide on the PR/issue. labels Nov 23, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 23, 2024
@compiler-errors compiler-errors removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 23, 2024
@compiler-errors
Copy link
Member Author

The reason this fails is because we store object types as fully elaborated lists of projections, but deduplicate them after substitution when they come identical.

Think of dyn Dyn<(), ()> as [Sup<(), Assoc = ()>, Sup<(), Assoc = ()>] which gets deduplicated as [Sup<(), Assoc = ()>]
and dyn Dyn<A, B> which in coherence gets replaced with inference vars to [Sup<?A, Assoc = ?A>, Sup<?B, Assoc = ?B>].

We then check that these lists are the same length, and if they're not, we return a "type mismatch". In effect, we consider the types to be not equal, which also means that we consider two impls to not overlap.

@compiler-errors
Copy link
Member Author

I'm actually somewhat afraid that a general solution to matching up the projections in various states of substitution and deduplication is impossible to solve; however, I think I have in mind a set of restrictions that we may be able to impose that is both not too restrictive but prevents this issue in general.

I'll noodle a bit on this... @rustbot claim

bors added a commit to rust-lang-ci/rust that referenced this issue Nov 23, 2024
[CRATER] Detect cases when user written object assoc bound differs from elaborated non-self-referential bound

an experiment relating to a potential fix for rust-lang#133361

r? `@ghost`
@lcnr lcnr added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 23, 2024
@lcnr lcnr moved this to unknown in T-types unsound issues Nov 23, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 23, 2024
…ss, r=<try>

Fix dyn incompleteness with multiple supertraits with different substitutions

So much to write about this.

Fixes rust-lang#133361

r? `@ghost`
@compiler-errors compiler-errors moved this from unknown to unblocked in T-types unsound issues Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coherence Area: Coherence A-trait-objects Area: trait objects, vtable layout C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
Status: unblocked
Development

Successfully merging a pull request may close this issue.

3 participants