From a30ea013f5b88f136438931328ee859ee4e5eff3 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Wed, 16 Jan 2013 18:45:05 -0800 Subject: [PATCH] Handle supertrait calls in default methods Add a new method_super origin for supertrait methods. Also make coherence create a table that maps pairs of trait IDs and self types to impl IDs, so that it's possible to check a supertrait method knowing only its index in its trait's methods (without knowing all supertraits for a given trait). r=nmatsakis and graydon -- with hope, we'll revamp all of this code as per #4678, but for now this fixes the bug. Closes #3979 --- src/librustc/middle/astencode.rs | 9 +- src/librustc/middle/privacy.rs | 6 +- src/librustc/middle/trans/meth.rs | 27 +++++- src/librustc/middle/trans/monomorphize.rs | 2 +- src/librustc/middle/trans/type_use.rs | 3 +- src/librustc/middle/ty.rs | 91 ++++++++++++------ src/librustc/middle/typeck/check/method.rs | 102 ++++++++++++++------- src/librustc/middle/typeck/coherence.rs | 23 +++++ src/librustc/middle/typeck/mod.rs | 9 +- src/test/auxiliary/issue_3979_traits.rs | 26 ++++++ src/test/run-pass/issue-3979-2.rs | 26 ++++++ src/test/run-pass/issue-3979-generics.rs | 41 +++++++++ src/test/run-pass/issue-3979-xcrate.rs | 33 +++++++ src/test/run-pass/issue-3979.rs | 26 +++++- 14 files changed, 352 insertions(+), 72 deletions(-) create mode 100644 src/test/auxiliary/issue_3979_traits.rs create mode 100644 src/test/run-pass/issue-3979-2.rs create mode 100644 src/test/run-pass/issue-3979-generics.rs create mode 100644 src/test/run-pass/issue-3979-xcrate.rs diff --git a/src/librustc/middle/astencode.rs b/src/librustc/middle/astencode.rs index f39bd552c3b..9e0b07dd9ee 100644 --- a/src/librustc/middle/astencode.rs +++ b/src/librustc/middle/astencode.rs @@ -564,7 +564,7 @@ impl method_origin: tr { fn tr(xcx: extended_decode_ctxt) -> method_origin { match self { typeck::method_static(did) => { - typeck::method_static(did.tr(xcx)) + typeck::method_static(did.tr(xcx)) } typeck::method_param(ref mp) => { typeck::method_param( @@ -575,10 +575,13 @@ impl method_origin: tr { ) } typeck::method_trait(did, m, vstore) => { - typeck::method_trait(did.tr(xcx), m, vstore) + typeck::method_trait(did.tr(xcx), m, vstore) } typeck::method_self(did, m) => { - typeck::method_self(did.tr(xcx), m) + typeck::method_self(did.tr(xcx), m) + } + typeck::method_super(trait_did, m) => { + typeck::method_super(trait_did.tr(xcx), m) } } } diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index 401510328da..fcc75e8867b 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -16,7 +16,8 @@ use core::prelude::*; use middle::ty::{ty_struct, ty_enum}; use middle::ty; -use middle::typeck::{method_map, method_origin, method_param, method_self}; +use middle::typeck::{method_map, method_origin, method_param, method_self, + method_super}; use middle::typeck::{method_static, method_trait}; use core::dvec::DVec; @@ -138,7 +139,8 @@ fn check_crate(tcx: ty::ctxt, method_map: &method_map, crate: @ast::crate) { _ }) | method_trait(trait_id, method_num, _) | - method_self(trait_id, method_num) => { + method_self(trait_id, method_num) | + method_super(trait_id, method_num) => { if trait_id.crate == local_crate { match tcx.items.find(trait_id.node) { Some(node_item(item, _)) => { diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index a90c25abf3d..0ee3e3e451a 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -193,6 +193,29 @@ fn trans_method_callee(bcx: block, callee_id: ast::node_id, method_name); origin = typeck::method_static(method_id); } + typeck::method_super(trait_id, method_index) => { + // is the self type for this method call + let self_ty = node_id_type(bcx, self.id); + let tcx = bcx.tcx(); + // is the ID of the implementation of + // trait for type + let impl_id = ty::get_impl_id(tcx, trait_id, self_ty); + // Get the supertrait's methods + let supertrait_methods = ty::trait_methods(tcx, trait_id); + // Make sure to fail with a readable error message if + // there's some internal error here + if !(method_index < supertrait_methods.len()) { + tcx.sess.bug(~"trans_method_callee: supertrait method \ + index is out of bounds"); + } + // Get the method name using the method index in the origin + let method_name = supertrait_methods[method_index].ident; + // Now that we know the impl ID, we can look up the method + // ID from its name + origin = typeck::method_static(method_with_name(bcx.ccx(), + impl_id, + method_name)); + } typeck::method_static(*) | typeck::method_param(*) | typeck::method_trait(*) => {} } @@ -236,8 +259,8 @@ fn trans_method_callee(bcx: block, callee_id: ast::node_id, vstore, mentry.explicit_self) } - typeck::method_self(*) => { - fail ~"method_self should have been handled above" + typeck::method_self(*) | typeck::method_super(*) => { + fail ~"method_self or method_super should have been handled above" } } } diff --git a/src/librustc/middle/trans/monomorphize.rs b/src/librustc/middle/trans/monomorphize.rs index 5aad9026814..f8c175389a7 100644 --- a/src/librustc/middle/trans/monomorphize.rs +++ b/src/librustc/middle/trans/monomorphize.rs @@ -312,7 +312,7 @@ fn make_mono_id(ccx: @crate_ctxt, item: ast::def_id, substs: ~[ty::t], let precise_param_ids = match vtables { Some(vts) => { let bounds = ty::lookup_item_type(ccx.tcx, item).bounds; - let mut i = 0u; + let mut i = 0; vec::map2(*bounds, substs, |bounds, subst| { let mut v = ~[]; for bounds.each |bound| { diff --git a/src/librustc/middle/trans/type_use.rs b/src/librustc/middle/trans/type_use.rs index ba1901f215c..39f3aecfcd4 100644 --- a/src/librustc/middle/trans/type_use.rs +++ b/src/librustc/middle/trans/type_use.rs @@ -239,7 +239,8 @@ fn mark_for_method_call(cx: ctx, e_id: node_id, callee_id: node_id) { }) => { cx.uses[param] |= use_tydesc; } - typeck::method_trait(*) | typeck::method_self(*) => (), + typeck::method_trait(*) | typeck::method_self(*) + | typeck::method_super(*) => (), } } } diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 26f4310bf1c..4458777ff1f 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -243,6 +243,7 @@ export iter_bound_traits_and_supertraits; export count_traits_and_supertraits; export IntVarValue, IntType, UintType; export creader_cache_key; +export get_impl_id; // Data types @@ -486,6 +487,9 @@ type ctxt = // Records the value mode (read, copy, or move) for every value. value_modes: HashMap, + + // Maps a trait onto a mapping from self-ty to impl + trait_impls: HashMap> }; enum tbox_flag { @@ -1051,7 +1055,9 @@ fn mk_ctxt(s: session::Session, supertraits: HashMap(), destructor_for_type: HashMap(), destructors: HashMap(), - value_modes: HashMap()} + value_modes: HashMap(), + trait_impls: HashMap() + } } @@ -3130,7 +3136,8 @@ fn method_call_bounds(tcx: ctxt, method_map: typeck::method_map, trait_id: trt_id, method_num: n_mth, _}) | typeck::method_trait(trt_id, n_mth, _) | - typeck::method_self(trt_id, n_mth) => { + typeck::method_self(trt_id, n_mth) | + typeck::method_super(trt_id, n_mth) => { // ...trait methods bounds, in contrast, include only the // method bounds, so we must preprend the tps from the // trait itself. This ought to be harmonized. @@ -4372,9 +4379,14 @@ pure fn determine_inherited_purity(parent_purity: ast::purity, // Iterate over a type parameter's bounded traits and any supertraits // of those traits, ignoring kinds. +// Here, the supertraits are the transitive closure of the supertrait +// relation on the supertraits from each bounded trait's constraint +// list. fn iter_bound_traits_and_supertraits(tcx: ctxt, bounds: param_bounds, f: &fn(t) -> bool) { + let mut fin = false; + for bounds.each |bound| { let bound_trait_ty = match *bound { @@ -4386,34 +4398,45 @@ fn iter_bound_traits_and_supertraits(tcx: ctxt, } }; - let mut worklist = ~[]; - - let init_trait_ty = bound_trait_ty; - - worklist.push(init_trait_ty); - + let mut supertrait_map = HashMap(); + let mut seen_def_ids = ~[]; let mut i = 0; - while i < worklist.len() { - let init_trait_ty = worklist[i]; - i += 1; + let trait_ty_id = ty_to_def_id(bound_trait_ty).expect( + ~"iter_trait_ty_supertraits got a non-trait type"); + let mut trait_ty = bound_trait_ty; - let init_trait_id = match ty_to_def_id(init_trait_ty) { - Some(id) => id, - None => tcx.sess.bug( - ~"trait type should have def_id") - }; + debug!("iter_bound_traits_and_supertraits: trait_ty = %s", + ty_to_str(tcx, trait_ty)); - // Add supertraits to worklist - let supertraits = trait_supertraits(tcx, - init_trait_id); - for supertraits.each |supertrait| { - worklist.push(supertrait.tpt.ty); + // Add the given trait ty to the hash map + supertrait_map.insert(trait_ty_id, trait_ty); + seen_def_ids.push(trait_ty_id); + + if f(trait_ty) { + // Add all the supertraits to the hash map, + // executing on each of them + while i < supertrait_map.size() && !fin { + let init_trait_id = seen_def_ids[i]; + i += 1; + // Add supertraits to supertrait_map + let supertraits = trait_supertraits(tcx, init_trait_id); + for supertraits.each |supertrait| { + let super_t = supertrait.tpt.ty; + let d_id = ty_to_def_id(super_t).expect("supertrait \ + should be a trait ty"); + if !supertrait_map.contains_key(d_id) { + supertrait_map.insert(d_id, super_t); + trait_ty = super_t; + seen_def_ids.push(d_id); + } + debug!("A super_t = %s", ty_to_str(tcx, trait_ty)); + if !f(trait_ty) { + fin = true; + } + } } - - if !f(init_trait_ty) { - return; - } - } + }; + fin = false; } } @@ -4428,6 +4451,22 @@ fn count_traits_and_supertraits(tcx: ctxt, return total; } +// Given a trait and a type, returns the impl of that type +fn get_impl_id(tcx: ctxt, trait_id: def_id, self_ty: t) -> def_id { + match tcx.trait_impls.find(trait_id) { + Some(ty_to_impl) => match ty_to_impl.find(self_ty) { + Some(the_impl) => the_impl.did, + None => // try autoderef! + match deref(tcx, self_ty, false) { + Some(some_ty) => get_impl_id(tcx, trait_id, some_ty.ty), + None => tcx.sess.bug(~"get_impl_id: no impl of trait for \ + this type") + } + }, + None => tcx.sess.bug(~"get_impl_id: trait isn't in trait_impls") + } +} + impl mt : cmp::Eq { pure fn eq(&self, other: &mt) -> bool { (*self).ty == (*other).ty && (*self).mutbl == (*other).mutbl diff --git a/src/librustc/middle/typeck/check/method.rs b/src/librustc/middle/typeck/check/method.rs index 4f3ca9950cc..5c012f474ab 100644 --- a/src/librustc/middle/typeck/check/method.rs +++ b/src/librustc/middle/typeck/check/method.rs @@ -93,7 +93,7 @@ use middle::typeck::check; use middle::typeck::coherence::get_base_type_def_id; use middle::typeck::infer; use middle::typeck::{method_map_entry, method_origin, method_param}; -use middle::typeck::{method_self, method_static, method_trait}; +use middle::typeck::{method_self, method_static, method_trait, method_super}; use util::common::indenter; use util::ppaux::expr_repr; @@ -543,33 +543,67 @@ pub impl LookupContext { did: def_id, substs: &ty::substs) { let tcx = self.tcx(); - let methods = ty::trait_methods(tcx, did); // XXX: Inherited methods. - let index; + // First, try self methods + let mut method = None; + let methods = ty::trait_methods(tcx, did); + let mut index = None; + let mut trait_did = None; match vec::position(*methods, |m| m.ident == self.m_name) { - Some(i) => index = i, - None => return + Some(i) => { + index = Some(i); + trait_did = Some(did); + method = Some((methods[i].self_ty, methods[i].tps.len())); + } + None => () } - let method = &methods[index]; + // No method found yet? Check each supertrait + if method.is_none() { + for ty::trait_supertraits(tcx, did).each() |trait_ref| { + let supertrait_methods = + ty::trait_methods(tcx, trait_ref.def_id); + match vec::position(*supertrait_methods, + |m| m.ident == self.m_name) { + Some(i) => { + index = Some(i); + trait_did = Some(trait_ref.def_id); + method = Some((supertrait_methods[i].self_ty, + supertrait_methods[i].tps.len())); + break; + } + None => () + } + } + } + match (method, index, trait_did) { + (Some((method_self_ty, method_num_tps)), + Some(index), Some(trait_did)) => { - let rcvr_substs = substs { - self_ty: Some(self_ty), - ../*bad*/copy *substs - }; - let (rcvr_ty, rcvr_substs) = - self.create_rcvr_ty_and_substs_for_method( - method.self_ty, - self_ty, - move rcvr_substs, - TransformTypeNormally); - - self.inherent_candidates.push(Candidate { - rcvr_ty: rcvr_ty, - rcvr_substs: move rcvr_substs, - explicit_self: method.self_ty, - num_method_tps: method.tps.len(), - self_mode: get_mode_from_self_type(method.self_ty), - origin: method_self(did, index) - }); + // We've found a method -- return it + let rcvr_substs = substs { self_ty: Some(self_ty), + ..copy *substs }; + let (rcvr_ty, rcvr_substs) = + self.create_rcvr_ty_and_substs_for_method( + method_self_ty, + self_ty, + move rcvr_substs, + TransformTypeNormally); + let origin = if trait_did == did { + method_self(trait_did, index) + } + else { + method_super(trait_did, index) + }; + self.inherent_candidates.push(Candidate { + rcvr_ty: rcvr_ty, + rcvr_substs: move rcvr_substs, + explicit_self: method_self_ty, + num_method_tps: method_num_tps, + self_mode: get_mode_from_self_type(method_self_ty), + origin: origin + }); + } + _ => return + } } fn push_inherent_impl_candidates_for_type(did: def_id) { @@ -1111,7 +1145,8 @@ pub impl LookupContext { * vtable and hence cannot be monomorphized. */ match candidate.origin { - method_static(*) | method_param(*) | method_self(*) => { + method_static(*) | method_param(*) | + method_self(*) | method_super(*) => { return; // not a call to a trait instance } method_trait(*) => {} @@ -1135,7 +1170,8 @@ pub impl LookupContext { // No code can call the finalize method explicitly. let bad; match candidate.origin { - method_static(method_id) | method_self(method_id, _) => { + method_static(method_id) | method_self(method_id, _) + | method_super(method_id, _) => { bad = self.tcx().destructors.contains_key(method_id); } method_param(method_param { trait_id: trait_id, _ }) | @@ -1165,7 +1201,8 @@ pub impl LookupContext { method_param(ref mp) => { type_of_trait_method(self.tcx(), mp.trait_id, mp.method_num) } - method_trait(did, idx, _) | method_self(did, idx) => { + method_trait(did, idx, _) | method_self(did, idx) | + method_super(did, idx) => { type_of_trait_method(self.tcx(), did, idx) } }; @@ -1186,7 +1223,8 @@ pub impl LookupContext { method_param(ref mp) => { self.report_param_candidate(idx, (*mp).trait_id) } - method_trait(trait_did, _, _) | method_self(trait_did, _) => { + method_trait(trait_did, _, _) | method_self(trait_did, _) + | method_super(trait_did, _) => { self.report_param_candidate(idx, trait_did) } } @@ -1194,9 +1232,9 @@ pub impl LookupContext { fn report_static_candidate(&self, idx: uint, did: def_id) { let span = if did.crate == ast::local_crate { - match self.tcx().items.get(did.node) { - ast_map::node_method(m, _, _) => m.span, - _ => fail ~"report_static_candidate: bad item" + match self.tcx().items.find(did.node) { + Some(ast_map::node_method(m, _, _)) => m.span, + _ => fail fmt!("report_static_candidate: bad item %?", did) } } else { self.expr.span diff --git a/src/librustc/middle/typeck/coherence.rs b/src/librustc/middle/typeck/coherence.rs index 153d09e82e6..334b2f46c3b 100644 --- a/src/librustc/middle/typeck/coherence.rs +++ b/src/librustc/middle/typeck/coherence.rs @@ -429,6 +429,11 @@ pub impl CoherenceChecker { let implementation_a = a; let polytype_a = self.get_self_type_for_implementation(implementation_a); + + // "We have an impl of trait for type , + // and that impl is " + self.add_impl_for_trait(trait_def_id, polytype_a.ty, + implementation_a); do self.iter_impls_of_trait(trait_def_id) |b| { let implementation_b = b; @@ -451,6 +456,24 @@ pub impl CoherenceChecker { } } + // Adds an impl of trait trait_t for self type self_t; that impl + // is the_impl + fn add_impl_for_trait(trait_t: def_id, self_t: t, the_impl: @Impl) { + debug!("Adding impl %? of %? for %s", + the_impl.did, trait_t, + ty_to_str(self.crate_context.tcx, self_t)); + match self.crate_context.tcx.trait_impls.find(trait_t) { + None => { + let m = HashMap(); + m.insert(self_t, the_impl); + self.crate_context.tcx.trait_impls.insert(trait_t, m); + } + Some(m) => { + m.insert(self_t, the_impl); + } + } + } + fn iter_impls_of_trait(trait_def_id: def_id, f: &fn(@Impl)) { diff --git a/src/librustc/middle/typeck/mod.rs b/src/librustc/middle/typeck/mod.rs index c264d8e0806..bc84b957865 100644 --- a/src/librustc/middle/typeck/mod.rs +++ b/src/librustc/middle/typeck/mod.rs @@ -91,6 +91,12 @@ pub mod coherence; #[auto_encode] #[auto_decode] pub enum method_origin { + // supertrait method invoked on "self" inside a default method + // first field is supertrait ID; + // second field is method index (relative to the *supertrait* + // method list) + method_super(ast::def_id, uint), + // fully statically resolved method method_static(ast::def_id), @@ -101,7 +107,8 @@ pub enum method_origin { method_trait(ast::def_id, uint, ty::vstore), // method invoked on "self" inside a default method - method_self(ast::def_id, uint), + method_self(ast::def_id, uint) + } // details for a method invoked with a receiver whose type is a type parameter diff --git a/src/test/auxiliary/issue_3979_traits.rs b/src/test/auxiliary/issue_3979_traits.rs new file mode 100644 index 00000000000..bf3167f6051 --- /dev/null +++ b/src/test/auxiliary/issue_3979_traits.rs @@ -0,0 +1,26 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[link(name = "issue_3979_traits", + vers = "0.1")]; + +#[crate_type = "lib"]; + +trait Positioned { + fn SetX(&self, int); + fn X(&self) -> int; +} + +#[allow(default_methods)] +trait Movable: Positioned { + fn translate(&self, dx: int) { + self.SetX(self.X() + dx); + } +} diff --git a/src/test/run-pass/issue-3979-2.rs b/src/test/run-pass/issue-3979-2.rs new file mode 100644 index 00000000000..7cc2bb49ef7 --- /dev/null +++ b/src/test/run-pass/issue-3979-2.rs @@ -0,0 +1,26 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +trait A { + fn a_method(); +} + +trait B: A { + fn b_method(); +} + +trait C: B { + fn c_method() { + self.a_method(); + } +} + +fn main() {} + diff --git a/src/test/run-pass/issue-3979-generics.rs b/src/test/run-pass/issue-3979-generics.rs new file mode 100644 index 00000000000..7261b74a7fa --- /dev/null +++ b/src/test/run-pass/issue-3979-generics.rs @@ -0,0 +1,41 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// xfail-test +trait Positioned { + fn SetX(&self, S); + fn X(&self) -> S; +} + +#[allow(default_methods)] +trait Movable: Positioned { + fn translate(&self, dx: T) { + self.SetX(self.X() + dx); + } +} + +struct Point { mut x: int, mut y: int } + +impl Point: Positioned { + fn SetX(&self, x: int) { + self.x = x; + } + fn X(&self) -> int { + self.x + } +} + +impl Point: Movable; + +fn main() { + let p = Point{ x: 1, y: 2}; + p.translate(3); + assert p.X() == 4; +} diff --git a/src/test/run-pass/issue-3979-xcrate.rs b/src/test/run-pass/issue-3979-xcrate.rs new file mode 100644 index 00000000000..bd48950534a --- /dev/null +++ b/src/test/run-pass/issue-3979-xcrate.rs @@ -0,0 +1,33 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// xfail-test // tjc: ??? +// aux-build:issue_3979_traits.rs +extern mod issue_3979_traits; +use issue_3979_traits::*; + +struct Point { mut x: int, mut y: int } + +impl Point: Positioned { + fn SetX(&self, x: int) { + self.x = x; + } + fn X(&self) -> int { + self.x + } +} + +impl Point: Movable; + +fn main() { + let p = Point{ x: 1, y: 2}; + p.translate(3); + assert p.X() == 4; +} diff --git a/src/test/run-pass/issue-3979.rs b/src/test/run-pass/issue-3979.rs index 52c7f26a195..2c680d178b2 100644 --- a/src/test/run-pass/issue-3979.rs +++ b/src/test/run-pass/issue-3979.rs @@ -8,15 +8,33 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// xfail-test trait Positioned { - fn SetX(int); + fn SetX(&self, int); + fn X(&self) -> int; } +#[allow(default_methods)] trait Movable: Positioned { - fn translate(dx: int) { + fn translate(&self, dx: int) { self.SetX(self.X() + dx); } } -fn main() {} +struct Point { mut x: int, mut y: int } + +impl Point: Positioned { + fn SetX(&self, x: int) { + self.x = x; + } + fn X(&self) -> int { + self.x + } +} + +impl Point: Movable; + +fn main() { + let p = Point{ x: 1, y: 2}; + p.translate(3); + assert p.X() == 4; +}