[PATCH] D120927: [llvm] remove empty __LLVM segment in llvm-bitcode-strip

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/bitcode-strip-remove-nonempty-segment.test:4-5
 # RUN: llvm-bitcode-strip -r %t -o %t2
 # RUN: llvm-readobj --sections %t2 | FileCheck --implicit-check-not=Name %s
+# RUN: llvm-otool -l %t2 | FileCheck --check-prefix=SEGMENTS %s
 
----------------
Ideally, we'd use the same invocation to check everything at once.  I believe llvm-readobj has an option to dump Mach-O segments. Does that suffice?


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/bitcode-strip-remove.test:12
+
+# SEGMENTS-NOT: segname __LLVM
 
----------------
It's usually a bad idea to have a purely negative check like this, because they can rot too easily. Better would be to check all segments and show the one you want isn't in that set.


================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:1224
+  // We only support -r for now, which removes all bitcode sections and
+  // the __LLVM segment if its now empty.
   cantFail(Config.ToRemove.addMatcher(NameOrPattern::create(
----------------
its - belonging to it
it's - it is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120927