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

[Tim Northover via Phabricator via llvm-commits]

t.p.northover created this revision.
Herald added subscribers: hiraditya, mcrosier.
Herald added a reviewer: jhenderson.
Herald added a project: All.
t.p.northover requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: LLVM.

Conventionally, `--` on its own tells a command to stop parsing further options and treat all future arguments as positional inputs. Used to act on pesky files starting with `-`, and possibly other things.

I wasn't entirely sure it should apply to all tools, but it seems like a reasonable default position to take. And the `--` would have been a hard error (unknown option) for all existing backends anyway so I don't think it can break anyone.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150568

Files:
  llvm/lib/Option/OptTable.cpp
  llvm/test/tools/llvm-nm/option--.test


Index: llvm/test/tools/llvm-nm/option--.test
===================================================================
--- /dev/null
+++ llvm/test/tools/llvm-nm/option--.test
@@ -0,0 +1,10 @@
+; 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
+
+; RUN: llvm-as < %s > %p/-.bc
+; RUN: llvm-nm -- %p/-.bc | FileCheck %s
+
+; CHECK: foo
+define void @foo() {
+  ret void
+}
Index: llvm/lib/Option/OptTable.cpp
===================================================================
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -455,6 +455,7 @@
 
   MissingArgIndex = MissingArgCount = 0;
   unsigned Index = 0, End = ArgArr.size();
+  bool SeenAllOptions = false;
   while (Index < End) {
     // Ingore nullptrs, they are response file's EOL markers
     if (Args.getArgString(Index) == nullptr) {
@@ -468,10 +469,26 @@
       continue;
     }
 
+    // "--" on its own marks the end of option parsing, anything afterwards
+    // should be treated as a positional input.
+    if (Str == "--") {
+      SeenAllOptions = true;
+      ++Index;
+      continue;
+    }
+
     unsigned Prev = Index;
-    std::unique_ptr<Arg> A = GroupedShortOptions
-                 ? parseOneArgGrouped(Args, Index)
-                 : ParseOneArg(Args, Index, FlagsToInclude, FlagsToExclude);
+    std::unique_ptr<Arg> A;
+    if (SeenAllOptions) {
+      A = std::make_unique<Arg>(getOption(InputOptionID), Str, Index,
+                                Args.getArgString(Index));
+      ++Index;
+    } else {
+      A = GroupedShortOptions
+              ? parseOneArgGrouped(Args, Index)
+              : ParseOneArg(Args, Index, FlagsToInclude, FlagsToExclude);
+    }
+
     assert((Index > Prev || GroupedShortOptions) &&
            "Parser failed to consume argument.");
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150568.522156.patch
Type: text/x-patch
Size: 1914 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230515/22e614fa/attachment.bin>