[PATCH] D102277: [llvm-objcopy][NFC] Refactor CopyConfig structure - categorize options.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ConfigManager.cpp:561
       const uint8_t Invalid = 0xff;
-      ResConfig.NewSymbolVisibility =
+      const_cast<ConfigManager *>(this)->ELF.NewSymbolVisibility =
           StringSwitch<uint8_t>(*NewSymbolVisibility)
----------------
`const_cast` is always a bit scary to me. Would it make more sense to make `NewSymbolVisibility` and `SymbolsToAdd` mutable in the ELFConfig?


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFConfig.h:32
 struct ELFConfig {
+  // Only applicable when --output-format!=binary (e.g. elf64-x86-64).
+  Optional<MachineInfo> OutputArch;
----------------



================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFConfig.h:35
+
+  // Cached gnu_debuglink's target CRC
+  uint32_t GnuDebugLinkCRC32;
----------------
Unlike the Section and Symbol matchers comments below, this is more of a sentence, so deserves a `.`. Might as well fix whilst you're moving the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102277