[PATCH] D78474: [llvm-objcopy][MachO] Make --remove-section clean up dead symbols

[James Henderson via Phabricator via llvm-commits]

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

LGTM, with one outstanding comment/question.



================
Comment at: llvm/test/tools/llvm-objcopy/MachO/remove-section-dead-symbols.test:4
+
+# RUN: llvm-readobj --sections --symbols %t.copy | FileCheck %s
+
----------------
jhenderson wrote:
> alexshap wrote:
> > jhenderson wrote:
> > > What's the reason for dumping the sections here?
> > we have removed one section ("C") - thus we'd like to verify that the list of sections is correct 
> > ["__text", "__data"]
> I guess my question is more along the lines of "we test the section is removed elsewhere, so do we need to test here?" Happy either way, if you think there's additional value.
> I guess my question is more along the lines of "we test the section is removed elsewhere, so do we need to test here?" Happy either way, if you think there's additional value.

Just flagging this repsonse up in case you missed it. Do you think there's additional value in testing it here as well as in the regular --remove-section test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78474