Page MenuHomePhabricator

[llvm-objcopy][MachO] Implement --remove-section
AcceptedPublic

Authored by seiya on Aug 15 2019, 2:38 AM.

Details

Event Timeline

seiya created this revision.Aug 15 2019, 2:38 AM
jhenderson added inline comments.Aug 15 2019, 3:35 AM
llvm/test/tools/llvm-objcopy/MachO/remove-section.test
3

in -> by

This test should also show that you can remove multiple sections at the same time with the switch.

LGTM, no concerns from me once the test is updated

seiya updated this revision to Diff 215524.Aug 15 2019, 7:06 PM
seiya marked an inline comment as done.
  • Addressed review comments.
seiya updated this revision to Diff 215811.Aug 18 2019, 9:13 PM
  • Added a comment in the test.
jhenderson added inline comments.Aug 19 2019, 3:04 AM
llvm/test/tools/llvm-objcopy/MachO/remove-section.test
7

"Error case 1:" is meaningless without other error cases.

63

How easy would it be to use FileCheck --check-prefixes, and possibly -D<NAME> options to share this block with the previous test case? That would avoid duplicating a big chunk of the expected output. You could do something like:

# RUN: ... | FileCheck --check-prefixes=REMOVE-COMMON,REMOVE-TEXT-ONLY
# RUN: ... | FileCheck --check-prefixes=REMOVE-COMMON

# REMOVE-COMMON:
# REMOVE-COMMON-NEXT:
...
# REMOVE-TEXT-ONLY-NEXT:
# REMOVE-TEXT-ONLY-NEXT:
...
# REMOVE-COMMON-NEXT:
seiya marked 2 inline comments as done.Aug 19 2019, 2:44 PM
seiya updated this revision to Diff 215989.Aug 19 2019, 2:44 PM

Address review comments.

jhenderson accepted this revision.Aug 20 2019, 2:11 AM

LGTM, thanks.

This revision is now accepted and ready to land.Aug 20 2019, 2:11 AM
seiya updated this revision to Diff 216120.Aug 20 2019, 6:06 AM

Update CommandGuide.

jhenderson accepted this revision.Aug 20 2019, 6:16 AM
seiya updated this revision to Diff 216129.Aug 20 2019, 6:24 AM

CommandGuide: Removed remove "the" before "<section>".

jhenderson added inline comments.Aug 20 2019, 6:24 AM
llvm/docs/CommandGuide/llvm-objcopy.rst
84

Delete "the"

seiya marked an inline comment as done.Aug 20 2019, 6:26 AM