[PATCH] D66281: [llvm-objcopy][MachO] Implement --strip-all

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:49
+    if (ISE.Symbol)
+      const_cast<SymbolEntry *>(*ISE.Symbol)->Referenced = true;
+}
----------------
Can you skip the `const_cast` here and add a mutable `getSymbolByIndex` overload so you can remove the const modifiers elsewhere?


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:53-57
+  auto RemovePred = [Config](const std::unique_ptr<SymbolEntry> &N) {
+    if (N->Referenced)
+      return false;
+    return Config.StripAll;
+  };
----------------
For ELF files, --strip-all seems to strip all symbols regardless of whether they're referenced by relocations, is that not true for MachO files?


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:102
   removeSections(Config, Obj);
+  markSymbols(Config, Obj);
+  removeSymbols(Config, Obj);
----------------
Marking symbols should be behind whatever flags require it, so we don't needlessly traverse symbols


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66281/new/

https://reviews.llvm.org/D66281