[PATCH] D88827: [llvm-objcopy][NFC] Move core implementation of llvm-objcopy into separate library.

Tue Nov 23 01:06:22 PST 2021

jhenderson added a reviewer: sbc100.
jhenderson added a comment.

A couple of nits remaining, otherwise basically looks good. I'd still like:

1. @sbc100's response to the library division. I have a very slight preference for separate, but don't care enough about it to push for it, if others are opposed. Adding them as a reviewer in case it gets their attention.
2. I'd like to see patches based on this patch for the other issues that your deferring (particularly the IDE one).

Once those are dealt with, I'll give this an LGTM.

================
Comment at: llvm/include/llvm/Object/ObjCopy/ObjCopy.h:12
+
+#include "llvm/Object/ArchiveWriter.h"
+#include "llvm/Support/Error.h"
----------------
I think you can forward declare `Binary`, `Archive` and `NewArchiveMember`, right, so that you don't need this header?

================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:627
                                      : MatchStyle::Wildcard;
   MatchStyle SymbolMatchStyle = InputArgs.hasArg(OBJCOPY_regex)
                                     ? MatchStyle::Regex
----------------
More clang-formatting that can be done as you rename the file.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88827