Fix a latent bug in trait dispatch where we sometimes counted associated types

when constructing the vtable-index. Not good.

Author: nikomatsakis

src/librustc/middle/astencode.rs

@@ -904,8 +904,8 @@ impl<'a, 'tcx> rbml_writer_helpers<'tcx> for Encoder<'a> {
                             try!(this.emit_struct_field("method_num", 0, |this| {
                                 this.emit_uint(o.method_num)
                             }));
-                            try!(this.emit_struct_field("real_index", 0, |this| {
-                                this.emit_uint(o.real_index)
+                            try!(this.emit_struct_field("vtable_index", 0, |this| {
+                                this.emit_uint(o.vtable_index)
                             }));
                             Ok(())
                         })
@@ -1492,8 +1492,8 @@ impl<'a, 'tcx> rbml_decoder_decoder_helpers<'tcx> for reader::Decoder<'a> {
                                             this.read_uint()
                                         }).unwrap()
                                     },
-                                    real_index: {
-                                        this.read_struct_field("real_index", 3, |this| {
+                                    vtable_index: {
+                                        this.read_struct_field("vtable_index", 3, |this| {
                                             this.read_uint()
                                         }).unwrap()
                                     },

src/librustc/middle/traits/select.rs

@@ -214,7 +214,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         self.closure_typer.param_env()
     }
 
-    pub fn closure_typer(&self) -> &'cx (ty::UnboxedClosureTyper<'tcx>+'cx) {
+    pub fn closure_typer(&self) -> &'cx (ty::ClosureTyper<'tcx>+'cx) {
         self.closure_typer
     }
 

src/librustc/middle/traits/util.rs

@@ -329,58 +329,67 @@ pub fn upcast<'tcx>(tcx: &ty::ctxt<'tcx>,
 pub fn get_vtable_index_of_object_method<'tcx>(tcx: &ty::ctxt<'tcx>,
                                                object_trait_ref: ty::PolyTraitRef<'tcx>,
                                                trait_def_id: ast::DefId,
-                                               method_index_in_trait: uint) -> uint {
+                                               method_offset_in_trait: uint) -> uint {
     // We need to figure the "real index" of the method in a
     // listing of all the methods of an object. We do this by
     // iterating down the supertraits of the object's trait until
     // we find the trait the method came from, counting up the
     // methods from them.
     let mut method_count = 0;
-    ty::each_bound_trait_and_supertraits(tcx, &[object_trait_ref], |bound_ref| {
+
+    for bound_ref in transitive_bounds(tcx, &[object_trait_ref]) {
         if bound_ref.def_id() == trait_def_id {
-            false
-        } else {
-            let trait_items = ty::trait_items(tcx, bound_ref.def_id());
-            for trait_item in trait_items.iter() {
-                match *trait_item {
-                    ty::MethodTraitItem(_) => method_count += 1,
-                    ty::TypeTraitItem(_) => {}
-                }
+            break;
+        }
+
+        let trait_items = ty::trait_items(tcx, bound_ref.def_id());
+        for trait_item in trait_items.iter() {
+            match *trait_item {
+                ty::MethodTraitItem(_) => method_count += 1,
+                ty::TypeTraitItem(_) => {}
             }
-            true
         }
+    }
+
+    // count number of methods preceding the one we are selecting and
+    // add them to the total offset; skip over associated types.
+    let trait_items = ty::trait_items(tcx, trait_def_id);
+    for trait_item in trait_items.iter().take(method_offset_in_trait) {
+        match *trait_item {
+            ty::MethodTraitItem(_) => method_count += 1,
+            ty::TypeTraitItem(_) => {}
+        }
+    }
+
+    // the item at the offset we were given really ought to be a method
+    assert!(match trait_items[method_offset_in_trait] {
+        ty::MethodTraitItem(_) => true,
+        ty::TypeTraitItem(_) => false
     });
-    method_count + method_index_in_trait
+
+    method_count
 }
 
-pub fn unboxed_closure_trait_ref_and_return_type<'tcx>(
-    closure_typer: &ty::UnboxedClosureTyper<'tcx>,
+pub enum TupleArgumentsFlag { Yes, No }
+
+pub fn closure_trait_ref_and_return_type<'tcx>(
+    tcx: &ty::ctxt<'tcx>,
     fn_trait_def_id: ast::DefId,
     self_ty: Ty<'tcx>,
-    closure_def_id: ast::DefId,
-    substs: &Substs<'tcx>)
+    sig: &ty::PolyFnSig<'tcx>,
+    tuple_arguments: TupleArgumentsFlag)
     -> ty::Binder<(Rc<ty::TraitRef<'tcx>>, Ty<'tcx>)>
 {
-    let tcx = closure_typer.param_env().tcx;
-    let closure_type = closure_typer.unboxed_closure_type(closure_def_id, substs);
-
-    debug!("unboxed_closure_trait_ref: closure_def_id={} closure_type={}",
-           closure_def_id.repr(tcx),
-           closure_type.repr(tcx));
-
-    let closure_sig = &closure_type.sig;
-    let arguments_tuple = closure_sig.0.inputs[0];
-    let trait_substs =
-        Substs::new_trait(
-            vec![arguments_tuple],
-            vec![],
-            self_ty);
+    let arguments_tuple = match tuple_arguments {
+        TupleArgumentsFlag::No => sig.0.inputs[0],
+        TupleArgumentsFlag::Yes => ty::mk_tup(tcx, sig.0.inputs.to_vec()),
+    };
+    let trait_substs = Substs::new_trait(vec![arguments_tuple], vec![], self_ty);
     let trait_ref = Rc::new(ty::TraitRef {
         def_id: fn_trait_def_id,
         substs: tcx.mk_substs(trait_substs),
     });
-
-    ty::Binder((trait_ref, closure_sig.0.output.unwrap()))
+    ty::Binder((trait_ref, sig.0.output.unwrap()))
 }
 
 impl<'tcx,O:Repr<'tcx>> Repr<'tcx> for super::Obligation<'tcx, O> {

src/librustc/middle/ty.rs

@@ -452,7 +452,10 @@ pub struct MethodParam<'tcx> {
     // never contains bound regions; those regions should have been
     // instantiated with fresh variables at this point.
     pub trait_ref: Rc<ty::TraitRef<'tcx>>,
-    // index of uint in the list of methods for the trait
+
+    // index of uint in the list of trait items. Note that this is NOT
+    // the index into the vtable, because the list of trait items
+    // includes associated types.
     pub method_num: uint,
 
     /// The impl for the trait from which the method comes. This
@@ -471,14 +474,14 @@ pub struct MethodObject<'tcx> {
     // the actual base trait id of the object
     pub object_trait_id: ast::DefId,
 
-    // index of the method to be invoked amongst the trait's methods
+    // index of the method to be invoked amongst the trait's items
     pub method_num: uint,
 
     // index into the actual runtime vtable.
     // the vtable is formed by concatenating together the method lists of
-    // the base object trait and all supertraits;  this is the index into
+    // the base object trait and all supertraits; this is the index into
     // that vtable
-    pub real_index: uint,
+    pub vtable_index: uint,
 }
 
 #[derive(Clone)]

src/librustc/middle/ty_fold.rs

@@ -319,7 +319,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::MethodOrigin<'tcx> {
                     trait_ref: object.trait_ref.fold_with(folder),
                     object_trait_id: object.object_trait_id,
                     method_num: object.method_num,
-                    real_index: object.real_index
+                    vtable_index: object.vtable_index,
                 })
             }
         }

