[CommandLine] Add long option flag for cl::ParseCommandLineOptions . Part 5 of 5

[hintonda]

Summary

If passed, the long option flag makes the CommandLine parser
mimic the behavior or GNU getopt_long. Short options are a single
character prefixed by a single dash, and long options are multiple
characters prefixed by a double dash.

This patch requires D61269 and D61270, and is motivated by the
discussion in the following thread:
http://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html

[MaskRay]

llvm/test/tools/llvm-readobj/merged.test

13

Will you fix the message -section-headers?

[hintonda]

Rebase

[hintonda]

Rebase to minimize diff.

[hintonda]

llvm/test/tools/llvm-readobj/merged.test

13

Fixed in D61270, which this patch includes.

[hintonda]

• Add LookupLongOption and simplify code.

[hintonda]

ping...

[MaskRay]

llvm/lib/Support/CommandLine.cpp

91

Just move the definition here?

373

return nullptr;

[hintonda]

llvm/lib/Support/CommandLine.cpp

91

Was trying to minimize the diff as much as possible, but there's no reason not to move these two here.

I could also move the others, but then it would be more difficult to see what I changed.

[hintonda]

• Address comments.

llvm/include/llvm/Support/CommandLine.h

 // from argv.  This precedence is currently implemented by parsing argv after
 // the environment variable, so it is only implemented correctly for options
 // that give precedence to later occurrences.  If your program supports options
 // that give precedence to earlier occurrences, you will need to extend this
 // function to support it correctly.
 bool ParseCommandLineOptions(int argc, const char *const *argv,
                              StringRef Overview = "",
                              raw_ostream *Errs = nullptr,
-                             const char *EnvVar = nullptr);
+                             const char *EnvVar = nullptr,
+                             bool LongOptionsUseDoubleDash = false);
 
 //===----------------------------------------------------------------------===//
 // ParseEnvironmentOptions - Environment variable option processing alternate
 //                           entry point.
 //
 void ParseEnvironmentOptions(const char *progName, const char *envvar,
                              const char *Overview = "");
 
template <> struct applicator<OptionHidden> {
   static void opt(OptionHidden OH, Option &O) { O.setHiddenFlag(OH); }
 };
 
 template <> struct applicator<FormattingFlags> {
   static void opt(FormattingFlags FF, Option &O) { O.setFormattingFlag(FF); }
 };
 
 template <> struct applicator<MiscFlags> {
-  static void opt(MiscFlags MF, Option &O) { O.setMiscFlag(MF); }
+  static void opt(MiscFlags MF, Option &O) {
+    assert((MF != Grouping || O.ArgStr.size() == 1) &&
+           "cl::Grouping can only apply to single charater Options.");
+    O.setMiscFlag(MF);
+  }
 };
 
 // apply method - Apply modifiers to an option in a type safe way.
 template <class Opt, class Mod, class... Mods>
 void apply(Opt *O, const Mod &M, const Mods &... Ms) {
   applicator<Mod>::opt(M, *O);
   apply(O, Ms...);
 }

