[PATCH] D46177: [llvm-objcopy] Add --globalize-symbol option

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:323
            (Sym.Visibility == STV_HIDDEN || Sym.Visibility == STV_INTERNAL)) ||
-          (!Config.SymbolsToLocalize.empty() &&
-           std::find(std::begin(Config.SymbolsToLocalize),
-                     std::end(Config.SymbolsToLocalize),
-                     Sym.Name) != std::end(Config.SymbolsToLocalize)))
+          contains(Config.SymbolsToLocalize, Sym.Name))
         Sym.Binding = STB_LOCAL;
----------------
empty() was not completely useless here imo  - this flag is probably not the most frequently used flag, thus when it's not specified the "early check" would save us at least 3 function calls per  symbol (at least in the debug builds) (std::begin/end/find)


================
Comment at: tools/llvm-objcopy/llvm-objcopy.h:38
 
+template <class C, class T>
+auto contains(const C &Vector, const T &Elt)
----------------
if it's only used in llvm-objcopy.cpp - maybe move it there ? (and make it static to avoid putting it into the global namespace)



Repository:
  rL LLVM

https://reviews.llvm.org/D46177