src/librustc/util/ppaux.rs

@@ -1061,7 +1061,7 @@ impl<'tcx> Repr<'tcx> for ty::MethodObject<'tcx> {
         format!("MethodObject({},{},{})",
                 self.trait_ref.repr(tcx),
                 self.method_num,
-                self.real_index)
+                self.vtable_index)
     }
 }
 

src/librustc_trans/trans/meth.rs

@@ -13,7 +13,7 @@ use back::abi;
 use back::link;
 use llvm::{self, ValueRef, get_param};
 use metadata::csearch;
-use middle::subst::{Subst, Substs};
+use middle::subst::Substs;
 use middle::subst::VecPerParamSpace;
 use middle::subst;
 use middle::traits;
@@ -29,6 +29,7 @@ use trans::expr::{SaveIn, Ignore};
 use trans::expr;
 use trans::glue;
 use trans::machine;
+use trans::monomorphize;
 use trans::type_::Type;
 use trans::type_of::*;
 use middle::ty::{self, Ty};
@@ -162,7 +163,7 @@ pub fn trans_method_callee<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
             };
             trans_trait_callee(bcx,
                                monomorphize_type(bcx, method_ty),
-                               mt.real_index,
+                               mt.vtable_index,
                                self_expr,
                                arg_cleanup_scope)
         }
