[OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

[serge-sans-paille]

Summary

StringMap are useful when we need to duplicate the key's memory. That's
not the case for llvm:🆑:Option because the keys are StringLiteral.

[nikic]

Makes sense to me.

From pre-merge checks:

FAILED: tools/clang/tools/clang-refactor/CMakeFiles/clang-refactor.dir/ClangRefactor.cpp.o
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/tools/clang-refactor -I/var/lib/buildkite-agent/builds/llvm-project/clang/tools/clang-refactor -I/var/lib/buildkite-agent/builds/llvm-project/clang/include -Itools/clang/include -Iinclude -I/var/lib/buildkite-agent/builds/llvm-project/llvm/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/clang/tools/clang-refactor/CMakeFiles/clang-refactor.dir/ClangRefactor.cpp.o -MF tools/clang/tools/clang-refactor/CMakeFiles/clang-refactor.dir/ClangRefactor.cpp.o.d -o tools/clang/tools/clang-refactor/CMakeFiles/clang-refactor.dir/ClangRefactor.cpp.o -c /var/lib/buildkite-agent/builds/llvm-project/clang/tools/clang-refactor/ClangRefactor.cpp
In file included from /var/lib/buildkite-agent/builds/llvm-project/clang/tools/clang-refactor/ClangRefactor.cpp:16:
In file included from /var/lib/buildkite-agent/builds/llvm-project/clang/include/clang/Frontend/CommandLineSourceLoc.h:18:
/var/lib/buildkite-agent/builds/llvm-project/llvm/include/llvm/Support/CommandLine.h:1254:66: error: no member named 'apply' in 'llvm::StringRef'
  template <class Opt> static void opt(const Mod &M, Opt &O) { M.apply(O); }
                                                               ~ ^
/var/lib/buildkite-agent/builds/llvm-project/llvm/include/llvm/Support/CommandLine.h:1302:20: note: in instantiation of function template specialization 'llvm::cl::applicator<llvm::StringRef>::opt<llvm::cl::opt<std::string>>' requested here
  applicator<Mod>::opt(M, *O);
                   ^
/var/lib/buildkite-agent/builds/llvm-project/llvm/include/llvm/Support/CommandLine.h:1494:5: note: in instantiation of function template specialization 'llvm::cl::apply<llvm::cl::opt<std::string>, llvm::StringRef, llvm::cl::desc, llvm::cl::NumOccurrencesFlag, llvm::cl::cat, llvm::cl::sub>' requested here
    apply(this, Ms...);
    ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/unique_ptr.h:962:34: note: in instantiation of function template specialization 'llvm::cl::opt<std::basic_string<char>, false>::opt<llvm::StringRef, llvm::cl::desc, llvm::cl::NumOccurrencesFlag, llvm::cl::cat, llvm::cl::sub>' requested here
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                                 ^
/var/lib/buildkite-agent/builds/llvm-project/clang/tools/clang-refactor/ClangRefactor.cpp:242:17: note: in instantiation of function template specialization 'std::make_unique<llvm::cl::opt<std::string>, llvm::StringRef, llvm::cl::desc, llvm::cl::NumOccurrencesFlag, llvm::cl::cat, llvm::cl::sub>' requested here
    return std::make_unique<cl::opt<T>>(
                ^
/var/lib/buildkite-agent/builds/llvm-project/clang/tools/clang-refactor/ClangRefactor.cpp:231:36: note: in instantiation of function template specialization '(anonymous namespace)::CommandLineRefactoringOptionCreator::create<std::basic_string<char>>' requested here
      Options.addStringOption(Opt, create<std::string>(Opt));
                                   ^
1 error generated.

llvm/lib/Support/CommandLine.cpp

588–589

Can probably use range based loop here and avoid naming the type altogether.

[barannikov88]

llvm/lib/CodeGen/TargetPassConfig.cpp

213–216

From the comment near StringLiteral:

StringLiteral should *only* be used in a constexpr context, as such:
constexpr StringLiteral S("test");

[serge-sans-paille]

Fix build + make StringLiteral initialization constexpr.

[nikic]

FAILED: tools/mlir/test/lib/Dialect/Linalg/CMakeFiles/MLIRLinalgTestPasses.dir/TestLinalgHoisting.cpp.o
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=1 -DMLIR_INCLUDE_TESTS -DMLIR_ROCM_CONVERSIONS_ENABLED=1 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/mlir/test/lib/Dialect/Linalg -I/var/lib/buildkite-agent/builds/llvm-project/mlir/test/lib/Dialect/Linalg -Iinclude -I/var/lib/buildkite-agent/builds/llvm-project/llvm/include -I/var/lib/buildkite-agent/builds/llvm-project/mlir/include -Itools/mlir/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Werror=mismatched-tags -O3 -DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/mlir/test/lib/Dialect/Linalg/CMakeFiles/MLIRLinalgTestPasses.dir/TestLinalgHoisting.cpp.o -MF tools/mlir/test/lib/Dialect/Linalg/CMakeFiles/MLIRLinalgTestPasses.dir/TestLinalgHoisting.cpp.o.d -o tools/mlir/test/lib/Dialect/Linalg/CMakeFiles/MLIRLinalgTestPasses.dir/TestLinalgHoisting.cpp.o -c /var/lib/buildkite-agent/builds/llvm-project/mlir/test/lib/Dialect/Linalg/TestLinalgHoisting.cpp
In file included from /var/lib/buildkite-agent/builds/llvm-project/mlir/test/lib/Dialect/Linalg/TestLinalgHoisting.cpp:17:
In file included from /var/lib/buildkite-agent/builds/llvm-project/mlir/include/mlir/Pass/Pass.h:13:
In file included from /var/lib/buildkite-agent/builds/llvm-project/mlir/include/mlir/Pass/PassRegistry.h:17:
In file included from /var/lib/buildkite-agent/builds/llvm-project/mlir/include/mlir/Pass/PassOptions.h:21:
/var/lib/buildkite-agent/builds/llvm-project/llvm/include/llvm/Support/CommandLine.h:1254:66: error: no member named 'apply' in 'llvm::StringRef'
  template <class Opt> static void opt(const Mod &M, Opt &O) { M.apply(O); }
                                                               ~ ^
/var/lib/buildkite-agent/builds/llvm-project/llvm/include/llvm/Support/CommandLine.h:1302:20: note: in instantiation of function template specialization 'llvm::cl::applicator<llvm::StringRef>::opt<llvm::cl::opt<bool>>' requested here
  applicator<Mod>::opt(M, *O);
                   ^
/var/lib/buildkite-agent/builds/llvm-project/llvm/include/llvm/Support/CommandLine.h:1494:5: note: in instantiation of function template specialization 'llvm::cl::apply<llvm::cl::opt<bool>, llvm::StringRef, llvm::cl::sub, llvm::cl::desc, llvm::cl::initializer<bool>>' requested here
    apply(this, Ms...);
    ^
/var/lib/buildkite-agent/builds/llvm-project/mlir/include/mlir/Pass/PassOptions.h:164:11: note: in instantiation of function template specialization 'llvm::cl::opt<bool, false>::opt<llvm::StringRef, llvm::cl::sub, llvm::cl::desc, llvm::cl::initializer<bool>>' requested here
        : llvm::cl::opt<DataType, /*ExternalStorage=*/false, OptionParser>(
          ^
/var/lib/buildkite-agent/builds/llvm-project/mlir/include/mlir/Pass/Pass.h:98:11: note: in instantiation of function template specialization 'mlir::detail::PassOptions::Option<bool, llvm::cl::parser<bool>>::Option<llvm::cl::desc, llvm::cl::initializer<bool>>' requested here
        : detail::PassOptions::Option<DataType, OptionParser>(
          ^
/var/lib/buildkite-agent/builds/llvm-project/mlir/test/lib/Dialect/Linalg/TestLinalgHoisting.cpp:39:43: note: in instantiation of function template specialization 'mlir::Pass::Option<bool, llvm::cl::parser<bool>>::Option<llvm::cl::desc, llvm::cl::initializer<bool>>' requested here
  Option<bool> testHoistRedundantTransfers{
                                          ^
1 error generated.

[serge-sans-paille]

@nikic : I thought I fixed that with e8a163dc03e6913360beb305620104ba129c081c ... is it included in your build?

[nikic]

In D142181#4070868, @serge-sans-paille wrote:

@nikic : I thought I fixed that with e8a163dc03e6913360beb305620104ba129c081c ... is it included in your build?

Oh yeah, I missed that fix. Sorry for the noise.

[serge-sans-paille]

In D142181#4070881, @nikic wrote:

In D142181#4070868, @serge-sans-paille wrote:

@nikic : I thought I fixed that with e8a163dc03e6913360beb305620104ba129c081c ... is it included in your build?

Oh yeah, I missed that fix. Sorry for the noise.

^^! Answered in the wrong thread, so I dupliaded that message to https://reviews.llvm.org/D142026 where it belongs :-)

[serge-sans-paille]

Do not enfore StringLiteral parameter. This is too invasive on the code base.

[serge-sans-paille]

Gentle ping :-)

clang-tools-extra/clangd/tool/ClangdMain.cpp

 OptionCategory Retired("clangd flags no longer in use");
 const OptionCategory *ClangdCategories[] = {&Features, &Protocol,
                                             &CompileCommands, &Misc, &Retired};
 
 template <typename T> class RetiredFlag {
   opt<T> Option;
 
 public:
-  RetiredFlag(llvm::StringRef Name)
+  RetiredFlag(llvm::StringLiteral Name)
       : Option(Name, cat(Retired), desc("Obsolete flag, ignored"), Hidden,
                llvm::cl::callback([Name](const T &) {
                  llvm::errs()
                      << "The flag `-" << Name << "` is obsolete and ignored.\n";
                })) {}
 };
 
 enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };

clang/include/clang/Tooling/Refactoring/RefactoringOption.h

 /// the clang-refactor tool is used.
 class RefactoringOption {
 public:
   virtual ~RefactoringOption() {}
 
   /// Returns the name of the refactoring option.
   ///
   /// Each refactoring option must have a unique name.
-  virtual StringRef getName() const = 0;
+  virtual llvm::StringLiteral getName() const = 0;
 
   virtual StringRef getDescription() const = 0;
 
   /// True when this option must be specified before invoking the refactoring
   /// action.
   virtual bool isRequired() const = 0;
 
   /// Invokes the \c visit method in the option consumer that's appropriate

clang/lib/Tooling/Refactoring/RefactoringActions.cpp

 
 namespace clang {
 namespace tooling {
 
 namespace {
 
 class DeclNameOption final : public OptionalRefactoringOption<std::string> {
 public:
-  StringRef getName() const override { return "name"; }
+  llvm::StringLiteral getName() const override { return "name"; }
   StringRef getDescription() const override {
     return "Name of the extracted declaration";
   }
 };
 
 // FIXME: Rewrite the Actions to avoid duplication of descriptions/names with
 // rules.
 class ExtractRefactoring final : public RefactoringAction {
Rules.push_back(createRefactoringActionRule<ExtractFunction>(
         CodeRangeASTSelectionRequirement(),
         OptionRequirement<DeclNameOption>()));
     return Rules;
   }
 };
 
 class OldQualifiedNameOption : public RequiredRefactoringOption<std::string> {
 public:
-  StringRef getName() const override { return "old-qualified-name"; }
+  llvm::StringLiteral getName() const override { return "old-qualified-name"; }
   StringRef getDescription() const override {
     return "The old qualified name to be renamed";
   }
 };
 
 class NewQualifiedNameOption : public RequiredRefactoringOption<std::string> {
 public:
-  StringRef getName() const override { return "new-qualified-name"; }
+  llvm::StringLiteral getName() const override { return "new-qualified-name"; }
   StringRef getDescription() const override {
     return "The new qualified name to change the symbol to";
   }
 };
 
 class NewNameOption : public RequiredRefactoringOption<std::string> {
 public:
-  StringRef getName() const override { return "new-name"; }
+  llvm::StringLiteral getName() const override { return "new-name"; }
   StringRef getDescription() const override {
     return "The new name to change the symbol to";
   }
 };
 
 // FIXME: Rewrite the Actions to avoid duplication of descriptions/names with
 // rules.
 class LocalRename final : public RefactoringAction {

llvm/include/llvm/Option/OptTable.h

 /// parts of the driver still use Option instances where convenient.
 class OptTable {
 public:
   /// Entry for a single option instance in the option data table.
   struct Info {
     /// A null terminated array of prefix strings to apply to name while
     /// matching.
     ArrayRef<StringLiteral> Prefixes;
-    StringRef Name;
+    StringLiteral Name;
     const char *HelpText;
     const char *MetaVar;
     unsigned ID;
     unsigned char Kind;
     unsigned char Param;
     unsigned int Flags;
     unsigned short GroupID;
     unsigned short AliasID;

llvm/include/llvm/Option/Option.h

public:
   }
 
   OptionClass getKind() const {
     assert(Info && "Must have a valid info!");
     return OptionClass(Info->Kind);
   }
 
   /// Get the name of this option without any prefix.
-  StringRef getName() const {
+  StringLiteral getName() const {
     assert(Info && "Must have a valid info!");
     return Info->Name;
   }
 
   const Option getGroup() const {
     assert(Info && "Must have a valid info!");
     assert(Owner && "Must have a valid owner!");
     return Owner->getOption(Info->GroupID);

llvm/include/llvm/Support/CommandLine.h

public:
 
   explicit operator bool() const;
 
   StringRef getName() const { return Name; }
   StringRef getDescription() const { return Description; }
 
   SmallVector<Option *, 4> PositionalOpts;
   SmallVector<Option *, 4> SinkOpts;
-  StringMap<Option *> OptionsMap;
+  DenseMap<StringRef, Option *> OptionsMap;
 
   Option *ConsumeAfterOpt = nullptr; // The ConsumeAfter option if it exists.
 };
 
 // A special subcommand representing no subcommand
 extern ManagedStatic<SubCommand> TopLevelSubCommand;
 
 // A special subcommand that can be used to put an option into all subcommands.
void printOptionDiff(
 
   OptionDiffPrinter<typename ParserClass::parser_data_type, ValDT> printer;
   printer.print(O, static_cast<const ParserClass &>(P), V, Default,
                 GlobalWidth);
 }
 
 //===----------------------------------------------------------------------===//
 // This class is used because we must use partial specialization to handle
-// literal string arguments specially (const char* does not correctly respond to
-// the apply method). Because the syntax to use this is a pain, we have the
-// 'apply' method below to handle the nastiness...
+// literal string arguments specially (const char[...] does not correctly
+// respond to the apply method). Because the syntax to use this is a pain, we
+// have the 'apply' method below to handle the nastiness...
+//
+// Note that char * and StringRef are not part of the specialization, which
+// helps preventing reference to stack variables (but doesn't totally prevents
+// it, unfortunately).
 //
 template <class Mod> struct applicator {
   template <class Opt> static void opt(const Mod &M, Opt &O) { M.apply(O); }
 };
 
-// Handle const char* as a special case...
 template <unsigned n> struct applicator<char[n]> {
   template <class Opt> static void opt(StringRef Str, Opt &O) {
     O.setArgStr(Str);
   }
 };
 template <unsigned n> struct applicator<const char[n]> {
   template <class Opt> static void opt(StringRef Str, Opt &O) {
     O.setArgStr(Str);
   }
 };
-template <> struct applicator<StringRef > {
+template <> struct applicator<StringLiteral> {
   template <class Opt> static void opt(StringRef Str, Opt &O) {
     O.setArgStr(Str);
   }
 };
 
 template <> struct applicator<NumOccurrencesFlag> {
   static void opt(NumOccurrencesFlag N, Option &O) {
     O.setNumOccurrencesFlag(N);

 ///
 /// This interface is useful for modifying options in libraries that are out of
 /// the control of the client. The options should be modified before calling
 /// llvm::cl::ParseCommandLineOptions().
 ///
 /// Hopefully this API can be deprecated soon. Any situation where options need
 /// to be modified by tools or libraries should be handled by sane APIs rather
 /// than just handing around a global list.
-StringMap<Option *> &
+DenseMap<StringRef, Option *> &
 getRegisteredOptions(SubCommand &Sub = SubCommand::getTopLevel());
 
 /// Use this to get all registered SubCommands from the provided parser.
 ///
 /// \return A range of all SubCommand pointers registered with the parser.
 ///
 /// Typical usage:
 /// \code

llvm/lib/CodeGen/TargetPassConfig.cpp

 
 // Experimental option to run live interval analysis early.
 static cl::opt<bool> EarlyLiveIntervals("early-live-intervals", cl::Hidden,
     cl::desc("Run live interval analysis earlier in the pipeline"));
 
 /// Option names for limiting the codegen pipeline.
 /// Those are used in error reporting and we didn't want
 /// to duplicate their names all over the place.
-static const char StartAfterOptName[] = "start-after";
-static const char StartBeforeOptName[] = "start-before";
-static const char StopAfterOptName[] = "stop-after";
-static const char StopBeforeOptName[] = "stop-before";
+static constexpr StringLiteral StartAfterOptName = "start-after";
+static constexpr StringLiteral StartBeforeOptName = "start-before";
+static constexpr StringLiteral StopAfterOptName = "stop-after";
+static constexpr StringLiteral StopBeforeOptName = "stop-before";
 
 static cl::opt<std::string>
-    StartAfterOpt(StringRef(StartAfterOptName),
+    StartAfterOpt(StartAfterOptName,
                   cl::desc("Resume compilation after a specific pass"),
                   cl::value_desc("pass-name"), cl::init(""), cl::Hidden);
 
 static cl::opt<std::string>
-    StartBeforeOpt(StringRef(StartBeforeOptName),
+    StartBeforeOpt(StartBeforeOptName,
                    cl::desc("Resume compilation before a specific pass"),
                    cl::value_desc("pass-name"), cl::init(""), cl::Hidden);
 
 static cl::opt<std::string>
-    StopAfterOpt(StringRef(StopAfterOptName),
+    StopAfterOpt(StopAfterOptName,
                  cl::desc("Stop compilation after a specific pass"),
                  cl::value_desc("pass-name"), cl::init(""), cl::Hidden);
 
 static cl::opt<std::string>
-    StopBeforeOpt(StringRef(StopBeforeOptName),
+    StopBeforeOpt(StopBeforeOptName,
                   cl::desc("Stop compilation before a specific pass"),
                   cl::value_desc("pass-name"), cl::init(""), cl::Hidden);
 
 /// Enable the machine function splitter pass.
 static cl::opt<bool> EnableMachineFunctionSplitter(
     "enable-split-machine-functions", cl::Hidden,
     cl::desc("Split out cold blocks from machine functions based on profile "
              "information."));

 
 static const PassInfo *getPassInfo(StringRef PassName) {
   if (PassName.empty())
     return nullptr;
 
   const PassRegistry &PR = *PassRegistry::getPassRegistry();
   const PassInfo *PI = PR.getPassInfo(PassName);
   if (!PI)
-    report_fatal_error(Twine('\"') + Twine(PassName) +
-                       Twine("\" pass is not registered."));
+    report_fatal_error("\"" + PassName + "\" pass is not registered.");
   return PI;
 }
 
 static AnalysisID getPassIDFromName(StringRef PassName) {
   const PassInfo *PI = getPassInfo(PassName);
   return PI ? PI->getTypeInfo() : nullptr;
 }
 
void TargetPassConfig::setStartStopPasses() {
   std::tie(StopAfterName, StopAfterInstanceNum)
     = getPassNameAndInstanceNum(StopAfterOpt);
 
   StartBefore = getPassIDFromName(StartBeforeName);
   StartAfter = getPassIDFromName(StartAfterName);
   StopBefore = getPassIDFromName(StopBeforeName);
   StopAfter = getPassIDFromName(StopAfterName);
   if (StartBefore && StartAfter)
-    report_fatal_error(Twine(StartBeforeOptName) + Twine(" and ") +
-                       Twine(StartAfterOptName) + Twine(" specified!"));
+    report_fatal_error(StartBeforeOptName + " and " + StartAfterOptName +
+                       " specified!");
   if (StopBefore && StopAfter)
-    report_fatal_error(Twine(StopBeforeOptName) + Twine(" and ") +
-                       Twine(StopAfterOptName) + Twine(" specified!"));
+    report_fatal_error(StopBeforeOptName + " and " + StopAfterOptName +
+                       " specified!");
   Started = (StartAfter == nullptr) && (StartBefore == nullptr);
 }
 
 CGPassBuilderOption llvm::getCGPassBuilderOption() {
   CGPassBuilderOption Opt;
 
 #define SET_OPTION(Option)                                                     \
   if (Option.getNumOccurrences())                                              \
std::tie(StartBefore, std::ignore) =
       LLVMTM.getPassNameFromLegacyName(StartBefore);
   std::tie(StartAfter, std::ignore) =
       LLVMTM.getPassNameFromLegacyName(StartAfter);
   std::tie(StopBefore, std::ignore) =
       LLVMTM.getPassNameFromLegacyName(StopBefore);
   std::tie(StopAfter, std::ignore) =
       LLVMTM.getPassNameFromLegacyName(StopAfter);
   if (!StartBefore.empty() && !StartAfter.empty())
-    report_fatal_error(Twine(StartBeforeOptName) + Twine(" and ") +
-                       Twine(StartAfterOptName) + Twine(" specified!"));
+    report_fatal_error(StartBeforeOptName + " and " + StartAfterOptName +
+                       " specified!");
   if (!StopBefore.empty() && !StopAfter.empty())
-    report_fatal_error(Twine(StopBeforeOptName) + Twine(" and ") +
-                       Twine(StopAfterOptName) + Twine(" specified!"));
+    report_fatal_error(StopBeforeOptName + " and " + StopAfterOptName +
+                       " specified!");
 
   PIC.registerShouldRunOptionalPassCallback(
       [=, EnableCurrent = StartBefore.empty() && StartAfter.empty(),
        EnableNext = std::optional<bool>(), StartBeforeCount = 0u,
        StartAfterCount = 0u, StopBeforeCount = 0u,
        StopAfterCount = 0u](StringRef P, Any) mutable {
         bool StartBeforePass = !StartBefore.empty() && P.contains(StartBefore);
         bool StartAfterPass = !StartAfter.empty() && P.contains(StartAfter);

 
 std::string
 TargetPassConfig::getLimitedCodeGenPipelineReason(const char *Separator) {
   if (!hasLimitedCodeGenPipeline())
     return std::string();
   std::string Res;
   static cl::opt<std::string> *PassNames[] = {&StartAfterOpt, &StartBeforeOpt,
                                               &StopAfterOpt, &StopBeforeOpt};
-  static const char *OptNames[] = {StartAfterOptName, StartBeforeOptName,
-                                   StopAfterOptName, StopBeforeOptName};
+  static constexpr StringLiteral OptNames[] = {
+      StartAfterOptName, StartBeforeOptName, StopAfterOptName,
+      StopBeforeOptName};
   bool IsFirst = true;
   for (int Idx = 0; Idx < 4; ++Idx)
     if (!PassNames[Idx]->empty()) {
       if (!IsFirst)
         Res += Separator;
       IsFirst = false;
       Res += OptNames[Idx];
     }

llvm/lib/Support/CommandLine.cpp

void removeOption(Option *O, SubCommand *SC) {
     O->getExtraOptionNames(OptionNames);
     if (O->hasArgStr())
       OptionNames.push_back(O->ArgStr);
 
     SubCommand &Sub = *SC;
     auto End = Sub.OptionsMap.end();
     for (auto Name : OptionNames) {
       auto I = Sub.OptionsMap.find(Name);
-      if (I != End && I->getValue() == O)
+      if (I != End && I->second == O)
         Sub.OptionsMap.erase(I);
     }
 
     if (O->getFormattingFlag() == cl::Positional)
       for (auto *Opt = Sub.PositionalOpts.begin();
            Opt != Sub.PositionalOpts.end(); ++Opt) {
         if (*Opt == O) {
           Sub.PositionalOpts.erase(Opt);
void registerSubCommand(SubCommand *sub) {
     // option to this subcommand now.
     if (sub != &SubCommand::getAll()) {
       for (auto &E : SubCommand::getAll().OptionsMap) {
         Option *O = E.second;
         if ((O->isPositional() || O->isSink() || O->isConsumeAfter()) ||
             O->hasArgStr())
           addOption(O, sub);
         else
-          addLiteralOption(*O, sub, E.first());
+          addLiteralOption(*O, sub, E.first);
       }
     }
   }
 
   void unregisterSubCommand(SubCommand *sub) {
     RegisteredSubCommands.erase(sub);
   }
 
SubCommand *CommandLineParser::LookupSubCommand(StringRef Name) {
   }
   return &SubCommand::getTopLevel();
 }
 
 /// LookupNearestOption - Lookup the closest match to the option specified by
 /// the specified option on the command line.  If there is a value specified
 /// (after an equal sign) return that as well.  This assumes that leading dashes
 /// have already been stripped.
-static Option *LookupNearestOption(StringRef Arg,
-                                   const StringMap<Option *> &OptionsMap,
-                                   std::string &NearestString) {
+static Option *
+LookupNearestOption(StringRef Arg,
+                    const DenseMap<StringRef, Option *> &OptionsMap,
   // Reject all dashes.
   if (Arg.empty())
     return nullptr;
 
   // Split on any equal sign.
   std::pair<StringRef, StringRef> SplitArg = Arg.split('=');
   StringRef &LHS = SplitArg.first; // LHS == Arg when no '=' is present.
   StringRef &RHS = SplitArg.second;
 
   // Find the closest match.
   Option *Best = nullptr;
   unsigned BestDistance = 0;
-  for (StringMap<Option *>::const_iterator it = OptionsMap.begin(),
-                                           ie = OptionsMap.end();
-       it != ie; ++it) {
-    Option *O = it->second;
+  for (auto const &Opt : OptionsMap) {
+    Option *O = Opt.second;
     // Do not suggest really hidden options (not shown in any help).
     if (O->getOptionHiddenFlag() == ReallyHidden)
       continue;
 
     SmallVector<StringRef, 16> OptionNames;
     O->getExtraOptionNames(OptionNames);
     if (O->hasArgStr())
       OptionNames.push_back(O->ArgStr);

 // getOptionPred - Check to see if there are any options that satisfy the
 // 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()))
+                             const DenseMap<StringRef, Option *> &OptionsMap) {
+  DenseMap<StringRef, Option *>::const_iterator OMI = OptionsMap.find(Name);
+  if (OMI != OptionsMap.end() && !Pred(OMI->second))
     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()))
+    if (OMI != OptionsMap.end() && !Pred(OMI->second))
       OMI = OptionsMap.end();
   }
 
   if (OMI != OptionsMap.end() && Pred(OMI->second)) {
     Length = Name.size();
     return OMI->second; // Found one!
   }
   return nullptr; // No option found!
 }
 
 /// HandlePrefixedOrGroupedOption - The specified argument string (which started
 /// with at least one '-') does not fully match an available option.  Check to
 /// see if this is a prefix or grouped option.  If so, split arg into output an
 /// Arg/Value pair and return the Option to parse it with.
 static Option *
 HandlePrefixedOrGroupedOption(StringRef &Arg, StringRef &Value,
                               bool &ErrorParsing,
-                              const StringMap<Option *> &OptionsMap) {
+                              const DenseMap<StringRef, Option *> &OptionsMap) {
   if (Arg.size() == 1)
     return nullptr;
 
   // Do the lookup!
   size_t Length = 0;
   Option *PGOpt = getOptionPred(Arg, Length, isPrefixedOrGrouping, OptionsMap);
   if (!PGOpt)
     return nullptr;

 }
 
 static int SubNameCompare(const std::pair<const char *, SubCommand *> *LHS,
                           const std::pair<const char *, SubCommand *> *RHS) {
   return strcmp(LHS->first, RHS->first);
 }
 
 // Copy Options into a vector so we can sort them as we like.
-static void sortOpts(StringMap<Option *> &OptMap,
+static void sortOpts(DenseMap<StringRef, Option *> &OptMap,
                      SmallVectorImpl<std::pair<const char *, Option *>> &Opts,
                      bool ShowHidden) {
   SmallPtrSet<Option *, 32> OptionSet; // Duplicate option detection.
 
-  for (StringMap<Option *>::iterator I = OptMap.begin(), E = OptMap.end();
+  for (DenseMap<StringRef, Option *>::iterator I = OptMap.begin(),
+                                               E = OptMap.end();
        I != E; ++I) {
     // Ignore really-hidden options.
     if (I->second->getOptionHiddenFlag() == ReallyHidden)
       continue;
 
     // Unless showhidden is set, ignore hidden flags.
     if (I->second->getOptionHiddenFlag() == Hidden && !ShowHidden)
       continue;
 
     // If we've already seen this option, don't add it to the list again.
     if (!OptionSet.insert(I->second).second)
       continue;
 
     Opts.push_back(
-        std::pair<const char *, Option *>(I->getKey().data(), I->second));
+        std::pair<const char *, Option *>(I->first.data(), I->second));
   }
 
   // Sort the options list alphabetically.
   array_pod_sort(Opts.begin(), Opts.end(), OptNameCompare);
 }
 
 static void
 sortSubCommands(const SmallPtrSetImpl<SubCommand *> &SubMap,

 void cl::SetVersionPrinter(VersionPrinterTy func) {
   CommonOptions->OverrideVersionPrinter = func;
 }
 
 void cl::AddExtraVersionPrinter(VersionPrinterTy func) {
   CommonOptions->ExtraVersionPrinters.push_back(func);
 }
 
-StringMap<Option *> &cl::getRegisteredOptions(SubCommand &Sub) {
+DenseMap<StringRef, Option *> &cl::getRegisteredOptions(SubCommand &Sub) {
   initCommonOptions();
   auto &Subs = GlobalParser->RegisteredSubCommands;
   (void)Subs;
   assert(is_contained(Subs, &Sub));
   return Sub.OptionsMap;
 }
 
 iterator_range<typename SmallPtrSet<SubCommand *, 4>::iterator>

llvm/unittests/Support/CommandLineInit/CommandLineInitTest.cpp

#if defined(_MSC_VER)
   _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
 #endif
 #endif
 
   return RUN_ALL_TESTS();
 }
 
 TEST(CommandLineInitTest, GetPresetOptions) {
-  StringMap<cl::Option *> &Map =
+  DenseMap<StringRef, cl::Option *> &Map =
       cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
 
   for (auto *Str :
        {"help", "help-hidden", "help-list", "help-list-hidden", "version"})
     EXPECT_EQ(Map.count(Str), (size_t)1)
         << "Could not get preset option `" << Str << '`';
 }

llvm/unittests/Support/CommandLineTest.cpp

 cl::OptionCategory TestCategory("Test Options", "Description");
 TEST(CommandLineTest, ModifyExisitingOption) {
   StackOption<int> TestOption("test-option", cl::desc("old description"));
 
   static const char Description[] = "New description";
   static const char ArgString[] = "new-test-option";
   static const char ValueString[] = "Integer";
 
-  StringMap<cl::Option *> &Map =
+  DenseMap<StringRef, cl::Option *> &Map =
       cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
 
   ASSERT_EQ(Map.count("test-option"), 1u) << "Could not find option in map.";
 
   cl::Option *Retrieved = Map["test-option"];
   ASSERT_EQ(&TestOption, Retrieved) << "Retrieved wrong option.";
 
   ASSERT_NE(Retrieved->Categories.end(),
TEST(CommandLineTest, HideUnrelatedOptions) {
 
   cl::HideUnrelatedOptions(TestCategory);
 
   ASSERT_EQ(cl::ReallyHidden, TestOption1.getOptionHiddenFlag())
       << "Failed to hide extra option.";
   ASSERT_EQ(cl::NotHidden, TestOption2.getOptionHiddenFlag())
       << "Hid extra option that should be visable.";
 
-  StringMap<cl::Option *> &Map =
+  DenseMap<StringRef, cl::Option *> &Map =
       cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
   ASSERT_TRUE(Map.count("help") == (size_t)0 ||
               cl::NotHidden == Map["help"]->getOptionHiddenFlag())
       << "Hid default option that should be visable.";
 }
 
 cl::OptionCategory TestCategory2("Test Options set 2", "Description");
 
TEST(CommandLineTest, HideUnrelatedOptionsMulti) {
 
   ASSERT_EQ(cl::ReallyHidden, TestOption1.getOptionHiddenFlag())
       << "Failed to hide extra option.";
   ASSERT_EQ(cl::NotHidden, TestOption2.getOptionHiddenFlag())
       << "Hid extra option that should be visable.";
   ASSERT_EQ(cl::NotHidden, TestOption3.getOptionHiddenFlag())
       << "Hid extra option that should be visable.";
 
-  StringMap<cl::Option *> &Map =
+  DenseMap<StringRef, cl::Option *> &Map =
       cl::getRegisteredOptions(cl::SubCommand::getTopLevel());
   ASSERT_TRUE(Map.count("help") == (size_t)0 ||
               cl::NotHidden == Map["help"]->getOptionHiddenFlag())
       << "Hid default option that should be visable.";
 }
 
 TEST(CommandLineTest, SetMultiValues) {
   StackOption<int> Option("option");
template <typename... Ts> std::string runTest(Ts... OptionAttributes) {
     }
     auto Buffer = MemoryBuffer::getFile(File.FilePath);
     if (!Buffer)
       return "";
     return Buffer->get()->getBuffer().str();
   }
 
   enum class OptionValue { Val };
-  const StringRef Opt = "some-option";
+  const StringLiteral Opt = "some-option";
   const StringRef HelpText = "some help";
 
 private:
   // This is a workaround for cl::Option sub-classes having their
   // printOptionInfo functions private.
   void printOptionInfo(const cl::Option &O, size_t Width) {
     O.printOptionInfo(Width);
   }
TEST_F(PrintOptionInfoTest, PrintOptionInfoMultilineValueDescription) {
   // clang-format on
 }
 
 class GetOptionWidthTest : public ::testing::Test {
 public:
   enum class OptionValue { Val };
 
   template <typename... Ts>
-  size_t runTest(StringRef ArgName, Ts... OptionAttributes) {
+  size_t runTest(StringLiteral ArgName, Ts... OptionAttributes) {
     StackOption<OptionValue> TestOption(ArgName, cl::desc("some help"),
                                         OptionAttributes...);
     return getOptionWidth(TestOption);
   }
 
 private:
   // This is a workaround for cl::Option sub-classes having their
   // printOptionInfo
   // functions private.
   size_t getOptionWidth(const cl::Option &O) { return O.getOptionWidth(); }
 };
 
 TEST_F(GetOptionWidthTest, GetOptionWidthArgNameLonger) {
-  StringRef ArgName("a-long-argument-name");
+  StringLiteral ArgName("a-long-argument-name");
   size_t ExpectedStrSize = ("  --" + ArgName + "=<value> - ").str().size();
   EXPECT_EQ(
       runTest(ArgName, cl::values(clEnumValN(OptionValue::Val, "v", "help"))),
       ExpectedStrSize);
 }
 
 TEST_F(GetOptionWidthTest, GetOptionWidthFirstOptionNameLonger) {
-  StringRef OptName("a-long-option-name");
+  StringLiteral OptName("a-long-option-name");
   size_t ExpectedStrSize = ("    =" + OptName + " - ").str().size();
   EXPECT_EQ(
       runTest("a", cl::values(clEnumValN(OptionValue::Val, OptName, "help"),
                               clEnumValN(OptionValue::Val, "b", "help"))),
       ExpectedStrSize);
 }
 
 TEST_F(GetOptionWidthTest, GetOptionWidthSecondOptionNameLonger) {
-  StringRef OptName("a-long-option-name");
+  StringLiteral OptName("a-long-option-name");
   size_t ExpectedStrSize = ("    =" + OptName + " - ").str().size();
   EXPECT_EQ(
       runTest("a", cl::values(clEnumValN(OptionValue::Val, "b", "help"),
                               clEnumValN(OptionValue::Val, OptName, "help"))),
       ExpectedStrSize);
 }
 
 TEST_F(GetOptionWidthTest, GetOptionWidthEmptyOptionNameLonger) {
   size_t ExpectedStrSize = StringRef("    =<empty> - ").size();
   // The length of a=<value> (including indentation) is actually the same as the
   // =<empty> string, so it is impossible to distinguish via testing the case
   // where the empty string is picked from where the option name is picked.
   EXPECT_EQ(runTest("a", cl::values(clEnumValN(OptionValue::Val, "b", "help"),
                                     clEnumValN(OptionValue::Val, "", "help"))),
             ExpectedStrSize);
 }
 
 TEST_F(GetOptionWidthTest,
        GetOptionWidthValueOptionalEmptyOptionWithNoDescription) {
-  StringRef ArgName("a");
+  StringLiteral ArgName("a");
   // The length of a=<value> (including indentation) is actually the same as the
   // =<empty> string, so it is impossible to distinguish via testing the case
   // where the empty string is ignored from where it is not ignored.
   // The dash will not actually be printed, but the space it would take up is
   // included to ensure a consistent column width.
   size_t ExpectedStrSize = ("  -" + ArgName + "=<value> - ").str().size();
   EXPECT_EQ(runTest(ArgName, cl::ValueOptional,
                     cl::values(clEnumValN(OptionValue::Val, "value", "help"),