When decoding types, indicate to the def-id conversion function what kind of

def-id we have, so that the inliner can distinguish between external and
internal def-ids.  Also add some comments explaining the distinction!

Fixes #4516.

r=graydon

Author: nikomatsakis

src/librustc/metadata/decoder.rs

@@ -264,9 +264,8 @@ fn variant_disr_val(d: ebml::Doc) -> Option<int> {
 
 fn doc_type(doc: ebml::Doc, tcx: ty::ctxt, cdata: cmd) -> ty::t {
     let tp = reader::get_doc(doc, tag_items_data_item_type);
-    parse_ty_data(tp.data, cdata.cnum, tp.start, tcx, |did| {
-        translate_def_id(cdata, did)
-    })
+    parse_ty_data(tp.data, cdata.cnum, tp.start, tcx,
+                  |_, did| translate_def_id(cdata, did))
 }
 
 fn item_type(item_id: ast::def_id, item: ebml::Doc,
@@ -289,9 +288,8 @@ fn item_ty_param_bounds(item: ebml::Doc, tcx: ty::ctxt, cdata: cmd)
     -> @~[ty::param_bounds] {
     let mut bounds = ~[];
     for reader::tagged_docs(item, tag_items_data_item_ty_param_bounds) |p| {
-        let bd = parse_bounds_data(p.data, p.start, cdata.cnum, tcx, |did| {
-            translate_def_id(cdata, did)
-        });
+        let bd = parse_bounds_data(p.data, p.start, cdata.cnum, tcx,
+                                   |_, did| translate_def_id(cdata, did));
         bounds.push(bd);
     }
     @bounds

src/librustc/metadata/tydecode.rs

@@ -33,13 +33,33 @@ export parse_state_from_data;
 export parse_arg_data, parse_ty_data, parse_def_id, parse_ident;
 export parse_bounds_data;
 export pstate;
+export DefIdSource, NominalType, TypeWithId, TypeParameter;
 
 // Compact string representation for ty::t values. API ty_str &
 // parse_from_str. Extra parameters are for converting to/from def_ids in the
 // data buffer. Whatever format you choose should not contain pipe characters.
 
-// Callback to translate defs to strs or back:
-type conv_did = fn(ast::def_id) -> ast::def_id;
+// Def id conversion: when we encounter def-ids, they have to be translated.
+// For example, the crate number must be converted from the crate number used
+// in the library we are reading from into the local crate numbers in use
+// here.  To perform this translation, the type decoder is supplied with a
+// conversion function of type `conv_did`.
+//
+// Sometimes, particularly when inlining, the correct translation of the
+// def-id will depend on where it originated from.  Therefore, the conversion
+// function is given an indicator of the source of the def-id.  See
+// astencode.rs for more information.
+enum DefIdSource {
+    // Identifies a struct, trait, enum, etc.
+    NominalType,
+
+    // Identifies a type alias (`type X = ...`).
+    TypeWithId,
+
+    // Identifies a type parameter (`fn foo<X>() { ... }`).
+    TypeParameter
+}
+type conv_did = fn(source: DefIdSource, ast::def_id) -> ast::def_id;
 
 type pstate = {data: @~[u8], crate: int, mut pos: uint, tcx: ty::ctxt};
 
@@ -245,21 +265,21 @@ fn parse_ty(st: @pstate, conv: conv_did) -> ty::t {
       'c' => return ty::mk_char(st.tcx),
       't' => {
         assert (next(st) == '[');
-        let def = parse_def(st, conv);
+        let def = parse_def(st, NominalType, conv);
         let substs = parse_substs(st, conv);
         assert next(st) == ']';
         return ty::mk_enum(st.tcx, def, substs);
       }
       'x' => {
         assert next(st) == '[';
-        let def = parse_def(st, conv);
+        let def = parse_def(st, NominalType, conv);
         let substs = parse_substs(st, conv);
         let vstore = parse_vstore(st);
         assert next(st) == ']';
         return ty::mk_trait(st.tcx, def, substs, vstore);
       }
       'p' => {
-        let did = parse_def(st, conv);
+        let did = parse_def(st, TypeParameter, conv);
         return ty::mk_param(st.tcx, parse_int(st) as uint, did);
       }
       's' => {
@@ -327,17 +347,14 @@ fn parse_ty(st: @pstate, conv: conv_did) -> ty::t {
         }
       }
       '"' => {
-        let def = parse_def(st, conv);
+        let def = parse_def(st, TypeWithId, conv);
         let inner = parse_ty(st, conv);
         ty::mk_with_id(st.tcx, inner, def)
       }
       'B' => ty::mk_opaque_box(st.tcx),
       'a' => {
-          debug!("saw a class");
           assert (next(st) == '[');
-          debug!("saw a [");
-          let did = parse_def(st, conv);
-          debug!("parsed a def_id %?", did);
+          let did = parse_def(st, NominalType, conv);
           let substs = parse_substs(st, conv);
           assert (next(st) == ']');
           return ty::mk_struct(st.tcx, did, substs);
@@ -356,11 +373,12 @@ fn parse_mt(st: @pstate, conv: conv_did) -> ty::mt {
     ty::mt { ty: parse_ty(st, conv), mutbl: m }
 }
 
-fn parse_def(st: @pstate, conv: conv_did) -> ast::def_id {
+fn parse_def(st: @pstate, source: DefIdSource,
+             conv: conv_did) -> ast::def_id {
     let mut def = ~[];
     while peek(st) != '|' { def.push(next_byte(st)); }
     st.pos = st.pos + 1u;
-    return conv(parse_def_id(def));
+    return conv(source, parse_def_id(def));
 }
 
 fn parse_int(st: @pstate) -> int {

src/librustc/middle/astencode.rs

@@ -17,6 +17,7 @@ use e = metadata::encoder;
 use metadata::decoder;
 use metadata::encoder;
 use metadata::tydecode;
+use metadata::tydecode::{DefIdSource, NominalType, TypeWithId, TypeParameter};
 use metadata::tyencode;
 use middle::freevars::freevar_entry;
 use middle::typeck::{method_origin, method_map_entry, vtable_res};
@@ -168,14 +169,55 @@ fn reserve_id_range(sess: Session,
 
 impl extended_decode_ctxt {
     fn tr_id(id: ast::node_id) -> ast::node_id {
+        /*!
+         *
+         * Translates an internal id, meaning a node id that is known
+         * to refer to some part of the item currently being inlined,
+         * such as a local variable or argument.  All naked node-ids
+         * that appear in types have this property, since if something
+         * might refer to an external item we would use a def-id to
+         * allow for the possibility that the item resides in another
+         * crate.
+         */
+
         // from_id_range should be non-empty
         assert !ast_util::empty(self.from_id_range);
         (id - self.from_id_range.min + self.to_id_range.min)
     }
     fn tr_def_id(did: ast::def_id) -> ast::def_id {
+        /*!
+         *
+         * Translates an EXTERNAL def-id, converting the crate number
+         * from the one used in the encoded data to the current crate
+         * numbers..  By external, I mean that it be translated to a
+         * reference to the item in its original crate, as opposed to
+         * being translated to a reference to the inlined version of
+         * the item.  This is typically, but not always, what you
+         * want, because most def-ids refer to external things like
+         * types or other fns that may or may not be inlined.  Note
+         * that even when the inlined function is referencing itself
+         * recursively, we would want `tr_def_id` for that
+         * reference--- conceptually the function calls the original,
+         * non-inlined version, and trans deals with linking that
+         * recursive call to the inlined copy.
+         *
+         * However, there are a *few* cases where def-ids are used but
+         * we know that the thing being referenced is in fact *internal*
+         * to the item being inlined.  In those cases, you should use
+         * `tr_intern_def_id()` below.
+         */
+
         decoder::translate_def_id(self.dcx.cdata, did)
     }
     fn tr_intern_def_id(did: ast::def_id) -> ast::def_id {
+        /*!
+         *
+         * Translates an INTERNAL def-id, meaning a def-id that is
+         * known to refer to some part of the item currently being
+         * inlined.  In that case, we want to convert the def-id to
+         * refer to the current crate and to the new, inlined node-id.
+         */
+
         assert did.crate == ast::local_crate;
         ast::def_id { crate: ast::local_crate, node: self.tr_id(did.node) }
     }
@@ -906,15 +948,17 @@ trait ebml_decoder_decoder_helpers {
     fn read_bounds(xcx: extended_decode_ctxt) -> @~[ty::param_bound];
     fn read_ty_param_bounds_and_ty(xcx: extended_decode_ctxt)
                                 -> ty::ty_param_bounds_and_ty;
+    fn convert_def_id(xcx: extended_decode_ctxt,
+                      source: DefIdSource,
+                      did: ast::def_id) -> ast::def_id;
 }
 
 impl reader::Decoder: ebml_decoder_decoder_helpers {
-
     fn read_arg(xcx: extended_decode_ctxt) -> ty::arg {
         do self.read_opaque |doc| {
             tydecode::parse_arg_data(
                 doc.data, xcx.dcx.cdata.cnum, doc.start, xcx.dcx.tcx,
-                |a| xcx.tr_def_id(a))
+                |s, a| self.convert_def_id(xcx, s, a))
         }
     }
 
@@ -927,7 +971,7 @@ impl reader::Decoder: ebml_decoder_decoder_helpers {
         do self.read_opaque |doc| {
             tydecode::parse_ty_data(
                 doc.data, xcx.dcx.cdata.cnum, doc.start, xcx.dcx.tcx,
-                |a| xcx.tr_def_id(a))
+                |s, a| self.convert_def_id(xcx, s, a))
         }
     }
 
@@ -939,7 +983,7 @@ impl reader::Decoder: ebml_decoder_decoder_helpers {
         do self.read_opaque |doc| {
             tydecode::parse_bounds_data(
                 doc.data, doc.start, xcx.dcx.cdata.cnum, xcx.dcx.tcx,
-                |a| xcx.tr_def_id(a))
+                |s, a| self.convert_def_id(xcx, s, a))
         }
     }
 
@@ -960,6 +1004,29 @@ impl reader::Decoder: ebml_decoder_decoder_helpers {
             }
         }
     }
+
+    fn convert_def_id(xcx: extended_decode_ctxt,
+                      source: tydecode::DefIdSource,
+                      did: ast::def_id) -> ast::def_id {
+        /*!
+         *
+         * Converts a def-id that appears in a type.  The correct
+         * translation will depend on what kind of def-id this is.
+         * This is a subtle point: type definitions are not
+         * inlined into the current crate, so if the def-id names
+         * a nominal type or type alias, then it should be
+         * translated to refer to the source crate.
+         *
+         * However, *type parameters* are cloned along with the function
+         * they are attached to.  So we should translate those def-ids
+         * to refer to the new, cloned copy of the type parameter.
+         */
+
+        match source {
+            NominalType | TypeWithId => xcx.tr_def_id(did),
+            TypeParameter => xcx.tr_intern_def_id(did)
+        }
+    }
 }
 
 fn decode_side_tables(xcx: extended_decode_ctxt,

src/test/auxiliary/issue4516_ty_param_lib.rs

@@ -0,0 +1,13 @@
+// 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+pub fn to_closure<A: Durable Copy>(x: A) -> @fn() -> A {
+    fn@() -> A { copy x }
+}

src/test/run-pass/issue4516_ty_param.rs

@@ -0,0 +1,25 @@
+// 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// xfail-fast - check-fast doesn't understand aux-build
+// aux-build:issue4516_ty_param_lib.rs
+
+// Trigger a bug concerning inlining of generic functions.
+// The def-ids in type parameters were not being correctly
+// resolved and hence when we checked the type of the closure
+// variable (see the library mod) to determine if the value
+// should be moved into the closure, trans failed to find
+// the relevant kind bounds.
+
+extern mod issue4516_ty_param_lib;
+use issue4516_ty_param_lib::to_closure;
+fn main() {
+    to_closure(22)();
+}