@@ -439,7 +440,7 @@ fn combine_impl_and_methods_tps<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 /// extract the self data and vtable out of the pair.
 fn trans_trait_callee<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                                   method_ty: Ty<'tcx>,
-                                  n_method: uint,
+                                  vtable_index: uint,
                                   self_expr: &ast::Expr,
                                   arg_cleanup_scope: cleanup::ScopeId)
                                   -> Callee<'blk, 'tcx> {
@@ -469,28 +470,28 @@ fn trans_trait_callee<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
         self_datum.val
     };
 
-    trans_trait_callee_from_llval(bcx, method_ty, n_method, llval)
+    trans_trait_callee_from_llval(bcx, method_ty, vtable_index, llval)
 }
 
 /// Same as `trans_trait_callee()` above, except that it is given a by-ref pointer to the object
 /// pair.
 pub fn trans_trait_callee_from_llval<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                                                  callee_ty: Ty<'tcx>,
-                                                 n_method: uint,
+                                                 vtable_index: uint,
                                                  llpair: ValueRef)
                                                  -> Callee<'blk, 'tcx> {
     let _icx = push_ctxt("meth::trans_trait_callee");
     let ccx = bcx.ccx();
 
     // Load the data pointer from the object.
-    debug!("(translating trait callee) loading second index from pair");
+    debug!("trans_trait_callee_from_llval(callee_ty={}, vtable_index={}, llpair={})",
+           callee_ty.repr(ccx.tcx()),
+           vtable_index,
+           bcx.val_to_string(llpair));
     let llboxptr = GEPi(bcx, llpair, &[0u, abi::FAT_PTR_ADDR]);
     let llbox = Load(bcx, llboxptr);
     let llself = PointerCast(bcx, llbox, Type::i8p(ccx));
 
-    // Load the function from the vtable and cast it to the expected type.
-    debug!("(translating trait callee) loading method");
-
     // Replace the self type (&Self or Box<Self>) with an opaque pointer.
     let llcallee_ty = match callee_ty.sty {
         ty::ty_bare_fn(_, ref f) if f.abi == Rust || f.abi == RustCall => {
@@ -500,10 +501,7 @@ pub fn trans_trait_callee_from_llval<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                     output: f.sig.0.output,
                     variadic: f.sig.0.variadic,
                 });
-            type_of_rust_fn(ccx,
-                            Some(Type::i8p(ccx)),
-                            &fake_sig,
-                            f.abi)
+            type_of_rust_fn(ccx, Some(Type::i8p(ccx)), &fake_sig, f.abi)
         }
         _ => {
             ccx.sess().bug("meth::trans_trait_callee given non-bare-rust-fn");
@@ -514,7 +512,7 @@ pub fn trans_trait_callee_from_llval<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                                     GEPi(bcx, llpair,
                                          &[0u, abi::FAT_PTR_EXTRA]),
                                     Type::vtable(ccx).ptr_to().ptr_to()));
