[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358428: [CommandLineParser] Add DefaultOption flag (authored 
by dhinton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59746?vs=195218=195220#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59746

Files:
  cfe/trunk/lib/Tooling/CommonOptionsParser.cpp
  llvm/trunk/docs/CommandLine.rst
  llvm/trunk/include/llvm/Support/CommandLine.h
  llvm/trunk/lib/Support/CommandLine.cpp
  llvm/trunk/test/Support/check-default-options.txt
  llvm/trunk/tools/llvm-opt-report/OptReport.cpp
  llvm/trunk/unittests/Support/CommandLineTest.cpp

Index: llvm/trunk/lib/Support/CommandLine.cpp
===
--- llvm/trunk/lib/Support/CommandLine.cpp
+++ llvm/trunk/lib/Support/CommandLine.cpp
@@ -98,6 +98,11 @@
   // This collects additional help to be printed.
   std::vector MoreHelp;
 
+  // This collects Options added with the cl::DefaultOption flag. Since they can
+  // be overridden, they are not added to the appropriate SubCommands until
+  // ParseCommandLineOptions actually runs.
+  SmallVector DefaultOptions;
+
   // This collects the different option categories that have been registered.
   SmallPtrSet RegisteredOptionCategories;
 
@@ -146,6 +151,11 @@
   void addOption(Option *O, SubCommand *SC) {
 bool HadErrors = false;
 if (O->hasArgStr()) {
+  // If it's a DefaultOption, check to make sure it isn't already there.
+  if (O->isDefaultOption() &&
+  SC->OptionsMap.find(O->ArgStr) != SC->OptionsMap.end())
+return;
+
   // Add argument to the argument map!
   if (!SC->OptionsMap.insert(std::make_pair(O->ArgStr, O)).second) {
 errs() << ProgramName << ": CommandLine Error: Option '" << O->ArgStr
@@ -185,7 +195,12 @@
 }
   }
 
-  void addOption(Option *O) {
+  void addOption(Option *O, bool ProcessDefaultOption = false) {
+if (!ProcessDefaultOption && O->isDefaultOption()) {
+  DefaultOptions.push_back(O);
+  return;
+}
+
 if (O->Subs.empty()) {
   addOption(O, &*TopLevelSubCommand);
 } else {
@@ -201,8 +216,12 @@
   OptionNames.push_back(O->ArgStr);
 
 SubCommand  = *SC;
-for (auto Name : OptionNames)
-  Sub.OptionsMap.erase(Name);
+auto End = Sub.OptionsMap.end();
+for (auto Name : OptionNames) {
+  auto I = Sub.OptionsMap.find(Name);
+  if (I != End && I->getValue() == O)
+Sub.OptionsMap.erase(I);
+  }
 
 if (O->getFormattingFlag() == cl::Positional)
   for (auto Opt = Sub.PositionalOpts.begin();
@@ -266,8 +285,13 @@
 if (O->Subs.empty())
   updateArgStr(O, NewName, &*TopLevelSubCommand);
 else {
-  for (auto SC : O->Subs)
-updateArgStr(O, NewName, SC);
+  if (O->isInAllSubCommands()) {
+for (auto SC : RegisteredSubCommands)
+  updateArgStr(O, NewName, SC);
+  } else {
+for (auto SC : O->Subs)
+  updateArgStr(O, NewName, SC);
+  }
 }
   }
 
@@ -331,6 +355,8 @@
 AllSubCommands->reset();
 registerSubCommand(&*TopLevelSubCommand);
 registerSubCommand(&*AllSubCommands);
+
+DefaultOptions.clear();
   }
 
 private:
@@ -366,6 +392,13 @@
   ArgStr = S;
 }
 
+void Option::reset() {
+  NumOccurrences = 0;
+  setDefault();
+  if (isDefaultOption())
+removeArgument();
+}
+
 // Initialise the general option category.
 OptionCategory llvm::cl::GeneralCategory("General options");
 
@@ -1167,6 +1200,10 @@
   auto  = ChosenSubCommand->SinkOpts;
   auto  = ChosenSubCommand->OptionsMap;
 
+  for (auto O: DefaultOptions) {
+addOption(O, true);
+  }
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2183,9 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::DefaultOption);
+
 static cl::opt>
 HHOp("help-hidden", cl::desc("Display all available options"),
  cl::location(WrappedHiddenPrinter), cl::Hidden, cl::ValueDisallowed,
Index: llvm/trunk/unittests/Support/CommandLineTest.cpp
===
--- llvm/trunk/unittests/Support/CommandLineTest.cpp
+++ llvm/trunk/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,68 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195218.
hintonda added a comment.

- Add new test back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,68 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+  cl::ResetCommandLineParser();
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-DWARF:   -h  - Alias for -help

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 195217.
hintonda added a comment.

- Original patch, r358337, that was reverted.
- Make sure to clear DefaultOptions on reset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,68 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+  cl::ResetCommandLineParser();
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -98,6 +98,11 @@
   // This collects additional help to be printed.
   std::vector MoreHelp;
 
+  // This collects Options added with the cl::DefaultOption flag. Since they can
+  // be overridden, they are not added to the appropriate SubCommands until
+  // ParseCommandLineOptions actually runs.
+  SmallVector DefaultOptions;
+
   // This collects the different option categories that have been registered.
   SmallPtrSet RegisteredOptionCategories;
 
@@ -146,6 +151,11 @@
   void addOption(Option *O, SubCommand *SC) {
 bool HadErrors = false;
 if (O->hasArgStr()) {
+  // If it's a DefaultOption, check to make sure it isn't already there.
+  if (O->isDefaultOption() &&

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda reopened this revision.
hintonda added a comment.
This revision is now accepted and ready to land.

Reopen to track fix after buildbot failure and revert -- r358414.

http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190415/644037.html


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358337: [CommandLineParser] Add DefaultOption flag (authored 
by dhinton, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D59746?vs=194899=195020#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59746

Files:
  cfe/trunk/lib/Tooling/CommonOptionsParser.cpp
  llvm/trunk/docs/CommandLine.rst
  llvm/trunk/include/llvm/Support/CommandLine.h
  llvm/trunk/lib/Support/CommandLine.cpp
  llvm/trunk/test/Support/check-default-options.txt
  llvm/trunk/tools/llvm-opt-report/OptReport.cpp
  llvm/trunk/unittests/Support/CommandLineTest.cpp

Index: llvm/trunk/docs/CommandLine.rst
===
--- llvm/trunk/docs/CommandLine.rst
+++ llvm/trunk/docs/CommandLine.rst
@@ -128,6 +128,7 @@
   USAGE: compiler [options]
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -194,6 +195,7 @@
   USAGE: compiler [options] 
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -1251,6 +1253,14 @@
   with ``cl::CommaSeparated``, this modifier only makes sense with a `cl::list`_
   option.
 
+.. _cl::DefaultOption:
+
+* The **cl::DefaultOption** modifier is used to specify that the option is a
+  default that can be overridden by application specific parsers. For example,
+  the ``-help`` alias, ``-h``, is registered this way, so it can be overridden
+  by applications that need to use the ``-h`` option for another purpose,
+  either as a regular option or an alias for another option.
+
 .. _response files:
 
 Response files
Index: llvm/trunk/include/llvm/Support/CommandLine.h
===
--- llvm/trunk/include/llvm/Support/CommandLine.h
+++ llvm/trunk/include/llvm/Support/CommandLine.h
@@ -175,7 +175,10 @@
   // If this is enabled, multiple letter options are allowed to bunch together
   // with only a single hyphen for the whole group.  This allows emulation
   // of the behavior that ls uses for example: ls -la === ls -l -a
-  Grouping = 0x08
+  Grouping = 0x08,
+
+  // Default option
+  DefaultOption = 0x10
 };
 
 //===--===//
@@ -270,7 +273,7 @@
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
-  unsigned Misc : 4;
+  unsigned Misc : 5;
   unsigned Position = 0;   // Position of last occurrence of the option
   unsigned AdditionalVals = 0; // Greater than 0 for multi-valued option.
 
@@ -306,6 +309,7 @@
   bool hasArgStr() const { return !ArgStr.empty(); }
   bool isPositional() const { return getFormattingFlag() == cl::Positional; }
   bool isSink() const { return getMiscFlags() & cl::Sink; }
+  bool isDefaultOption() const { return getMiscFlags() & cl::DefaultOption; }
 
   bool isConsumeAfter() const {
 return getNumOccurrencesFlag() == cl::ConsumeAfter;
@@ -382,7 +386,7 @@
   }
 
   inline int getNumOccurrences() const { return NumOccurrences; }
-  inline void reset() { NumOccurrences = 0; }
+  void reset();
 };
 
 //===--===//
@@ -1732,7 +1736,10 @@
   error("cl::alias must have argument name specified!");
 if (!AliasFor)
   error("cl::alias must have an cl::aliasopt(option) specified!");
+if (!Subs.empty())
+  error("cl::alias must not have cl::sub(), aliased option's cl::sub() will be used!");
 Subs = AliasFor->Subs;
+Category = AliasFor->Category;
 addArgument();
   }
 
Index: llvm/trunk/test/Support/check-default-options.txt
===
--- llvm/trunk/test/Support/check-default-options.txt
+++ llvm/trunk/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-DWARF:   -h  - Alias for -help
+
+# llvm-dwarfdump declares `-h` option and prints special help in that case,
+# which is weird, but makes for 

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1465445 , @klimek wrote:

> lg


Thanks again for the help and the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Manuel Klimek via Phabricator via cfe-commits]

klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194899.
hintonda added a comment.

- Fix comments and add isDefaultOption per review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,67 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-DWARF:   -h  - Alias for 

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:101
 
+  SmallVector DefaultOptions;
+

A comment explaining what this contains and how it'll be used would help.



Comment at: llvm/lib/Support/CommandLine.cpp:152
+  // If it's a DefaultOption, check to make sure it isn't already there.
+  if (O->getMiscFlags() & cl::DefaultOption &&
+  SC->OptionsMap.find(O->ArgStr) != SC->OptionsMap.end())

This is used multiple times, add isDefault() to Option?



Comment at: llvm/lib/Support/CommandLine.cpp:196
+  void addOption(Option *O, bool ProcessDefaultOption = false) {
+// Handle DefaultOptions
+if (!ProcessDefaultOption && O->getMiscFlags() & cl::DefaultOption) {

I'd delete this comment, I don't think it helps.



Comment at: llvm/lib/Support/CommandLine.cpp:1199
 
+  // Handle DefaultOptions.
+  for (auto O: DefaultOptions) {

Same here, this comment doesn't say anything non-obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

@klimek, this patch is now ready for review.  Thanks...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194729.
hintonda added a comment.

Enhance Option::reset to reset the default value and remove the option
if it's a cl::DefaultOption.

Also, make sure removeOption only removes an option if both the name
and the Option matches, not just the name -- different Options with
the same name in different SubCommands have always been possible, but
are more common with DefaultOptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,67 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# 

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194708.
hintonda added a comment.

- Add unittest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,74 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  Bar = "";
+  Foo = false;
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  Bar = "";
+  Foo = false;
+  SC1_B = false;
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), ::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for 

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1462352 , @jhenderson wrote:

> I've not looked at the implementation in depth, but cl::DefaultOption seems 
> like a nice way of handling this. Please make sure that there is testing in 
> llvm-readobj and llvm-objdump that test their own short alias interpretation 
> somewhere though.


While I'm still working on unittests, I have added a few tests that should 
cover this.  Please let me know if you have any concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda marked 2 inline comments as done.
hintonda added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:282
 else {
-  for (auto SC : O->Subs)
-updateArgStr(O, NewName, SC);
+  if (O->isInAllSubCommands()) {
+for (auto SC : RegisteredSubCommands)

Will move this bug fix to its own patch and add test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194669.
hintonda added a comment.

- Delay actually adding default options until ParseCommandLineOptions which 
simplifies handling and makes it easy to only add them to the correct 
SubCommand.

  Add simple tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp

Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-DWARF:   -h  - Alias for -help
+
+# llvm-dwarfdump declares `-h` option and prints special help in that case,
+# which is weird, but makes for a good test, i.e., shows the default `-h`
+# wasn't used.
+# CHECK-DWARF-H-NOT: -help-list  - Display list of available options (-help-list-hidden for more)
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -98,6 +98,8 @@
   // This collects additional help to be printed.
   std::vector MoreHelp;
 
+  SmallVector DefaultOptions;
+
   // This collects the different option categories that have been registered.
   SmallPtrSet RegisteredOptionCategories;
 
@@ -146,6 +148,11 @@
   void addOption(Option *O, SubCommand *SC) {
 bool HadErrors = false;
 if (O->hasArgStr()) {
+  // If it's a DefaultOption, check to make sure it isn't already there.
+  if (O->getMiscFlags() & cl::DefaultOption &&
+  SC->OptionsMap.find(O->ArgStr) != SC->OptionsMap.end())
+return;
+
   // Add argument to the argument map!
   if (!SC->OptionsMap.insert(std::make_pair(O->ArgStr, O)).second) {
 errs() << ProgramName << ": CommandLine Error: Option '" << O->ArgStr
@@ -185,7 +192,13 @@
 }
   }
 
-  void addOption(Option *O) {
+  void addOption(Option *O, bool ProcessDefaultOption = false) {
+// Handle DefaultOptions
+if (!ProcessDefaultOption && O->getMiscFlags() & cl::DefaultOption) {
+  DefaultOptions.push_back(O);
+  return;
+}
+
 if (O->Subs.empty()) {
   addOption(O, &*TopLevelSubCommand);
 } else {
@@ -266,8 +279,13 @@
 if (O->Subs.empty())
   updateArgStr(O, NewName, &*TopLevelSubCommand);
 else {
-  for (auto SC : O->Subs)
-updateArgStr(O, NewName, SC);
+  if (O->isInAllSubCommands()) {
+for (auto SC : RegisteredSubCommands)
+  updateArgStr(O, NewName, SC);
+  } else {
+for (auto SC : O->Subs)
+  updateArgStr(O, NewName, SC);
+  }
 }
   }
 
@@ -1167,6 +1185,11 @@
   auto  = ChosenSubCommand->SinkOpts;
   auto  = ChosenSubCommand->OptionsMap;
 
+  // Handle DefaultOptions.
+  for (auto O: DefaultOptions) {
+addOption(O, true);
+  }
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2169,9 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::DefaultOption);
+
 static cl::opt>
 

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

I've not looked at the implementation in depth, but cl::DefaultOption seems 
like a nice way of handling this. Please make sure that there is testing in 
llvm-readobj and llvm-objdump that test their own short alias interpretation 
somewhere though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194633.
hintonda edited the summary of this revision.
hintonda added a comment.

- Fix bug in updateArgStr() where it didn't handle isInAllSubCommands() 
correctly, and refactored code based on how SubCommands work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -98,6 +98,8 @@
   // This collects additional help to be printed.
   std::vector MoreHelp;
 
+  SmallVector, 4> DefaultOptions;
+
   // This collects the different option categories that have been registered.
   SmallPtrSet RegisteredOptionCategories;
 
@@ -111,6 +113,10 @@
 
   void ResetAllOptionOccurrences();
 
+  bool tryRemoveDefaultOption(Option *O, SubCommand *SC, StringRef NewArg);
+
+  void handleDefaultOptions();
+
   bool ParseCommandLineOptions(int argc, const char *const *argv,
StringRef Overview, raw_ostream *Errs = nullptr);
 
@@ -186,6 +192,13 @@
   }
 
   void addOption(Option *O) {
+// Handle DefaultOptions
+if (O->getMiscFlags() & cl::DefaultOption) {
+  std::string DefaultArg(O->ArgStr + ":default_option");
+  DefaultOptions.push_back(std::make_pair(DefaultArg, O));
+  O->setArgStr(DefaultOptions.back().first);
+}
+
 if (O->Subs.empty()) {
   addOption(O, &*TopLevelSubCommand);
 } else {
@@ -266,8 +279,13 @@
 if (O->Subs.empty())
   updateArgStr(O, NewName, &*TopLevelSubCommand);
 else {
-  for (auto SC : O->Subs)
-updateArgStr(O, NewName, SC);
+  if (O->isInAllSubCommands()) {
+for (auto SC : RegisteredSubCommands)
+  updateArgStr(O, NewName, SC);
+  } else {
+for (auto SC : O->Subs)
+  updateArgStr(O, NewName, SC);
+  }
 }
   }
 
@@ -1118,6 +1136,41 @@
   }
 }
 
+bool CommandLineParser::tryRemoveDefaultOption(Option *O, SubCommand *SC,
+   StringRef NewArg) {
+  StringRef Val;
+  if (LookupOption(*SC, NewArg, Val)) {
+O->removeArgument();
+return true;
+  }
+  return false;
+}
+
+void CommandLineParser::handleDefaultOptions() {
+  for (auto Pair : DefaultOptions) {
+StringRef Arg(Pair.first);
+Option *O = Pair.second;
+StringRef NewArg = Arg.substr(0, Arg.find(":"));
+
+bool Removed = false;
+if (O->isInAllSubCommands()) {
+  for (auto SC : RegisteredSubCommands) {
+if (SC == &*AllSubCommands)
+  continue;
+if ((Removed = tryRemoveDefaultOption(O, SC, NewArg)))
+  break;
+  }
+} else {
+  for (auto SC : O->Subs) {
+if ((Removed = tryRemoveDefaultOption(O, SC, NewArg)))
+  break;
+  }
+}
+if (!Removed)
+  O->setArgStr(NewArg);
+  }
+}
+
 bool CommandLineParser::ParseCommandLineOptions(int argc,
 const char *const *argv,
 StringRef Overview,
@@ -1167,6 +1220,8 @@
   auto  = ChosenSubCommand->SinkOpts;
   auto  = ChosenSubCommand->OptionsMap;
 
+  handleDefaultOptions();
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2201,9 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::DefaultOption);
+
 static cl::opt>
 HHOp("help-hidden", cl::desc("Display all available options"),
  cl::location(WrappedHiddenPrinter), cl::Hidden, cl::ValueDisallowed,
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -175,7 +175,10 @@
   // If this is enabled, multiple letter options are allowed to bunch together
   // with only a single hyphen for the whole group.  This allows emulation
   // of the behavior that ls uses for example: ls -la === ls -l -a
-  Grouping = 0x08
+  Grouping = 0x08,
+
+  // Default option
+  DefaultOption = 0x10
 };
 
 //===--===//
@@ -270,12 +273,12 @@
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
-  unsigned Misc : 4;
+  unsigned Misc : 5;
   unsigned Position = 0;   // Position of last occurrence of the option
   unsigned AdditionalVals = 0; // Greater than 0 for multi-valued