[PATCH] D69146: [install-name-tool] Add first bits for install-name-tool

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/InstallNameToolOpts.td:1
+include "llvm/Option/OptParser.td"
+
----------------
compnerd wrote:
> Add the LLVM copyright header please.
While we're at it, the other tablegen option files are missing the header...


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:75
   // copied-over without digging into its structure.
-  ArrayRef<uint8_t> Payload;
+  std::vector<uint8_t> Payload; 
 
----------------
Changing this to an owning container could make llvm-objcopy/strip significantly slower on MachO files due to an extra copy. Is there any way to avoid this? Maybe even using a bump allocator (like StringSaver) if you do need to allocate in some cases?


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:320-322
+  const bool IsStrip = Stem.contains("strip");
+  const bool IsInstallNameTool = (Stem.contains("install-name-tool") ||
+                                  Stem.contains("install_name_tool"));
----------------
Once we've gone past 2 tools, I think we should have an enum instead -- and maybe add something to `StringSwitch` so you could write:

```
ToolType Tool = StringSwitch(Stem)
  .Contains("strip", ToolType::Strip)
  .Contains("install-name-tool", ToolType::InstallNameTool)
  .Contains("install_name_tool", ToolType::InstallNameTool)
  .Default(ToolType::Objcopy);
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D69146