[PATCH] D121005: [objcopy][NFC] Move NameOrPattern::create() into CommonConfig.h

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 6 22:41:33 PST 2022


jhenderson added inline comments.


================
Comment at: llvm/include/llvm/ObjCopy/CommonConfig.h:113-115
+  static inline Expected<NameOrPattern>
   create(StringRef Pattern, MatchStyle MS,
+         llvm::function_ref<Error(Error)> ErrorCallback) {
----------------
Given the complexity of this function, I think it should be in a .cpp file.


================
Comment at: llvm/unittests/ObjCopy/ObjCopyTest.cpp:369-372
+void removeSectionByPatternImpl(
+    const char *YamlCreationString,
+    std::function<bool(const Binary &File)> IsValidFormat,
+    StringRef SectionWildcard, StringRef SectionName) {
----------------
It seems to me like this function has a lot of copied code in it. I think you should be  able to share most of it rather than just copying.


================
Comment at: llvm/unittests/ObjCopy/ObjCopyTest.cpp:382
+
+  // Check that section is presented.
+  bool HasSection = false;
----------------



================
Comment at: llvm/unittests/ObjCopy/ObjCopyTest.cpp:439
+      R"(
+--- !COFF
+header:
----------------
If this YAML is identical to other cases, pull it into a shared constant somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121005



More information about the llvm-commits mailing list