[clang][analyzer] Own string keys in AnalyzerOptions::ConfigTable

[jansvoboda11]

Summary

This patch replaces StringMap<...> with std::map<std::string, ...> in AnalyzerOptions::ConfigTable. It seems to be the only non-owning reference to command line arguments in the whole CompilerInvocation, which would introduce a lot of complexity with round-tripping arguments (D94472).

[dexonsmith]

This patch replaces StringMap<...> with std::map<std::string, ...> in AnalyzerOptions::ConfigTable. It seems to be the only non-owning reference to command line arguments in the whole CompilerInvocation, which would introduce a lot of complexity with round-tripping arguments (D94472).

I think this patch can be skipped. StringMap owns its own strings. StringMapEntry uses tail allocation to include a null-terminated string inline. For details, have a look at StringMapEntry::Create in llvm/ADT/StringMapEntry.h.

[jansvoboda11]

In D95629#2528931, @dexonsmith wrote:

I think this patch can be skipped. StringMap owns its own strings. StringMapEntry uses tail allocation to include a null-terminated string inline. For details, have a look at StringMapEntry::Create in llvm/ADT/StringMapEntry.h.

You're right.

I first discovered lifetime issues with Config when using a non-stable data structure for holding allocated strings. Reading the documentation and looking at the API of StringMap made me believe the keys are not owned and I forgot to double-check.

