[PATCH] D103260: [llvm-objcopy][NFC] Refactor CopyConfig structure - remove lazy options processing.

Fri May 28 00:36:39 PDT 2021

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM overall. Just a few nits.

FYI, I'm off work all of next week, so it'll be the week after before I can comment on any more patches.

================
Comment at: llvm/tools/llvm-objcopy/CommonConfig.h:155
+  Function,
+  Indirect_function,
+  Debug,
----------------
It's not explicitly stated, but reading between the lines, and looking at the example in the coding standards, I think this should be `IndirectFunction` (and `UniqueObject` below).

================
Comment at: llvm/tools/llvm-objcopy/CommonConfig.h:165
+// Symbol info specified by --add-symbol option. Symbol flags not supported
+// on a concrete format should be ignored.
+struct NewSymbolInfo {
----------------

================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:503-504

+// Add symbol to the Object symbol table according to NewSymbolInfo SymInfo
+// descriptor.
+static void addSymbol(Object &Obj, const NewSymbolInfo &SymInfo,
----------------
I think this keeps the comment a little more generic, so that if the signature changes, the comment remains valid.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103260