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

Bug with trait inheritance and trait objects #9394

Closed
Kimundi opened this issue Sep 21, 2013 · 9 comments
Closed

Bug with trait inheritance and trait objects #9394

Kimundi opened this issue Sep 21, 2013 · 9 comments
Labels
A-traits Area: Trait system

Comments

@Kimundi
Copy link
Member

Kimundi commented Sep 21, 2013

If you try to call an inherited method on an trait object, it instead calls a regular method of the trait itself, or potentially crashes if the signatures don't match.

Test case:

trait Base {
    fn baz(&self);
}

trait Super: Base {
    fn bar(&self);
}

struct X;

impl Base for X {
    fn baz(&self) {
        println("base baz");
    }
}

impl Super for X {
    fn bar(&self) {
        println("super bar");
    }
}

fn main() {
    let n = X;
    let b = &n as &Base;
    let s = &n as &Super;

    n.baz();   // base baz
    n.bar();   // super bar

    println("");

    b.baz();   // base baz
    //b.bar(); // Base has no bar()

    println("");

    s.baz();   // BUG: super bar - should be base baz
    s.bar();   // super bar
}
@bmaxa
Copy link

bmaxa commented Sep 24, 2013

I have solved this bug, here is patch:

diff --git a/src/librustc/middle/typeck/check/method.rs b/src/librustc/middle/typeck/check/method.rs
index 48d630b..36c9b30 100644
--- a/src/librustc/middle/typeck/check/method.rs
+++ b/src/librustc/middle/typeck/check/method.rs
@@ -372,7 +372,7 @@ impl<'self> LookupContext<'self> {
     // to a trait and its supertraits.
     fn get_method_index(&self,
                         trait_ref: @TraitRef,
-                        subtrait_id: ast::DefId,
+                                               subtrait: @TraitRef,
                         n_method: uint) -> uint {
         let tcx = self.tcx();

@@ -382,15 +382,14 @@ impl<'self> LookupContext<'self> {
         // we find the trait the method came from, counting up the
         // methods from them.
         let mut method_count = 0;
-        do ty::each_bound_trait_and_supertraits(tcx, &[trait_ref])
+        do ty::each_bound_trait_and_supertraits(tcx, &[subtrait])
             |bound_ref| {
-            if bound_ref.def_id == subtrait_id { false }
+            if bound_ref.def_id == trait_ref.def_id { false }
                 else {
-                method_count += ty::trait_methods(tcx, bound_ref.def_id).len();
+                method_count+=ty::trait_methods(tcx, bound_ref.def_id).len();
                 true
             }
         };
-
         return method_count + n_method;
     }

@@ -418,9 +417,9 @@ impl<'self> LookupContext<'self> {
         let trait_ref = @TraitRef { def_id: did, substs: rcvr_substs.clone() };

         do self.push_inherent_candidates_from_bounds_inner(&[trait_ref])
-            |trait_ref, m, method_num, _bound_num| {
+            |new_trait_ref, m, method_num, _bound_num| {
             let vtable_index =
-                self.get_method_index(trait_ref, trait_ref.def_id, method_num);
+                self.get_method_index(new_trait_ref, trait_ref, method_num);
             // We need to fix up the transformed self type.
             let transformed_self_ty =
                 self.construct_transformed_self_ty_for_object(

@bmaxa
Copy link

bmaxa commented Sep 24, 2013

Hmmm, seems that I cannot post it here ;)

@alexcrichton
Copy link
Member

I edited some backticks into your comment so we could see the diff a little better, but feel free to open a pull request! It tends to be easier to review the patch in a pull request than on a bug report...

@bmaxa
Copy link

bmaxa commented Sep 24, 2013

https://gist.github.com/bmaxa/6680566 I have posted to gist , don't know how to make pull request ;)

@jdm
Copy link
Contributor

jdm commented Sep 24, 2013

@bmaxa
Copy link

bmaxa commented Sep 24, 2013

Done ;) url for git is https://github.com/bmaxa/changes_and_fixes/tree/fixes

@Kimundi
Copy link
Member Author

Kimundi commented Sep 24, 2013

@bmaxa You need to actually open an Pull Request.

@bmaxa
Copy link

bmaxa commented Sep 24, 2013

Heh I made pull request to my repository, didn't knew I have to fork first then compare with mozilla/rust
and make request there ;)

bmaxa pushed a commit to bmaxa/rust that referenced this issue Sep 26, 2013
    This solves problem of incorrect indexing into vtable
    when method from super trait was called through pointer
    to derived trait.
    Problem was that offset of super trait vtables
    was not calculated at all.
    Now it works, correct offset is calculated by
    traversing all super traits up to super trait
    where method belongs. That is how it is
    intended to work.
@flaper87
Copy link
Contributor

flaper87 commented May 5, 2014

Triage: This was fixed in a8a69ec (contains a regression test)

@flaper87 flaper87 closed this as completed May 5, 2014
Jarcho pushed a commit to Jarcho/rust that referenced this issue Aug 31, 2022
Fix missing parens in `suboptimal_flops` suggestion

Fixes rust-lang#9391. The problem is simple enough, I didn't check if the same problem occurs elsewhere, though.

changelog: fix missing parenthesis in `suboptimal_flops` suggestion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system
Projects
None yet
Development

No branches or pull requests

5 participants