Improve name mangling for gdb

Remove __extensions__ from method symbols as well as the meth_XXX. The XXX is
now used to append a few characters at the end of the name of the symbol.

Closes #6602

5 files changed, 99 insertions, 58 deletions

diff --git a/src/librustc/back/link.rs b/src/librustc/back/link.rs
index 170f38f3342..fe2d59a0ba7 100644
--- a/src/librustc/back/link.rs
+++ b/src/librustc/back/link.rs
@@ -663,8 +663,7 @@ pub fn truncated_hash_result(symbol_hasher: &mut hash::State) -> ~str {
 pub fn symbol_hash(tcx: ty::ctxt,
                    symbol_hasher: &mut hash::State,
                    t: ty::t,
-                   link_meta: LinkMeta)
-                   -> @str {
+                   link_meta: LinkMeta) -> @str {
     // NB: do *not* use abbrevs here as we want the symbol names
     // to be independent of one another in the crate.
 
@@ -719,7 +718,7 @@ pub fn sanitize(s: &str) -> ~str {
             'a' .. 'z'
             | 'A' .. 'Z'
             | '0' .. '9'
-            | '_' => result.push_char(c),
+            | '_' | '.' => result.push_char(c),
 
             _ => {
                 let mut tstr = ~"";
@@ -740,22 +739,37 @@ pub fn sanitize(s: &str) -> ~str {
     return result;
 }
 
-pub fn mangle(sess: Session, ss: path) -> ~str {
+pub fn mangle(sess: Session, ss: path,
+              hash: Option<&str>, vers: Option<&str>) -> ~str {
     // Follow C++ namespace-mangling style, see
     // http://en.wikipedia.org/wiki/Name_mangling for more info.
+    //
+    // It turns out that on OSX you can actually have arbitrary symbols in
+    // function names (at least when given to LLVM), but this is not possible
+    // when using unix's linker. Perhaps one day when we just a linker from LLVM
+    // we won't need to do this name mangling. The problem with name mangling is
+    // that it seriously limits the available characters. For example we can't
+    // have things like @T or ~[T] in symbol names when one would theoretically
+    // want them for things like impls of traits on that type.
+    //
+    // To be able to work on all platforms and get *some* reasonable output, we
+    // use C++ name-mangling.
 
     let mut n = ~"_ZN"; // _Z == Begin name-sequence, N == nested
 
+    let push = |s: &str| {
+        let sani = sanitize(s);
+        n.push_str(fmt!("%u%s", sani.len(), sani));
+    };
+
     // First, connect each component with <len, name> pairs.
     for s in ss.iter() {
         match *s {
             path_name(s) | path_mod(s) | path_pretty_name(s, _) => {
-                let sani = sanitize(sess.str_of(s));
-                n.push_str(fmt!("%u%s", sani.len(), sani));
+                push(sess.str_of(s))
             }
         }
     }
-    n.push_char('E'); // End name-sequence.
 
     // next, if any identifiers are "pretty" and need extra information tacked
     // on, then use the hash to generate two unique characters. For now
@@ -764,17 +778,27 @@ pub fn mangle(sess: Session, ss: path) -> ~str {
         "abcdefghijklmnopqrstuvwxyz\
          ABCDEFGHIJKLMNOPQRSTUVWXYZ\
          0123456789";
+    let mut hash = match hash { Some(s) => s.to_owned(), None => ~"" };
     for s in ss.iter() {
         match *s {
             path_pretty_name(_, extra) => {
                 let hi = (extra >> 32) as u32 as uint;
                 let lo = extra as u32 as uint;
-                n.push_char(EXTRA_CHARS[hi % EXTRA_CHARS.len()] as char);
-                n.push_char(EXTRA_CHARS[lo % EXTRA_CHARS.len()] as char);
+                hash.push_char(EXTRA_CHARS[hi % EXTRA_CHARS.len()] as char);
+                hash.push_char(EXTRA_CHARS[lo % EXTRA_CHARS.len()] as char);
             }
             _ => {}
         }
     }
+    if hash.len() > 0 {
+        push(hash);
+    }
+    match vers {
+        Some(s) => push(s),
+        None => {}
+    }
+
+    n.push_char('E'); // End name-sequence.
     n
 }
 
@@ -782,10 +806,15 @@ pub fn exported_name(sess: Session,
                      path: path,
                      hash: &str,
                      vers: &str) -> ~str {
-    mangle(sess,
-           vec::append_one(
-               vec::append_one(path, path_name(sess.ident_of(hash))),
-               path_name(sess.ident_of(vers))))
+    // The version will get mangled to have a leading '_', but it makes more
+    // sense to lead with a 'v' b/c this is a version...
+    let vers = if vers.len() > 0 && !char::is_XID_start(vers.char_at(0)) {
+        "v" + vers
+    } else {
+        vers.to_owned()
+    };
+
+    mangle(sess, path, Some(hash), Some(vers.as_slice()))
 }
 
 pub fn mangle_exported_name(ccx: &mut CrateContext,
@@ -803,31 +832,33 @@ pub fn mangle_internal_name_by_type_only(ccx: &mut CrateContext,
     let s = ppaux::ty_to_short_str(ccx.tcx, t);
     let hash = get_symbol_hash(ccx, t);
     return mangle(ccx.sess,
-        ~[path_name(ccx.sess.ident_of(name)),
-          path_name(ccx.sess.ident_of(s)),
-          path_name(ccx.sess.ident_of(hash))]);
+                  ~[path_name(ccx.sess.ident_of(name)),
+                    path_name(ccx.sess.ident_of(s))],
+                  Some(hash.as_slice()),
+                  None);
 }
 
 pub fn mangle_internal_name_by_type_and_seq(ccx: &mut CrateContext,
-                                         t: ty::t,
-                                         name: &str) -> ~str {
+                                            t: ty::t,
+                                            name: &str) -> ~str {
     let s = ppaux::ty_to_str(ccx.tcx, t);
     let hash = get_symbol_hash(ccx, t);
     return mangle(ccx.sess,
-        ~[path_name(ccx.sess.ident_of(s)),
-          path_name(ccx.sess.ident_of(hash)),
-          path_name(gensym_name(name))]);
+                  ~[path_name(ccx.sess.ident_of(s)),
+                    path_name(gensym_name(name))],
+                  Some(hash.as_slice()),
+                  None);
 }
 
 pub fn mangle_internal_name_by_path_and_seq(ccx: &mut CrateContext,
                                             mut path: path,
                                             flav: &str) -> ~str {
     path.push(path_name(gensym_name(flav)));
-    mangle(ccx.sess, path)
+    mangle(ccx.sess, path, None, None)
 }
 
 pub fn mangle_internal_name_by_path(ccx: &mut CrateContext, path: path) -> ~str {
-    mangle(ccx.sess, path)
+    mangle(ccx.sess, path, None, None)
 }
 
 pub fn mangle_internal_name_by_seq(_ccx: &mut CrateContext, flav: &str) -> ~str {
diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs
index 79440b03d76..16afc6f36ca 100644
--- a/src/librustc/middle/trans/base.rs
+++ b/src/librustc/middle/trans/base.rs
@@ -77,7 +77,7 @@ use std::local_data;
 use extra::time;
 use extra::sort;
 use syntax::ast::Ident;
-use syntax::ast_map::{path, path_elt_to_str, path_name};
+use syntax::ast_map::{path, path_elt_to_str, path_name, path_pretty_name};
 use syntax::ast_util::{local_def};
 use syntax::attr;
 use syntax::attr::AttrMetaMethods;
@@ -2646,8 +2646,7 @@ pub fn register_method(ccx: @mut CrateContext,
     let mty = ty::node_id_to_type(ccx.tcx, id);
 
     let mut path = (*path).clone();
-    path.push(path_name(gensym_name("meth")));
-    path.push(path_name(m.ident));
+    path.push(path_pretty_name(m.ident, token::gensym("meth") as u64));
 
     let sym = exported_name(ccx, path, mty, m.attrs);
 
diff --git a/src/libsyntax/ast_map.rs b/src/libsyntax/ast_map.rs
index cb1c0eab720..1165ca0c68f 100644
--- a/src/libsyntax/ast_map.rs
+++ b/src/libsyntax/ast_map.rs
@@ -16,20 +16,20 @@ use ast_util;
 use codemap::Span;
 use codemap;
 use diagnostic::span_handler;
+use parse::token::get_ident_interner;
 use parse::token::ident_interner;
 use parse::token::special_idents;
 use print::pprust;
 use visit::{Visitor, fn_kind};
 use visit;
 
-use std::hash;
 use std::hashmap::HashMap;
 use std::vec;
 
 #[deriving(Clone, Eq)]
 pub enum path_elt {
     path_mod(Ident),
-    path_name(Ident)
+    path_name(Ident),
 
     // A pretty name can come from an `impl` block. We attempt to select a
     // reasonable name for debuggers to see, but to guarantee uniqueness with
@@ -98,8 +98,8 @@ pub struct Ctx {
 }
 
 impl Ctx {
-    fn extend(&self, elt: Ident) -> @path {
-        @vec::append(self.path.clone(), [path_name(elt)])
+    fn extend(&self, elt: path_elt) -> @path {
+        @vec::append(self.path.clone(), [elt])
     }
 
     fn map_method(&mut self,
@@ -120,7 +120,7 @@ impl Ctx {
                       struct_def: @ast::struct_def,
                       parent_node: ast_node,
                       ident: ast::Ident) {
-        let p = self.extend(ident);
+        let p = self.extend(path_name(ident));
 
         // If this is a tuple-like struct, register the constructor.
         match struct_def.ctor_id {
@@ -196,6 +196,28 @@ impl Ctx {
 
         visit::walk_pat(self, pat, ());
     }
+
+    fn impl_pretty_name(&self, trait_ref: &Option<trait_ref>,
+                        ty: &Ty, default: Ident) -> path_elt {
+        let itr = get_ident_interner();
+        let ty_ident = match ty.node {
+            ty_path(ref path, _, _) => path.segments.last().identifier,
+            _ => default
+        };
+        let hash = (trait_ref, ty).hash();
+        match *trait_ref {
+            None => path_pretty_name(ty_ident, hash),
+            Some(ref trait_ref) => {
+                // XXX: this dollar sign is actually a relic of being one of the
+                //      very few valid symbol names on unix. These kinds of
+                //      details shouldn't be exposed way up here in the ast.
+                let s = fmt!("%s$%s",
+                             itr.get(trait_ref.path.segments.last().identifier.name),
+                             itr.get(ty_ident.name));
+                path_pretty_name(Ident::new(itr.gensym(s)), hash)
+            }
+        }
+    }
 }
 
 impl Visitor<()> for Ctx {
@@ -205,40 +227,27 @@ impl Visitor<()> for Ctx {
         self.map.insert(i.id, node_item(i, item_path));
         match i.node {
             item_impl(_, ref maybe_trait, ref ty, ref ms) => {
+                // Right now the ident on impls is __extensions__ which isn't
+                // very pretty when debugging, so attempt to select a better
+                // name to use.
+                let elt = self.impl_pretty_name(maybe_trait, ty, i.ident);
+
                 let impl_did = ast_util::local_def(i.id);
                 for m in ms.iter() {
-                    let extended = { self.extend(i.ident) };
+                    let extended = { self.extend(elt) };
                     self.map_method(impl_did, extended, *m, false)
                 }
 
-                // Right now the ident on impls is __extensions__ which isn't
-                // very pretty when debugging, so attempt to select a better
-                // name to use.
-                let name = match *maybe_trait {
-                    Some(ref trait_ref) => {
-                        trait_ref.path.segments.last().identifier
-                    }
-                    None => {
-                        match ty.node {
-                            ty_path(ref p, _, _) => {
-                                p.segments.last().identifier
-                            }
-                            // oh well, just give up for now
-                            _ => { i.ident }
-                        }
-                    }
-                };
-
-                let hash = hash::hash_keyed_2(maybe_trait, ty, 0, 0);
-                self.path.push(path_pretty_name(name, hash));
+                self.path.push(elt);
             }
             item_enum(ref enum_definition, _) => {
                 for v in (*enum_definition).variants.iter() {
+                    let elt = path_name(i.ident);
                     // FIXME #2543: bad clone
                     self.map.insert(v.node.id,
                                     node_variant((*v).clone(),
                                                  i,
-                                                 self.extend(i.ident)));
+                                                 self.extend(elt)));
                 }
             }
             item_foreign_mod(ref nm) => {
@@ -257,7 +266,9 @@ impl Visitor<()> for Ctx {
                                                       // FIXME (#2543)
                                                       if nm.sort ==
                                                             ast::named {
-                                                        self.extend(i.ident)
+                                                          let e = path_name(
+                                                              i.ident);
+                                                          self.extend(e)
                                                       } else {
                                                         // Anonymous extern
                                                         // mods go in the
@@ -276,7 +287,7 @@ impl Visitor<()> for Ctx {
                     self.map.insert(p.ref_id, node_item(i, item_path));
                 }
                 for tm in methods.iter() {
-                    let ext = { self.extend(i.ident) };
+                    let ext = { self.extend(path_name(i.ident)) };
                     let d_id = ast_util::local_def(i.id);
                     match *tm {
                         required(ref m) => {
diff --git a/src/test/compile-fail/ambig_impl_2_exe.rs b/src/test/compile-fail/ambig_impl_2_exe.rs
index 1cf08b7f503..0f5bac795e7 100644
--- a/src/test/compile-fail/ambig_impl_2_exe.rs
+++ b/src/test/compile-fail/ambig_impl_2_exe.rs
@@ -15,6 +15,6 @@ use ambig_impl_2_lib::me;
 trait me {
     fn me(&self) -> uint;
 }
-impl me for uint { fn me(&self) -> uint { *self } } //~ NOTE is `__extensions__::me`
+impl me for uint { fn me(&self) -> uint { *self } } //~ NOTE is `me$uint::me`
 fn main() { 1u.me(); } //~ ERROR multiple applicable methods in scope
 //~^ NOTE is `ambig_impl_2_lib::__extensions__::me`
diff --git a/src/test/compile-fail/ambig_impl_unify.rs b/src/test/compile-fail/ambig_impl_unify.rs
index ce8c2a29544..df88ff1f0d0 100644
--- a/src/test/compile-fail/ambig_impl_unify.rs
+++ b/src/test/compile-fail/ambig_impl_unify.rs
@@ -13,11 +13,11 @@ trait foo {
 }
 
 impl foo for ~[uint] {
-    fn foo(&self) -> int {1} //~ NOTE candidate #1 is `__extensions__::foo`
+    fn foo(&self) -> int {1} //~ NOTE candidate #1 is `foo$__extensions__::foo`
 }
 
 impl foo for ~[int] {
-    fn foo(&self) -> int {2} //~ NOTE candidate #2 is `__extensions__::foo`
+    fn foo(&self) -> int {2} //~ NOTE candidate #2 is `foo$__extensions__::foo`
 }
 
 fn main() {