[PATCH] D102665: [llvm-objcopy] Add support for '--' for delimiting options from input/output files

Tue May 18 03:09:23 PDT 2021

jhenderson added a comment.

In D102665#2765456 <https://reviews.llvm.org/D102665#2765456>, @sdmitriev wrote:

> I agree that it would be better to create a single LIT test for a file name beginning with dash, but it is not clear how to write it. I assume such test would require use of %T (directory of a %t), but according to the documentation it is deprecated now. So, I decided to follow the same strategy for testing this patch as was used in a similar llvm-rc change (D56743 <https://reviews.llvm.org/D56743>) - updating few arbitrary tests to use ‘--’ separator for the file names. If you have ideas how to write a test for a file name beginning with ‘-‘, please share.

You can use something like `mkdir %t` in tests (see various cases throughout the testsuites) and then cd into that directory, where you can then use relative names, e.g. to files starting with `-`, without neeing `%t` to be involved.

>> Is there a particular reason you've singled out llvm-objcopy, and not llvm-strip?
>
> No, no particular reason. I only needed this to be supported in llvm-objcopy, so I’ve changed this tool only. But I can prepare a similar change for llvm-strip as well in a separate patch.

Sounds reasonable.

================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:478
+
+  const auto *DashDash =
+      std::find_if(RawArgsArr.begin(), RawArgsArr.end(),
----------------
Actually, `auto` doesn't really gain us anything here, as the return type is not obvious from the single line here. Just make the type explicit.

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

https://reviews.llvm.org/D102665