-    let mptr = Load(bcx, GEPi(bcx, llvtable, &[0u, n_method + VTABLE_OFFSET]));
+    let mptr = Load(bcx, GEPi(bcx, llvtable, &[0u, vtable_index + VTABLE_OFFSET]));
     let mptr = PointerCast(bcx, mptr, llcallee_ty.ptr_to());
 
     return Callee {
@@ -558,7 +556,7 @@ pub fn trans_object_shim<'a, 'tcx>(
     let _icx = push_ctxt("trans_object_shim");
     let tcx = ccx.tcx();
 
-    debug!("trans_object_shim(object_ty={}, trait_id={}, n_method={})",
+    debug!("trans_object_shim(object_ty={}, trait_id={}, method_offset_in_trait={})",
            object_ty.repr(tcx),
            trait_id.repr(tcx),
            method_offset_in_trait);
@@ -587,7 +585,7 @@ pub fn trans_object_shim<'a, 'tcx>(
             tcx.sess.bug("can't create a method shim for an associated type")
         }
     };
-    let fty = method_ty.fty.subst(tcx, &object_substs);
+    let fty = monomorphize::apply_param_substs(tcx, &object_substs, &method_ty.fty);
     let fty = tcx.mk_bare_fn(fty);
     debug!("trans_object_shim: fty={}", fty.repr(tcx));
 

src/librustc_trans/trans/type_of.rs

@@ -103,6 +103,10 @@ pub fn type_of_rust_fn<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                                  abi: abi::Abi)
                                  -> Type
 {
+    debug!("type_of_rust_fn(sig={},abi={:?})",
+           sig.repr(cx.tcx()),
+           abi);
+
     let sig = ty::erase_late_bound_regions(cx.tcx(), sig);
     assert!(!sig.variadic); // rust fns are never variadic
 

src/librustc_typeck/check/method/confirm.rs

@@ -206,7 +206,7 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
                 (impl_polytype.substs, MethodStatic(pick.method_ty.def_id))
             }
 
-            probe::ObjectPick(trait_def_id, method_num, real_index) => {
+            probe::ObjectPick(trait_def_id, method_num, vtable_index) => {
                 self.extract_trait_ref(self_ty, |this, object_ty, data| {
                     // The object data has no entry for the Self
                     // Type. For the purposes of this method call, we
@@ -233,7 +233,7 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
                         trait_ref: upcast_trait_ref,
                         object_trait_id: trait_def_id,
                         method_num: method_num,
-                        real_index: real_index,
+                        vtable_index: vtable_index,
                     });
                     (substs, origin)
                 })

src/librustc_typeck/check/method/probe.rs

@@ -59,7 +59,7 @@ struct Candidate<'tcx> {
 
 enum CandidateKind<'tcx> {
     InherentImplCandidate(/* Impl */ ast::DefId, subst::Substs<'tcx>),
-    ObjectCandidate(/* Trait */ ast::DefId, /* method_num */ uint, /* real_index */ uint),
+    ObjectCandidate(/* Trait */ ast::DefId, /* method_num */ uint, /* vtable index */ uint),
     ExtensionImplCandidate(/* Impl */ ast::DefId, Rc<ty::TraitRef<'tcx>>,
                            subst::Substs<'tcx>, MethodIndex),
     ClosureCandidate(/* Trait */ ast::DefId, MethodIndex),
@@ -318,7 +318,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
         // itself. Hence, a `&self` method will wind up with an
         // argument type like `&Trait`.
         let trait_ref = data.principal_trait_ref_with_self_ty(self.tcx(), self_ty);
-        self.elaborate_bounds(&[trait_ref.clone()], false, |this, new_trait_ref, m, method_num| {
+        self.elaborate_bounds(&[trait_ref.clone()], |this, new_trait_ref, m, method_num| {
             let new_trait_ref = this.erase_late_bound_regions(&new_trait_ref);
 
             let vtable_index =
@@ -365,7 +365,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
             })
             .collect();
 
-        self.elaborate_bounds(bounds.as_slice(), true, |this, poly_trait_ref, m, method_num| {
+        self.elaborate_bounds(bounds.as_slice(), |this, poly_trait_ref, m, method_num| {
             let trait_ref =
                 this.erase_late_bound_regions(&poly_trait_ref);
 
@@ -405,7 +405,6 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
     fn elaborate_bounds<F>(
         &mut self,
         bounds: &[ty::PolyTraitRef<'tcx>],
-        num_includes_types: bool,
         mut mk_cand: F,
     ) where
         F: for<'b> FnMut(
@@ -427,8 +426,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
 
             let (pos, method) = match trait_method(tcx,
                                                    bound_trait_ref.def_id(),
-                                                   self.method_name,
-                                                   num_includes_types) {
+                                                   self.method_name) {
                 Some(v) => v,
                 None => { continue; }
             };
@@ -1139,19 +1137,13 @@ fn impl_method<'tcx>(tcx: &ty::ctxt<'tcx>,
 /// index (or `None`, if no such method).
 fn trait_method<'tcx>(tcx: &ty::ctxt<'tcx>,
                       trait_def_id: ast::DefId,
-                      method_name: ast::Name,
-                      num_includes_types: bool)
+                      method_name: ast::Name)
                       -> Option<(uint, Rc<ty::Method<'tcx>>)>
 {
     let trait_items = ty::trait_items(tcx, trait_def_id);
     debug!("trait_method; items: {:?}", trait_items);
     trait_items
         .iter()
-        .filter(|item|
-            num_includes_types || match *item {
-                &ty::MethodTraitItem(_) => true,
-                &ty::TypeTraitItem(_) => false
-            })
         .enumerate()
         .find(|&(_, ref item)| item.name() == method_name)
         .and_then(|(idx, item)| item.as_opt_method().map(|m| (idx, m)))