Closing this, as D95369 works correctly regardless.

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

 //===- AnalyzerOptions.h - Analysis Engine Options --------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This header defines various options for the static analyzer that are set
 // by the frontend and are consulted throughout the analyzer.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_ANALYZEROPTIONS_H
 #define LLVM_CLANG_STATICANALYZER_CORE_ANALYZEROPTIONS_H
 
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
-#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
 namespace clang {
 
 namespace ento {
 

 /// whenever.
 ///
 /// Some of the options are controlled by raw frontend flags for no good reason,
 /// and should be eventually converted into -analyzer-config flags. New analyzer
 /// options should not be implemented as frontend flags. Frontend flags still
 /// make sense for things that do not affect the actual analysis.
 class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
 public:
-  using ConfigTable = llvm::StringMap<std::string>;
+  using ConfigTable = std::map<std::string, std::string>;
 
   /// Retrieves the list of checkers generated from Checkers.td. This doesn't
   /// contain statically linked but non-generated checkers and plugin checkers!
   static std::vector<StringRef>
   getRegisteredCheckers(bool IncludeExperimental = false);
 
   /// Retrieves the list of packages generated from Checkers.td. This doesn't
   /// contain statically linked but non-generated packages and plugin packages!

clang/lib/Frontend/CompilerInvocation.cpp

 //===- CompilerInvocation.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
for (const auto &configVal : configVals) {
       // Leave unknown non-checker configs unclaimed.
       if (!key.contains(":") && Opts.isUnknownAnalyzerConfig(key)) {
         if (Opts.ShouldEmitErrorsOnInvalidConfigValue)
           Diags.Report(diag::err_analyzer_config_unknown) << key;
         continue;
       }
 
       A->claim();
-      Opts.Config[key] = std::string(val);
+      Opts.Config[std::string(key)] = std::string(val);
     }
   }
 
   if (Opts.ShouldEmitErrorsOnInvalidConfigValue)
     parseAnalyzerConfigs(Opts, &Diags);
   else
     parseAnalyzerConfigs(Opts, nullptr);
 
   llvm::raw_string_ostream os(Opts.FullCompilerInvocation);
   for (unsigned i = 0; i < Args.getNumInputArgStrings(); ++i) {
     if (i != 0)
       os << " ";
     os << Args.getArgString(i);
   }
   os.flush();
 
   return Success;
 }
 
 static StringRef getStringOption(AnalyzerOptions::ConfigTable &Config,
                                  StringRef OptionName, StringRef DefaultVal) {
-  return Config.insert({OptionName, std::string(DefaultVal)}).first->second;
+  return Config.insert({std::string(OptionName), std::string(DefaultVal)})
+      .first->second;
 }
 
 static void initOption(AnalyzerOptions::ConfigTable &Config,
                        DiagnosticsEngine *Diags,
                        StringRef &OptionField, StringRef Name,
                        StringRef DefaultVal) {
   // String options may be known to invalid (e.g. if the expected string is a
   // file name, but the file does not exist), those will have to be checked in

clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp

 //==- DebugCheckers.cpp - Debugging Checkers ---------------------*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

 //===----------------------------------------------------------------------===//
 // ConfigDumper
 //===----------------------------------------------------------------------===//
 
 namespace {
 class ConfigDumper : public Checker< check::EndOfTranslationUnit > {
   typedef AnalyzerOptions::ConfigTable Table;
 
-  static int compareEntry(const Table::MapEntryTy *const *LHS,
-                          const Table::MapEntryTy *const *RHS) {
-    return (*LHS)->getKey().compare((*RHS)->getKey());
+  static int compareEntry(const Table::value_type *const *LHS,
+                          const Table::value_type *const *RHS) {
+    return (*LHS)->first.compare((*RHS)->first);
   }
 
 public:
   void checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
                                  AnalysisManager& mgr,
                                  BugReporter &BR) const {
     const Table &Config = mgr.options.Config;
 
-    SmallVector<const Table::MapEntryTy *, 32> Keys;
+    SmallVector<const Table::value_type *, 32> Keys;
     for (Table::const_iterator I = Config.begin(), E = Config.end(); I != E;
          ++I)
       Keys.push_back(&*I);
     llvm::array_pod_sort(Keys.begin(), Keys.end(), compareEntry);
 
     llvm::errs() << "[config]\n";
     for (unsigned I = 0, E = Keys.size(); I != E; ++I)
-      llvm::errs() << Keys[I]->getKey() << " = "
+      llvm::errs() << Keys[I]->first << " = "
                    << (Keys[I]->second.empty() ? "\"\"" : Keys[I]->second)
                    << '\n';
   }
 };
 }
 
 void ento::registerConfigDumper(CheckerManager &mgr) {
   mgr.registerChecker<ConfigDumper>();

clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

 //===- AnalyzerOptions.cpp - Analysis Engine Options ----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
assert(!CheckerName.empty() &&
          "Empty checker name! Make sure the checker object (including it's "
          "bases!) if fully initialized before calling this function!");
 
   ConfigTable::const_iterator E = Config.end();
   do {
     ConfigTable::const_iterator I =
         Config.find((Twine(CheckerName) + ":" + OptionName).str());
     if (I != E)
-      return StringRef(I->getValue());
+      return StringRef(I->second);
     size_t Pos = CheckerName.rfind('.');
     if (Pos == StringRef::npos)
       break;
 
     CheckerName = CheckerName.substr(0, Pos);
   } while (!CheckerName.empty() && SearchInParents);
 
   llvm_unreachable("Unknown checker option! Did you call getChecker*Option "

clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

 //===- CheckerRegistry.cpp - Maintains all available checkers -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
static void insertAndValidate(StringRef FullName, const CmdLineOption &Option,
   if (It.second)
     return;
 
   // Insertion failed, the user supplied this package/checker option on the
   // command line. If the supplied value is invalid, we'll restore the option
   // to it's default value, and if we're in non-compatibility mode, we'll also
   // emit an error.
 
-  StringRef SuppliedValue = It.first->getValue();
+  StringRef SuppliedValue = It.first->second;
 
   if (Option.OptionType == "bool") {
     if (SuppliedValue != "true" && SuppliedValue != "false") {
       if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
         Diags.Report(diag::err_analyzer_checker_option_invalid_input)
             << FullOption << "a boolean value";
       }
 
-      It.first->setValue(std::string(Option.DefaultValStr));
+      It.first->second = std::string(Option.DefaultValStr);
     }
     return;
   }
 
   if (Option.OptionType == "int") {
     int Tmp;
     bool HasFailed = SuppliedValue.getAsInteger(0, Tmp);
     if (HasFailed) {
       if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
         Diags.Report(diag::err_analyzer_checker_option_invalid_input)
             << FullOption << "an integer value";
       }
 
-      It.first->setValue(std::string(Option.DefaultValStr));
+      It.first->second = std::string(Option.DefaultValStr);
     }
     return;
   }
 }
 
 template <class T>
 static void insertOptionToCollection(StringRef FullName, T &Collection,
                                      const CmdLineOption &Option,

 }
 
 void CheckerRegistry::validateCheckerOptions() const {
   for (const auto &Config : AnOpts.Config) {
 
     StringRef SuppliedCheckerOrPackage;
     StringRef SuppliedOption;
     std::tie(SuppliedCheckerOrPackage, SuppliedOption) =
-        Config.getKey().split(':');
+        StringRef(Config.first).split(':');
 
     if (SuppliedOption.empty())
       continue;
 
     // AnalyzerOptions' config table contains the user input, so an entry could
     // look like this:
     //
     //   cor:NoFalsePositives=true