[PATCH] D21930: [ELF] - Implement extern "c++" version script tag

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/SymbolTable.cpp:560
@@ -558,1 +559,3 @@
 
+static void overrideSymbolVersion(Symbol *Sym, StringRef Entry,
+                                  uint16_t Version) {
----------------
ruiu wrote:
> I'd name this `setVersionId`. Also rename Entry -> Name.
Done.

================
Comment at: ELF/SymbolTable.cpp:560
@@ -558,1 +559,3 @@
 
+static void overrideSymbolVersion(Symbol *Sym, StringRef Entry,
+                                  uint16_t Version) {
----------------
grimar wrote:
> ruiu wrote:
> > I'd name this `setVersionId`. Also rename Entry -> Name.
> Done.
Done.

================
Comment at: ELF/SymbolTable.cpp:591
@@ -581,1 +590,3 @@
   //   matching version tag in the file).
+
+  // Handle exact matches and build a map of demangled externs for
----------------
ruiu wrote:
> Add
> 
>   DenseMap<StringRef, SymbolBody *> Demangled;
>   if (hasExternCpp())
>     Demangled = getDemangledSyms();
> 
> here and then
I implemented the close way below. I can't use StringRef as a key as demangle() returns string.
DenseMapInfo does not have specialization for std::strings.
And so I had to use std::map instead with std::string as a key.

================
Comment at: ELF/SymbolTable.cpp:609
@@ -587,3 +608,3 @@
 
-      SymbolBody *B = find(Name);
+      SymbolBody *B = find(Sym.Name);
       if (!B || B->isUndefined()) {
----------------
ruiu wrote:
> replace this line with
> 
>   SymbolBody *B = Sym.IsExternCpp ? Demangled[Sym.Name] : find(Sym.Name);
This just will not work I think..
Ian Lance Taylor wrote about current rules in gold (http://www.airs.com/blog/archives/300):

> 
> Here are the current rules for gold:
>   If there is an exact match for the mangled name, we use it.
> ... 
>   Otherwise, we look for an extern C++ or an extern Java exact match. If we find an exact match, we use it.
> ...

So externs matching should be done after common case matching, according to that rules we probably want to follow.


http://reviews.llvm.org/D21930