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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 18 16:01:51 PDT 2023


MaskRay added a comment.

This behavior makes sense to me. This is `llvm/lib/SupportCommandLine.cpp` and GNU `getopt_long`'s behavior, used by a lot of utilities, well beyond GNU. (The convention might be by others?)

POSIX considers `--` special as well.

  argv[optind]   points to the string "--"
  
  getopt() shall return -1 after incrementing optind.

`clang/include/clang/Driver/Options.td` currently defines `def _DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>,` and this change will change the behavior and cause some interesting interop issues (I don't investigate closely) with D98824 <https://reviews.llvm.org/D98824>.



================
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
----------------
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`


================
Comment at: llvm/test/tools/llvm-nm/option--.test:2
+; 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
+
----------------
The message is OS dependent. We can use `%errc_ENOENT` (see llvm/docs/TestingGuide.rst).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150568



More information about the llvm-commits mailing list