From 07a81ad12e5cb7f84138af6624f30c5dbb75512f Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Tue, 26 Jun 2012 16:25:52 -0700 Subject: [PATCH] Refactor how impl self types are stored In order to avoid a confusing use of the tcache, I added an extra node ID field to trait refs. Now trait refs have a "ref ID" (the one that resolve3 resolves) and an "impl ID" (the one that you look up in the tcache to get the self type). Closes #2434 --- src/libsyntax/ast.rs | 9 +++++++-- src/libsyntax/ast_map.rs | 12 +++++++++--- src/libsyntax/fold.rs | 3 ++- src/libsyntax/parse/parser.rs | 4 ++-- src/rustc/metadata/encoder.rs | 4 +++- src/rustc/middle/resolve.rs | 11 +++++++---- src/rustc/middle/resolve3.rs | 4 ++-- src/rustc/middle/ty.rs | 2 +- src/rustc/middle/typeck/coherence.rs | 4 ++-- src/rustc/middle/typeck/collect.rs | 19 ++++++------------- 10 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index f79f6cbcae2..095038f02e3 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -675,10 +675,15 @@ type attribute_ = {style: attr_style, value: meta_item, is_sugared_doc: bool}; /* trait_refs appear in both impls and in classes that implement traits. - resolve maps each trait_ref's id to its defining trait. + resolve maps each trait_ref's ref_id to its defining trait; that's all + that the ref_id is for. The impl_id maps to the "self type" of this impl. + If this impl is an item_impl, the impl_id is redundant (it could be the + same as the impl's node id). If this impl is actually an impl_class, then + conceptually, the impl_id stands in for the pair of (this class, this + trait) */ #[auto_serialize] -type trait_ref = {path: @path, id: node_id}; +type trait_ref = {path: @path, ref_id: node_id, impl_id: node_id}; #[auto_serialize] enum visibility { public, private } diff --git a/src/libsyntax/ast_map.rs b/src/libsyntax/ast_map.rs index 129a03f71aa..a5ae45d54ee 100644 --- a/src/libsyntax/ast_map.rs +++ b/src/libsyntax/ast_map.rs @@ -188,7 +188,7 @@ fn map_item(i: @item, cx: ctx, v: vt) { let item_path = @/* FIXME (#2543) */ copy cx.path; cx.map.insert(i.id, node_item(i, item_path)); alt i.node { - item_impl(_, _, _, ms) { + item_impl(_, opt_ir, _, ms) { let impl_did = ast_util::local_def(i.id); for ms.each |m| { map_method(impl_did, extend(cx, i.ident), m, @@ -218,8 +218,14 @@ fn map_item(i: @item, cx: ctx, v: vt) { let (_, ms) = ast_util::split_class_items(items); // Map trait refs to their parent classes. This is // so we can find the self_ty - do vec::iter(traits) |p| { cx.map.insert(p.id, - node_item(i, item_path)); }; + do vec::iter(traits) |p| { cx.map.insert(p.ref_id, + node_item(i, item_path)); + // This is so we can look up the right things when + // encoding/decoding + cx.map.insert(p.impl_id, + node_item(i, item_path)); + + }; let d_id = ast_util::local_def(i.id); let p = extend(cx, i.ident); // only need to handle methods diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 0e16d4bdca8..f5edb96f5e1 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -287,7 +287,8 @@ fn noop_fold_item_underscore(i: item_, fld: ast_fold) -> item_ { } fn fold_trait_ref(&&p: @trait_ref, fld: ast_fold) -> @trait_ref { - @{path: fld.fold_path(p.path), id: fld.new_id(p.id)} + @{path: fld.fold_path(p.path), ref_id: fld.new_id(p.ref_id), + impl_id: fld.new_id(p.impl_id)} } fn noop_fold_method(&&m: @method, fld: ast_fold) -> @method { diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 5a1eb7636ba..7448f246698 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2189,7 +2189,7 @@ class parser { if option::is_none(ident) { ident = some(vec::last(path.idents)); } - some(@{path: path, id: self.get_id()}) + some(@{path: path, ref_id: self.get_id(), impl_id: self.get_id()}) } else { none }; let ident = alt ident { some(name) { name } @@ -2223,7 +2223,7 @@ class parser { fn parse_trait_ref() -> @trait_ref { @{path: self.parse_path_with_tps(false), - id: self.get_id()} + ref_id: self.get_id(), impl_id: self.get_id()} } fn parse_trait_ref_list() -> ~[@trait_ref] { diff --git a/src/rustc/metadata/encoder.rs b/src/rustc/metadata/encoder.rs index 74fd9e1c17e..12106e5bc45 100644 --- a/src/rustc/metadata/encoder.rs +++ b/src/rustc/metadata/encoder.rs @@ -227,7 +227,7 @@ fn encode_module_item_paths(ebml_w: ebml::writer, ecx: @encode_ctxt, fn encode_trait_ref(ebml_w: ebml::writer, ecx: @encode_ctxt, t: @trait_ref) { ebml_w.start_tag(tag_impl_trait); - encode_type(ecx, ebml_w, node_id_to_type(ecx.tcx, t.id)); + encode_type(ecx, ebml_w, node_id_to_type(ecx.tcx, t.ref_id)); ebml_w.end_tag(); } @@ -392,6 +392,7 @@ fn encode_info_for_mod(ecx: @encode_ctxt, ebml_w: ebml::writer, md: _mod, encode_family(ebml_w, 'm'); encode_name(ebml_w, name); #debug("(encoding info for module) encoding info for module ID %d", id); + // the impl map contains ref_ids let impls = ecx.impl_map(id); for impls.each |i| { let (ident, did) = i; @@ -415,6 +416,7 @@ fn encode_info_for_mod(ecx: @encode_ctxt, ebml_w: ebml::writer, md: _mod, } none { // Must be a re-export, then! + // ...or an iface ref ebml_w.wr_str(def_to_str(did)); } }; diff --git a/src/rustc/middle/resolve.rs b/src/rustc/middle/resolve.rs index 8efdfac95ea..918d278a6f8 100644 --- a/src/rustc/middle/resolve.rs +++ b/src/rustc/middle/resolve.rs @@ -408,8 +408,11 @@ fn maybe_insert(e: @env, id: node_id, def: option) { } fn resolve_trait_ref(p: @trait_ref, sc: scopes, e: @env) { - maybe_insert(e, p.id, + maybe_insert(e, p.ref_id, lookup_path_strict(*e, sc, p.path.span, p.path, ns_type)); + maybe_insert(e, p.impl_id, + lookup_path_strict(*e, sc, p.path.span, p.path, ns_type)); + } fn resolve_names(e: @env, c: @ast::crate) { @@ -440,8 +443,8 @@ fn resolve_names(e: @env, c: @ast::crate) { /* At this point, the code knows what traits the trait refs refer to, so it's possible to resolve them. */ - ast::item_impl(_, ifce, _, _) { - ifce.iter(|p| resolve_trait_ref(p, sc, e)) + ast::item_impl(_, t, _, _) { + t.iter(|p| resolve_trait_ref(p, sc, e)); } ast::item_class(_, traits, _, _, _) { for traits.each |p| { @@ -2290,7 +2293,7 @@ fn find_impls_in_item(e: env, i: @ast::item, &impls: ~[@_impl], do vec::iter(ifces) |p| { // The def_id, in this case, identifies the combination of // class and trait - vec::push(impls, @{did: local_def(p.id), + vec::push(impls, @{did: local_def(p.impl_id), ident: i.ident, methods: vec::map(mthds, |m| { @{did: local_def(m.id), diff --git a/src/rustc/middle/resolve3.rs b/src/rustc/middle/resolve3.rs index 138f021982a..933f23ad103 100644 --- a/src/rustc/middle/resolve3.rs +++ b/src/rustc/middle/resolve3.rs @@ -3241,7 +3241,7 @@ class Resolver { #debug("(resolving class) found trait def: %?", def); - self.record_def(interface.id, def); + self.record_def(interface.ref_id, def); // XXX: This is wrong but is needed for tests to // pass. @@ -3349,7 +3349,7 @@ class Resolver { unknown interface"); } some(def) { - self.record_def(interface_reference.id, def); + self.record_def(interface_reference.ref_id, def); } } } diff --git a/src/rustc/middle/ty.rs b/src/rustc/middle/ty.rs index 3bd19805e91..2dd9f414677 100644 --- a/src/rustc/middle/ty.rs +++ b/src/rustc/middle/ty.rs @@ -2549,7 +2549,7 @@ fn impl_trait(cx: ctxt, id: ast::def_id) -> option { #debug("(impl_trait) searching for trait impl %?", id); alt cx.items.find(id.node) { some(ast_map::node_item(@{node: ast::item_impl( - _, some(@{id: id, _}), _, _), _}, _)) { + _, some(@{ref_id: id, _}), _, _), _}, _)) { some(node_id_to_type(cx, id)) } some(ast_map::node_item(@{node: ast::item_class(*), diff --git a/src/rustc/middle/typeck/coherence.rs b/src/rustc/middle/typeck/coherence.rs index ad121772bd7..4773788593a 100644 --- a/src/rustc/middle/typeck/coherence.rs +++ b/src/rustc/middle/typeck/coherence.rs @@ -128,7 +128,7 @@ class CoherenceChecker { } some(associated_trait) { let def = - self.crate_context.tcx.def_map.get(associated_trait.id); + self.crate_context.tcx.def_map.get(associated_trait.ref_id); let def_id = def_id_of_def(def); let implementation_list; @@ -349,7 +349,7 @@ class CoherenceChecker { let def_map = self.crate_context .tcx.def_map; let trait_def = - def_map.get(trait_ref.id); + def_map.get(trait_ref.ref_id); let trait_id = def_id_of_def(trait_def); if trait_id.crate != local_crate { diff --git a/src/rustc/middle/typeck/collect.rs b/src/rustc/middle/typeck/collect.rs index 30c5c04dc04..5d3842e6085 100644 --- a/src/rustc/middle/typeck/collect.rs +++ b/src/rustc/middle/typeck/collect.rs @@ -392,17 +392,9 @@ fn convert(ccx: @crate_ctxt, it: @ast::item) { let cms = convert_methods(ccx, methods, rp, bounds, selfty); for traits.each |ifce| { check_methods_against_trait(ccx, tps, rp, selfty, ifce, cms); - - // FIXME #2434---this is somewhat bogus, but it seems that - // the id of trait_ref is also the id of the impl, and so - // we want to store the "self type" of the impl---in this - // case, the class. The reason I say this is somewhat - // bogus (and should be refactored) is that the tcache - // stores the class type for ifce.id but the node_type - // table stores the trait type. Weird. Probably just - // adding a "self type" table rather than overloading the - // tcache would be ok, or else adding more than one id. - tcx.tcache.insert(local_def(ifce.id), tpt); + // ifce.impl_id represents (class, iface) pair + write_ty_to_tcx(tcx, ifce.impl_id, tpt.ty); + tcx.tcache.insert(local_def(ifce.impl_id), tpt); } } _ { @@ -462,9 +454,10 @@ fn instantiate_trait_ref(ccx: @crate_ctxt, t: @ast::trait_ref, rp: bool) let rscope = type_rscope(rp); - alt lookup_def_tcx(ccx.tcx, t.path.span, t.id) { + alt lookup_def_tcx(ccx.tcx, t.path.span, t.ref_id) { ast::def_ty(t_id) { - let tpt = astconv::ast_path_to_ty(ccx, rscope, t_id, t.path, t.id); + let tpt = astconv::ast_path_to_ty(ccx, rscope, t_id, t.path, + t.ref_id); alt ty::get(tpt.ty).struct { ty::ty_trait(*) { (t_id, tpt)