[PATCH] D120669: [llvm] add -r functionality to llvm-bitcode-strip

[Richard Howell via Phabricator via llvm-commits]

rmaz marked 3 inline comments as done.
rmaz added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:1225-1229
+  // We only support -r for now, which removes all bitcode sections
+  if (Error E = Config.ToRemove.addMatcher(NameOrPattern::create(
+          "__LLVM,__bundle", MatchStyle::Literal, ErrorCallback))) {
+    return std::move(E);
+  }
----------------
jhenderson wrote:
> Period at end of sentences. Also, no braces for single-line if statements.
> 
> I think you need additional sections in your test case with a section in the `__LLVM` segment with a different name, and a `__bundle` section in a non-LLVM segment, to show that it's only the combination of the two that is removed.
> 
> It may also be a good idea to show what happens (in a test case) when the error callback is triggered in this code path.
I don't believe you can trigger the error callback on this code path as we are using a `MatchStyle::Literal`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120669