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

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Config.h:38
@@ -37,1 +37,3 @@
 
+struct VersionSymbol {
+  llvm::StringRef Name;
----------------
ruiu wrote:
> `SymbolVersion` is probably better?
Done.

================
Comment at: ELF/SymbolTable.cpp:564
@@ -563,4 +563,3 @@
 
-// This function processes the --version-script option by marking all global
-// symbols with the VersionScriptGlobal flag, which acts as a filter on the
-// dynamic symbol table.
+static std::string demangleCpp(StringRef Name) {
+#if !defined(HAVE_CXXABI_H)
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > We already have `elf::demangle` function.
> > Sure we have it, but it will not work here I think. At least it has 2 conditions that we don't want to see here:
> > 
> > 
> > ```
> >   if (!Config->Demangle)
> >     return Name;
> > ```
> > 
> > ```
> >   // __cxa_demangle can be used to demangle strings other than symbol
> >   // names which do not necessarily start with "_Z". Name can be
> >   // either a C or C++ symbol. Don't call __cxa_demangle if the name
> >   // does not look like a C++ symbol name to avoid getting unexpected
> >   // result for a C symbol that happens to match a mangled type name.
> >   if (!Name.startswith("_Z"))
> >     return Name;
> > ```
> > 
> > 
> > 
> You can remove `if (!Config->Demangle)` from the function and check the condition on caller side. As to the second condition, you actually want it, no?
Right.


http://reviews.llvm.org/D21930