[PATCH] D150568: OptTable: stop parsing options after "--" alone.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

The patch looks good to me. It's a shame that we have to explicitly teach a tool to use `OPT_DASH_DASH` instead of making this a built-in feature of LLVMOption and making the feature opt-out.
That said, accepting `--` in llvm-nm is still a good change.

@jhenderson usually has strong opinions on option handling and should sign off on this patch.



================
Comment at: llvm/test/tools/llvm-nm/option--.test:4
+
+; RUN: mkdir -p %t && cd %t
+; RUN: llvm-as < %s > -.bc
----------------
to prevent issues when `%t` already exists as a file or exists as a directory with existing files potentially intervening with this test execution.


================
Comment at: llvm/test/tools/llvm-nm/option--.test:1
+; RUN: not llvm-nm -- -weird-filename-that-doesnt-exist 2>&1 | FileCheck %s --check-prefix=CHECK-NOTOPT
+; CHECK-NOTOPT: error: -weird-filename-that-doesnt-exist: No such file or directory
----------------
MaskRay wrote:
> jhenderson wrote:
> > I don't think a test in tools/llvm-nm is the right place for this sort of behaviour, as it isn't code that is specific to llvm-nm. Can it be tested using the gtest unit test system? If not, then the test still would belong in llvm/test/Option or something like that.
> The test can be added to `llvm/unittests/Option/OptionParsingTest.cpp`
See `llvm/docs/TestingGuide.rst:692` `%errc_ENOENT`


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

https://reviews.llvm.org/D150568