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

Rustc selects the wrong impl for trait objects when a trait is implemented multiple times with different parameters on the same struct #26339

Closed
jorisgio opened this issue Jun 16, 2015 · 4 comments
Labels
P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jorisgio
Copy link

In the following test case, rustc selects the second implementation instead of the first one :

trait A : PartialEq<Foo> + PartialEq<Bar> { }

struct Foo;
struct Bar;

struct Aimpl;

impl PartialEq<Foo> for Aimpl {
    fn eq(&self, _rhs: &Foo) -> bool {
        panic!("Compare with foo")
    }
}

impl PartialEq<Bar> for Aimpl {
    fn eq(&self, _rhs: &Bar) -> bool {
        panic!("Compare with bar")
    }
}

impl A for Aimpl { }

fn main() {
    let a = &Aimpl as &A;

    *a == Foo;
}

This can lead to uninitialised memory access. Thanks to bluss for further reducing the testcase.

@jorisgio jorisgio changed the title Rustc selects the wrong impl when a trait is implemented multiple times with different parameters on the same struct Rustc selects the wrong impl for trait objects when a trait is implemented multiple times with different parameters on the same struct Jun 16, 2015
@jorisgio
Copy link
Author

I'm starting to be convinced that such a trait shouldn't be object safe, it introduces dynamic multi-dispatch into the language.

@bluss bluss added the I-wrong label Jun 16, 2015
@nikomatsakis nikomatsakis added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 16, 2015
@nikomatsakis
Copy link
Contributor

triage: P-high

@rust-highfive rust-highfive added the P-high High priority label Jun 16, 2015
@eefriedman
Copy link
Contributor

Looks like the immediate issue is that get_vtable_index_of_object_method isn't finding the right trait: it cuts off its search at the first trait with a matching def_id, without considering the type arguments. I'm not exactly sure how to compare two PolyTraitRefs though.

(I don't think handling cases like this requires dynamic multi-dispatch: the trait A contains PartialEq<Foo> and PartialEq<Bar> in its vtable, and any call is statically resolved to one or the other.)

@nikomatsakis
Copy link
Contributor

A couple of notes:

  1. This was intended to work, and it indeed does not require dynamic multiple dispatch, though it does allow you to emulate overloading based on argument type to some extent.
  2. The code was originally written with the assumption however that a given supertrait occured at most once, so yeah it looks like some of it was not correctly updated. Off the top of my head, I'm not sure what's required to check the full trait ref, I'll have to go look at the code.
  3. I feel like this is a dup of an issue somewhere :)

@eddyb eddyb closed this as completed in 536e71b Jul 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants