[llvm] 926e51c - [Option] Support special argument "--"

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 7 08:06:11 PDT 2023


Author: Fangrui Song
Date: 2023-06-07T08:06:05-07:00
New Revision: 926e51c1370c711946e4b04101008773ea9d2052

URL: https://github.com/llvm/llvm-project/commit/926e51c1370c711946e4b04101008773ea9d2052
DIFF: https://github.com/llvm/llvm-project/commit/926e51c1370c711946e4b04101008773ea9d2052.diff

LOG: [Option] Support special argument "--"

Many command line option implementations, including getopt_long and our
Support/CommandLine.cpp, support `--` as an end-of-option indicator. All
the subsequent arguments are then treated as positional arguments.

D1387 added KIND_REMAINING_ARGS and 76ff1d915c9c42823a3f2b08ff936cf7a48933c5 dropped special handling of `--`.
Users need to add `def DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>;` and
append `OPT_DASH_DASH` to the `OPT_INPUT` list., which is not ergonomic.

Restore this feature under an option and modify llvm-strings to utilize the
feature as an example. In the future, we probably should enable this feature by
default and exclude some tools that handle `DASH_DASH` differently (clang,
clang-scan-deps, etc. I suspect that many are workarounds for LLVMOption not
supporting `--` as a built-in feature).

Reviewed By: serge-sans-paille

Differential Revision: https://reviews.llvm.org/D152286

Added: 
    llvm/test/tools/llvm-strings/dash-filename.test

