[PATCH] D41684: [llvm-objcopy] Add --localize-hidden option

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/localize-hidden.test:35
+Symbols:
+  Global:
+    - Name:     _start
----------------
Why do you have so many different symbols here?

Also, you should have symbols with hidden visibility with both local and weak bindings, to show that a) local symbols remain unchanged, and b) weak symbols are changed.

Finally, a symbol with STV_PROTECTED visibility should also be included to show that it is not localized.


================
Comment at: tools/llvm-objcopy/Object.cpp:184-188
+  // Now that the local symbols aren't grouped at the start we have to reorder
+  // the symbols to respect this property.
+  std::stable_partition(
+      std::begin(Symbols), std::end(Symbols),
+      [](const SymPtr &Sym) { return Sym->Binding == STB_LOCAL; });
----------------
I'd add blank lines either side of this block.


Repository:
  rL LLVM

https://reviews.llvm.org/D41684