llvm/lib/Support/CommandLine.cpp

 void parser<unsigned long long>::anchor() {}
 void parser<double>::anchor() {}
 void parser<float>::anchor() {}
 void parser<std::string>::anchor() {}
 void parser<char>::anchor() {}
 
 //===----------------------------------------------------------------------===//
 
 namespace {
 
 class CommandLineParser {
 public:
   // Globals for name and overview of program.  Program name is not a string to
   // avoid static ctor/dtor issues.
   std::string ProgramName;
   StringRef ProgramOverview;
 
public:
   CommandLineParser() : ActiveSubCommand(nullptr) {
     registerSubCommand(&*TopLevelSubCommand);
     registerSubCommand(&*AllSubCommands);
   }
 
   void ResetAllOptionOccurrences();
 
   bool ParseCommandLineOptions(int argc, const char *const *argv,
-                               StringRef Overview, raw_ostream *Errs = nullptr);
+                               StringRef Overview, raw_ostream *Errs = nullptr,
+                               bool LongOptionsUseDoubleDash = false);
 
   void addLiteralOption(Option &Opt, SubCommand *SC, StringRef Name) {
     if (Opt.hasArgStr())
       return;
     if (!SC->OptionsMap.insert(std::make_pair(Name, &Opt)).second) {
       errs() << ProgramName << ": CommandLine Error: Option '" << Name
              << "' registered more than once!\n";
       report_fatal_error("inconsistency in registered CommandLine options");

 private:
   SubCommand *ActiveSubCommand;
 
   Option *LookupOption(SubCommand &Sub, StringRef &Arg, StringRef &Value);
   SubCommand *LookupSubCommand(StringRef Name);
 };
 
 } // namespace
 
 static ManagedStatic<CommandLineParser> GlobalParser;
 
 void cl::AddLiteralOption(Option &O, StringRef Name) {
   GlobalParser->addLiteralOption(O, Name);
 }
 
 extrahelp::extrahelp(StringRef Help) : morehelp(Help) {
   GlobalParser->MoreHelp.push_back(Help);
 }
 
 void Option::addArgument() {
   GlobalParser->addOption(this);
   FullyInitialized = true;
 }
 
 void Option::removeArgument() { GlobalParser->removeOption(this); }
 
 void Option::setArgStr(StringRef S) {
   if (FullyInitialized)
     GlobalParser->updateArgStr(this, S);
   assert((S.empty() || S[0] != '-') && "Option can't start with '-");
   ArgStr = S;
+  if (ArgStr.size() == 1)
+    setMiscFlag(Grouping);
 }
 
 void Option::reset() {
   NumOccurrences = 0;
   setDefault();
   if (isDefaultOption())
     removeArgument();
 }

 // specified predicate with names that are the prefixes in Name.  This is
 // checked by progressively stripping characters off of the name, checking to
 // see if there options that satisfy the predicate.  If we find one, return it,
 // otherwise return null.
 //
 static Option *getOptionPred(StringRef Name, size_t &Length,
                              bool (*Pred)(const Option *),
                              const StringMap<Option *> &OptionsMap) {
-
   StringMap<Option *>::const_iterator OMI = OptionsMap.find(Name);
+  if (OMI != OptionsMap.end() && !Pred(OMI->getValue()))
+    OMI = OptionsMap.end();
 
   // Loop while we haven't found an option and Name still has at least two
   // characters in it (so that the next iteration will not be the empty
   // string.
   while (OMI == OptionsMap.end() && Name.size() > 1) {
     Name = Name.substr(0, Name.size() - 1); // Chop off the last character.
     OMI = OptionsMap.find(Name);
+    if (OMI != OptionsMap.end() && !Pred(OMI->getValue()))
+      OMI = OptionsMap.end();
   }
 
   if (OMI != OptionsMap.end() && Pred(OMI->second)) {
     Length = Name.size();
     return OMI->second; // Found one!
   }
   return nullptr; // No option found!
 }
void cl::ParseEnvironmentOptions(const char *progName, const char *envVar,
   // and hand it off to ParseCommandLineOptions().
   TokenizeGNUCommandLine(*envValue, Saver, newArgv);
   int newArgc = static_cast<int>(newArgv.size());
   ParseCommandLineOptions(newArgc, &newArgv[0], StringRef(Overview));
 }
 
 bool cl::ParseCommandLineOptions(int argc, const char *const *argv,
                                  StringRef Overview, raw_ostream *Errs,
-                                 const char *EnvVar) {
+                                 const char *EnvVar,
+                                 bool LongOptionsUseDoubleDash) {
   SmallVector<const char *, 20> NewArgv;
   BumpPtrAllocator A;
   StringSaver Saver(A);
   NewArgv.push_back(argv[0]);
 
   // Parse options from environment variable.
   if (EnvVar) {
     if (llvm::Optional<std::string> EnvValue =
             sys::Process::GetEnv(StringRef(EnvVar)))
       TokenizeGNUCommandLine(*EnvValue, Saver, NewArgv);
   }
 
   // Append options from command line.
   for (int I = 1; I < argc; ++I)
     NewArgv.push_back(argv[I]);
   int NewArgc = static_cast<int>(NewArgv.size());
 
   // Parse all options.
   return GlobalParser->ParseCommandLineOptions(NewArgc, &NewArgv[0], Overview,
-                                               Errs);
+                                               Errs, LongOptionsUseDoubleDash);
 }
 
 void CommandLineParser::ResetAllOptionOccurrences() {
   // So that we can parse different command lines multiple times in succession
   // we reset all option values to look like they have never been seen before.
   for (auto SC : RegisteredSubCommands) {
     for (auto &O : SC->OptionsMap)
       O.second->reset();
   }
 }
 
 bool CommandLineParser::ParseCommandLineOptions(int argc,
                                                 const char *const *argv,
                                                 StringRef Overview,
-                                                raw_ostream *Errs) {
+                                                raw_ostream *Errs,
+                                                bool LongOptionsUseDoubleDash) {
   assert(hasOptions() && "No options specified!");
 
   // Expand response files.
   SmallVector<const char *, 20> newArgv(argv, argv + argc);
   BumpPtrAllocator A;
   StringSaver Saver(A);
   ExpandResponseFiles(Saver,
          Triple(sys::getProcessTriple()).isOSWindows() ?
bool CommandLineParser::ParseCommandLineOptions(int argc,
   // Loop over all of the arguments... processing them.
   bool DashDashFound = false; // Have we read '--'?
   for (int i = FirstArg; i < argc; ++i) {
     Option *Handler = nullptr;
     Option *NearestHandler = nullptr;
     std::string NearestHandlerString;
     StringRef Value;
     StringRef ArgName = "";
+    bool HaveDoubleDash = false;
 
     // Check to see if this is a positional argument.  This argument is
     // considered to be positional if it doesn't start with '-', if it is "-"
     // itself, or if we have seen "--" already.
     //
     if (argv[i][0] != '-' || argv[i][1] == 0 || DashDashFound) {
       // Positional argument!
       if (ActivePositionalArg) {
if (argv[i][0] != '-' || argv[i][1] == 0 || DashDashFound) {
       continue;             // Don't try to process it as an argument itself.
     } else if (ActivePositionalArg &&
                (ActivePositionalArg->getMiscFlags() & PositionalEatsArgs)) {
       // If there is a positional argument eating options, check to see if this
       // option is another positional argument.  If so, treat it as an argument,
       // otherwise feed it to the eating positional.
       ArgName = StringRef(argv[i] + 1);
       // Eat second dash.
-      if (!ArgName.empty() && ArgName[0] == '-')
+      if (!ArgName.empty() && ArgName[0] == '-') {
+        HaveDoubleDash = true;
         ArgName = ArgName.substr(1);
+      }
 
       Handler = LookupOption(*ChosenSubCommand, ArgName, Value);
+      if (Handler && LongOptionsUseDoubleDash && !HaveDoubleDash &&
+          !isGrouping(Handler))
+        Handler = nullptr;
       if (!Handler || Handler->getFormattingFlag() != cl::Positional) {
         ProvidePositionalOption(ActivePositionalArg, StringRef(argv[i]), i);
         continue; // We are done!
       }
-
     } else { // We start with a '-', must be an argument.
       ArgName = StringRef(argv[i] + 1);
       // Eat second dash.
-      if (!ArgName.empty() && ArgName[0] == '-')
+      if (!ArgName.empty() && ArgName[0] == '-') {
+        HaveDoubleDash = true;
         ArgName = ArgName.substr(1);
+      }
 
       Handler = LookupOption(*ChosenSubCommand, ArgName, Value);
+      if (Handler && LongOptionsUseDoubleDash && !HaveDoubleDash &&
+          !isGrouping(Handler))
+        Handler = nullptr;
 
       // Check to see if this "option" is really a prefixed or grouped argument.
-      if (!Handler)
+      if (!Handler && !(LongOptionsUseDoubleDash && HaveDoubleDash))
         Handler = HandlePrefixedOrGroupedOption(ArgName, Value, ErrorParsing,
                                                 OptionsMap);
 
       // Otherwise, look for the closest available option to report to the user
       // in the upcoming error.
       if (!Handler && SinkOpts.empty())
         NearestHandler =
             LookupNearestOption(ArgName, OptionsMap, NearestHandlerString);

 //
 static StringRef getValueStr(const Option &O, StringRef DefaultMsg) {
   if (O.ValueStr.empty())
     return DefaultMsg;
   return O.ValueStr;
 }
 
 static StringRef ArgPrefix = "  -";
+static StringRef ArgPrefixLong = "  --";
 static StringRef ArgHelpPrefix = " - ";
-static size_t ArgPrefixesSize = ArgPrefix.size() + ArgHelpPrefix.size();
+
+static size_t argPrefixesSize(size_t len) {
+  if (len == 1)
+    return ArgPrefix.size() + ArgHelpPrefix.size();
+  return ArgPrefixLong.size() + ArgHelpPrefix.size();
+}
+
+static StringRef argPrefix(size_t len) {
+  if (len == 1)
+    return ArgPrefix;
+  return ArgPrefixLong;
+}
 
 //===----------------------------------------------------------------------===//
 // cl::alias class implementation
 //
 
 // Return the width of the option tag for printing...
-size_t alias::getOptionWidth() const { return ArgStr.size() + ArgPrefixesSize; }
+size_t alias::getOptionWidth() const {
+  return ArgStr.size() + argPrefixesSize(ArgStr.size());
+}
 
 void Option::printHelpStr(StringRef HelpStr, size_t Indent,
                           size_t FirstLineIndentedBy) {
   assert(Indent >= FirstLineIndentedBy);
   std::pair<StringRef, StringRef> Split = HelpStr.split('\n');
   outs().indent(Indent - FirstLineIndentedBy)
       << ArgHelpPrefix << Split.first << "\n";
   while (!Split.second.empty()) {
     Split = Split.second.split('\n');
     outs().indent(Indent) << Split.first << "\n";
   }
 }
 
 // Print out the option for the alias.
 void alias::printOptionInfo(size_t GlobalWidth) const {
-  outs() << ArgPrefix << ArgStr;
-  printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + ArgPrefixesSize);
+  outs() << argPrefix(ArgStr.size()) << ArgStr;
+  printHelpStr(HelpStr, GlobalWidth,
+               ArgStr.size() + argPrefixesSize(ArgStr.size()));
 }
 
 //===----------------------------------------------------------------------===//
 // Parser Implementation code...
 //
 
 // basic_parser implementation
 //
 
 // Return the width of the option tag for printing...
 size_t basic_parser_impl::getOptionWidth(const Option &O) const {
   size_t Len = O.ArgStr.size();
   auto ValName = getValueName();
   if (!ValName.empty()) {
     size_t FormattingLen = 3;
     if (O.getMiscFlags() & PositionalEatsArgs)
       FormattingLen = 6;
     Len += getValueStr(O, ValName).size() + FormattingLen;
   }
 
-  return Len + ArgPrefixesSize;
+  return Len + argPrefixesSize(O.ArgStr.size());
 }
 
 // printOptionInfo - Print out information about this option.  The
 // to-be-maintained width is specified.
 //
 void basic_parser_impl::printOptionInfo(const Option &O,
                                         size_t GlobalWidth) const {
-  outs() << ArgPrefix << O.ArgStr;
+  outs() << argPrefix(O.ArgStr.size()) << O.ArgStr;
 
   auto ValName = getValueName();
   if (!ValName.empty()) {
     if (O.getMiscFlags() & PositionalEatsArgs) {
       outs() << " <" << getValueStr(O, ValName) << ">...";
     } else {
       outs() << "=<" << getValueStr(O, ValName) << '>';
     }
   }
 
   Option::printHelpStr(O.HelpStr, GlobalWidth, getOptionWidth(O));
 }
 
 void basic_parser_impl::printOptionName(const Option &O,
                                         size_t GlobalWidth) const {
-  outs() << ArgPrefix << O.ArgStr;
+  outs() << argPrefix(O.ArgStr.size()) << O.ArgStr;
   outs().indent(GlobalWidth - O.ArgStr.size());
 }
 
 // parser<bool> implementation
 //
 bool parser<bool>::parse(Option &O, StringRef ArgName, StringRef Arg,
                          bool &Value) {
   if (Arg == "" || Arg == "true" || Arg == "TRUE" || Arg == "True" ||
static bool shouldPrintOption(StringRef Name, StringRef Description,
                               const Option &O) {
   return O.getValueExpectedFlag() != ValueOptional || !Name.empty() ||
          !Description.empty();
 }
 
 // Return the width of the option tag for printing...
 size_t generic_parser_base::getOptionWidth(const Option &O) const {
   if (O.hasArgStr()) {
-    size_t Size = O.ArgStr.size() + ArgPrefixesSize + EqValue.size();
+    size_t Size =
+        O.ArgStr.size() + argPrefixesSize(O.ArgStr.size()) + EqValue.size();
     for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
       StringRef Name = getOption(i);
       if (!shouldPrintOption(Name, getDescription(i), O))
         continue;
       size_t NameSize = Name.empty() ? EmptyOption.size() : Name.size();
       Size = std::max(Size, NameSize + OptionPrefixesSize);
     }
     return Size;

 void generic_parser_base::printOptionInfo(const Option &O,
                                           size_t GlobalWidth) const {
   if (O.hasArgStr()) {
     // When the value is optional, first print a line just describing the
     // option without values.
     if (O.getValueExpectedFlag() == ValueOptional) {
       for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
         if (getOption(i).empty()) {
-          outs() << ArgPrefix << O.ArgStr;
+          outs() << argPrefix(O.ArgStr.size()) << O.ArgStr;
           Option::printHelpStr(O.HelpStr, GlobalWidth,
-                               O.ArgStr.size() + ArgPrefixesSize);
+                               O.ArgStr.size() +
+                                   argPrefixesSize(O.ArgStr.size()));
           break;
         }
       }
     }
 
-    outs() << ArgPrefix << O.ArgStr << EqValue;
+    outs() << argPrefix(O.ArgStr.size()) << O.ArgStr << EqValue;
     Option::printHelpStr(O.HelpStr, GlobalWidth,
-                         O.ArgStr.size() + EqValue.size() + ArgPrefixesSize);
+                         O.ArgStr.size() + EqValue.size() +
+                             argPrefixesSize(O.ArgStr.size()));
     for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
       StringRef OptionName = getOption(i);
       StringRef Description = getDescription(i);
       if (!shouldPrintOption(OptionName, Description, O))
         continue;
       assert(GlobalWidth >= OptionName.size() + OptionPrefixesSize);
       size_t NumSpaces = GlobalWidth - OptionName.size() - OptionPrefixesSize;
       outs() << OptionPrefix << OptionName;

llvm/test/tools/llvm-readobj/merged.test

 # Check merged args produce identical output to when not merged.
 RUN: llvm-readelf -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 > %t.merged
 RUN: llvm-readelf -a -e -W -h -S -r -n -u -d -l -V -g -I -s %p/Inputs/trivial.obj.elf-i386 > %t.not-merged
 RUN: cmp %t.merged %t.not-merged
 RUN: FileCheck %s --input-file %t.merged
 
 # llvm-readobj does not support merged args, because it also supports some old
 # flags (-st, -sd, etc.), and it would be confusing if only some merged args
 # were supported.
 RUN: not llvm-readobj -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 2>&1 | FileCheck %s --check-prefix=UNKNOWN
 
 CHECK-NOT: Unknown command line argument
-UNKNOWN:   Unknown command line argument
+UNKNOWN:   for the -section-headers option: may only occur zero or one times!

llvm/unittests/Support/CommandLineTest.cpp

EXPECT_TRUE(
       cl::ParseCommandLineOptions(2, args14, StringRef(), &llvm::nulls()));
   EXPECT_TRUE(OptF);
   EXPECT_STREQ("b", OptA.c_str());
   EXPECT_FALSE(OptB);
   OptA.clear();
   cl::ResetAllOptionOccurrences();
 }
 
+TEST(CommandLineTest, LongOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption<bool> OptA("a", cl::desc("Some flag"));
+  StackOption<bool> OptBLong("long-flag", cl::desc("Some long flag"));
+  StackOption<bool, cl::alias> OptB("b", cl::desc("Alias to --long-flag"),
+                                    cl::aliasopt(OptBLong));
+  StackOption<std::string> OptAB("ab", cl::desc("Another long option"));
+
+  std::string Errs;
+  raw_string_ostream OS(Errs);
+
+  const char *args1[] = {"prog", "-a", "-ab", "val1"};
+  const char *args2[] = {"prog", "-a", "--ab", "val1"};
+  const char *args3[] = {"prog", "-ab", "--ab", "val1"};
+
+  //
+  // The following tests treat `-` and `--` the same, and always match the
+  // longest string.
+  //
+
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(4, args1, StringRef(), &OS)); OS.flush();
+  EXPECT_TRUE(OptA);
+  EXPECT_FALSE(OptBLong);
+  EXPECT_STREQ("val1", OptAB.c_str());
+  EXPECT_TRUE(Errs.empty()); Errs.clear();
+  cl::ResetAllOptionOccurrences();
+
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(4, args2, StringRef(), &OS)); OS.flush();
+  EXPECT_TRUE(OptA);
+  EXPECT_FALSE(OptBLong);
+  EXPECT_STREQ("val1", OptAB.c_str());
+  EXPECT_TRUE(Errs.empty()); Errs.clear();
+  cl::ResetAllOptionOccurrences();
+
+  // Fails because `-ab` and `--ab` are treated the same and appear more than
+  // once.  Also, `val1` is unexpected.
+  EXPECT_FALSE(
+      cl::ParseCommandLineOptions(4, args3, StringRef(), &OS)); OS.flush();
+  outs()<< Errs << "\n";
+  EXPECT_FALSE(Errs.empty()); Errs.clear();
+  cl::ResetAllOptionOccurrences();
+
+  //
+  // The following tests treat `-` and `--` differently, with `-` for short, and
+  // `--` for long options.
+  //
+
+  // Fails because `-ab` is treated as `-a -b`, so `-a` is seen twice, and
+  // `val1` is unexpected.
+  EXPECT_FALSE(cl::ParseCommandLineOptions(4, args1, StringRef(),
+                                           &OS, nullptr, true)); OS.flush();
+  EXPECT_FALSE(Errs.empty()); Errs.clear();
+  cl::ResetAllOptionOccurrences();
+
+  // Works because `-a` is treated differently than `--ab`.
+  EXPECT_TRUE(cl::ParseCommandLineOptions(4, args2, StringRef(),
+                                           &OS, nullptr, true)); OS.flush();
+  EXPECT_TRUE(Errs.empty()); Errs.clear();
+  cl::ResetAllOptionOccurrences();
+
+  // Works because `-ab` is treated as `-a -b`, and `--ab` is a long option.
+  EXPECT_TRUE(cl::ParseCommandLineOptions(4, args3, StringRef(),
+                                           &OS, nullptr, true));
+  EXPECT_TRUE(OptA);
+  EXPECT_TRUE(OptBLong);
+  EXPECT_STREQ("val1", OptAB.c_str());
+  OS.flush();
+  EXPECT_TRUE(Errs.empty()); Errs.clear();
+  cl::ResetAllOptionOccurrences();
+}
 }  // anonymous namespace