Modified: 
    llvm/include/llvm/Option/OptTable.h
    llvm/lib/Option/OptTable.cpp
    llvm/tools/llvm-strings/llvm-strings.cpp
    llvm/unittests/Option/OptionParsingTest.cpp
    llvm/unittests/Option/Opts.td

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h
index 0cef9b65d2f23..6f3d6032e59ac 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -62,6 +62,7 @@ class OptTable {
   ArrayRef<Info> OptionInfos;
   bool IgnoreCase;
   bool GroupedShortOptions = false;
+  bool DashDashParsing = false;
   const char *EnvVar = nullptr;
 
   unsigned InputOptionID = 0;
@@ -139,6 +140,10 @@ class OptTable {
   /// Support grouped short options. e.g. -ab represents -a -b.
   void setGroupedShortOptions(bool Value) { GroupedShortOptions = Value; }
 
+  /// Set whether "--" stops option parsing and treats all subsequent arguments
+  /// as positional. E.g. -- -a -b gives two positional inputs.
+  void setDashDashParsing(bool Value) { DashDashParsing = Value; }
+
   /// Find possible value for given flags. This is used for shell
   /// autocompletion.
   ///

diff  --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index 5848191fe3936..3f53ac119c69e 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -468,6 +468,16 @@ InputArgList OptTable::ParseArgs(ArrayRef<const char *> ArgArr,
       continue;
     }
 
+    // In DashDashParsing mode, the first "--" stops option scanning and treats
+    // all subsequent arguments as positional.
+    if (DashDashParsing && Str == "--") {
+      while (++Index < End) {
+        Args.append(new Arg(getOption(InputOptionID), Str, Index,
+                            Args.getArgString(Index)));
+      }
+      break;
+    }
+
     unsigned Prev = Index;
     std::unique_ptr<Arg> A = GroupedShortOptions
                  ? parseOneArgGrouped(Args, Index)

diff  --git a/llvm/test/tools/llvm-strings/dash-filename.test b/llvm/test/tools/llvm-strings/dash-filename.test
new file mode 100644
index 0000000000000..0f325fabbef96
--- /dev/null
+++ b/llvm/test/tools/llvm-strings/dash-filename.test
@@ -0,0 +1,6 @@
+## Show that -- stops option scanning.
+
+RUN: rm -rf %t && mkdir %t && cd %t
+RUN: echo abcd > -a
+RUN: llvm-strings -f -- -a | FileCheck %s
+CHECK: -a: abcd

diff  --git a/llvm/tools/llvm-strings/llvm-strings.cpp b/llvm/tools/llvm-strings/llvm-strings.cpp
index f6d08a1988b7d..449a833334cd1 100644
--- a/llvm/tools/llvm-strings/llvm-strings.cpp
+++ b/llvm/tools/llvm-strings/llvm-strings.cpp
@@ -62,6 +62,7 @@ class StringsOptTable : public opt::GenericOptTable {
 public:
   StringsOptTable() : GenericOptTable(InfoTable) {
     setGroupedShortOptions(true);
+    setDashDashParsing(true);
   }
 };
 } // namespace

diff  --git a/llvm/unittests/Option/OptionParsingTest.cpp b/llvm/unittests/Option/OptionParsingTest.cpp
index 9d1b46a87494c..6468872f8118d 100644
--- a/llvm/unittests/Option/OptionParsingTest.cpp
+++ b/llvm/unittests/Option/OptionParsingTest.cpp
@@ -391,6 +391,39 @@ TYPED_TEST(OptTableTest, ParseGroupedShortOptions) {
   EXPECT_TRUE(AL3.hasArg(OPT_Blorp));
 }
 
+TYPED_TEST(OptTableTest, ParseDashDash) {
+  TypeParam T;
+  T.setDashDashParsing(true);
+  unsigned MAI, MAC;
+
+  const char *Args1[] = {"-A", "--"};
+  InputArgList AL = T.ParseArgs(Args1, MAI, MAC);
+  EXPECT_TRUE(AL.hasArg(OPT_A));
+  EXPECT_EQ(size_t(0), AL.getAllArgValues(OPT_INPUT).size());
+  EXPECT_EQ(size_t(0), AL.getAllArgValues(OPT_UNKNOWN).size());
+
+  const char *Args2[] = {"-A", "--", "-A", "--", "-B"};
+  AL = T.ParseArgs(Args2, MAI, MAC);
+  EXPECT_TRUE(AL.hasArg(OPT_A));
+  EXPECT_FALSE(AL.hasArg(OPT_B));
+  const std::vector<std::string> Input = AL.getAllArgValues(OPT_INPUT);
+  ASSERT_EQ(size_t(3), Input.size());
+  EXPECT_EQ("-A", Input[0]);
+  EXPECT_EQ("--", Input[1]);
+  EXPECT_EQ("-B", Input[2]);
+  EXPECT_EQ(size_t(0), AL.getAllArgValues(OPT_UNKNOWN).size());
+
+  T.setDashDashParsing(false);
+  AL = T.ParseArgs(Args2, MAI, MAC);
+  EXPECT_TRUE(AL.hasArg(OPT_A));
+  EXPECT_TRUE(AL.hasArg(OPT_B));
+  EXPECT_EQ(size_t(0), AL.getAllArgValues(OPT_INPUT).size());
+  const std::vector<std::string> Unknown = AL.getAllArgValues(OPT_UNKNOWN);
+  ASSERT_EQ(size_t(2), Unknown.size());
+  EXPECT_EQ("--", Unknown[0]);
+  EXPECT_EQ("--", Unknown[1]);
+}
+
 TYPED_TEST(OptTableTest, UnknownOptions) {
   TypeParam T;
   unsigned MAI, MAC;

diff  --git a/llvm/unittests/Option/Opts.td b/llvm/unittests/Option/Opts.td
index 028ea366cbc87..cdfc614e4621c 100644
--- a/llvm/unittests/Option/Opts.td
+++ b/llvm/unittests/Option/Opts.td
@@ -43,8 +43,6 @@ def Glorrmp_eq : Flag<["--"], "glorrmp=">;
 def Blurmpq : Flag<["--"], "blurmp">;
 def Blurmpq_eq : Flag<["--"], "blurmp=">;
 
-def DashDash : Option<["--"], "", KIND_REMAINING_ARGS>;
-
 class XOpts<string base> : KeyPathAndMacro<"X->", base> {}
 
 def marshalled_flag_d : Flag<["-"], "marshalled-flag-d">,


        


More information about the llvm-commits mailing list