[PATCH] D102825: [llvm-strip] Add support for '--' for delimiting options from input files

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/dash-dash.test:19
 
-# CHECK-NO-INPUT: error: no input file specified
+# CHECK-OBJCOPY-NO-INPUT: error: no input file specified
+
----------------
You don't need to make this CHECK unique to objcopy, since llvm-strip and llvm-objcopy produce the same string. Just use `--check-prefix=CHECK-NO-INPUT`.


================
Comment at: llvm/test/tools/llvm-objcopy/dash-dash.test:21
+
+## Check that llvm-strip correctly treats paths after '--' as file paths,
+## even if they start with dashes.
----------------
I'd probably reorder the test cases so that equivalent cases for llvm-strip and llvm-objcopy are side-by-side, i.e. something like:

```
## llvm-objcopy case 1
## llvm-strip case 1

## llvm-objcopy case 2
## llvm-strip case 2

## llvm-objcopy case 3
## llvm-strip case 3
```

(where case 1/2/3 are the three distinct cases you test here).


================
Comment at: llvm/test/tools/llvm-objcopy/dash-dash.test:23
+## even if they start with dashes.
+# RUN: yaml2obj %s -o --strip-symbol
+# RUN: llvm-strip --remove-section=.test -o out-strip-obj1 -- --strip-symbol
----------------
Use cp to copy `test-obj` rather than running yaml2obj again unnecessarily.


================
Comment at: llvm/test/tools/llvm-objcopy/dash-dash.test:24-25
+# RUN: yaml2obj %s -o --strip-symbol
+# RUN: llvm-strip --remove-section=.test -o out-strip-obj1 -- --strip-symbol
+# RUN: llvm-strip --remove-section=.test -o out-strip-obj2 test-obj
+# RUN: cmp out-strip-obj1 out-strip-obj2
----------------
I'd avoid using -o anywhere in these tests, since that's an option itself, and just muddies the waters. Instead, use 2+ input files, which will be modified in-place (you may need to make copies of these files first so you have pristine versions for use later). Same applies in the below cases too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102825