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

Author: catamorphism

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 }

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

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 {

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] {

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));
               }
             };

src/rustc/middle/resolve.rs

@@ -408,8 +408,11 @@ fn maybe_insert(e: @env, id: node_id, def: option<def>) {
 }
 
 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),

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);
                         }
                     }
                 }

src/rustc/middle/ty.rs

@@ -2549,7 +2549,7 @@ fn impl_trait(cx: ctxt, id: ast::def_id) -> option<t> {
         #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(*),

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 {

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)