steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun, jansvoboda11.
Herald added subscribers: PiotrZSL, carlosgalvezp, manas, ASDenysPetrov,
martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, mgrang,
szepet, baloghadamsoftware.
Herald added a reviewer: Szelethus.
Herald added a reviewer: njames93.
Herald added a project: All.
steakhal requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
In asserted builds, we do a so-called "round-tripping". This is called
from `CompilerInvocation::CreateFromArgs`, which eventually calls
`generateCC1CommandLine` from the lambda. That, in turn, calls a bunch
of `GenerateXXX` functions, thus ours as well: `GenerateAnalyzerArgs`.
The round tripping ensures some consistency property, also that the
generator generates the arguments in a deterministic order.
This works at the moment, but as soon as we add a new AnalyzerOption to
the AnalyzerOptions.def file, it's pretty likely that it will produce
the same sequence of args BUT in a different order for the second round
of round-tripping, resulting in an `err_cc1_round_trip_mismatch`
diagnostic.
This happens because inside `GenerateAnalyzerArgs`, we iterate over the
`Opts.Config`, which is of type `StringMap`. Well, the iteration order
is not guaranteed to be deterministic for that. Quote
https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringmap-h
> StringMap iteration order, however, is not guaranteed to be
> deterministic, so any uses which require that should instead use a
> std::map.
Consequently, to allow adding/removing analyzer options to
`AnalyzerOptions.def` without relying on the current iteration order, we
need to make that deterministic. And that is what I'm proposing in this
patch.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154437
Files:
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -357,7 +357,7 @@
// 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") {
@@ -366,7 +366,7 @@
<< FullOption << "a boolean value";
}
- It.first->setValue(std::string(Option.DefaultValStr));
+ It.first->second = Option.DefaultValStr.str();
}
return;
}
@@ -380,7 +380,7 @@
<< FullOption << "an integer value";
}
- It.first->setValue(std::string(Option.DefaultValStr));
+ It.first->second = Option.DefaultValStr.str();
}
return;
}
@@ -492,7 +492,7 @@
StringRef SuppliedCheckerOrPackage;
StringRef SuppliedOption;
std::tie(SuppliedCheckerOrPackage, SuppliedOption) =
- Config.getKey().split(':');
+ StringRef(Config.first).split(':');
if (SuppliedOption.empty())
continue;
Index: clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -136,7 +136,7 @@
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;
Index: clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
@@ -259,9 +259,9 @@
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:
@@ -270,7 +270,7 @@
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);
@@ -278,7 +278,7 @@
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';
}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -880,11 +880,11 @@
for (const auto &C : Opts.Config) {
// Don't generate anything that came from parseAnalyzerConfigs. It would be
// redundant and may not be valid on the command line.
- auto Entry = ConfigOpts.Config.find(C.getKey());
- if (Entry != ConfigOpts.Config.end() && Entry->getValue() == C.getValue())
+ auto Entry = ConfigOpts.Config.find(C.first);
+ if (Entry != ConfigOpts.Config.end() && Entry->second == C.second)
continue;
- GenerateArg(Args, OPT_analyzer_config, C.getKey() + "=" + C.getValue(), SA);
+ GenerateArg(Args, OPT_analyzer_config, C.first + "=" + C.second, SA);
}
// Nothing to generate for FullCompilerInvocation.
@@ -1019,7 +1019,7 @@
}
A->claim();
- Opts.Config[key] = std::string(val);
+ Opts.Config[key.str()] = val.str();
// FIXME: Remove this hunk after clang-17 released.
constexpr auto SingleFAM =
@@ -1050,7 +1050,7 @@
static StringRef getStringOption(AnalyzerOptions::ConfigTable &Config,
StringRef OptionName, StringRef DefaultVal) {
- return Config.insert({OptionName, std::string(DefaultVal)}).first->second;
+ return Config.insert({OptionName.str(), DefaultVal.str()}).first->second;
}
static void initOption(AnalyzerOptions::ConfigTable &Config,
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -148,7 +148,7 @@
/// 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!
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -345,7 +345,7 @@
if (!OptName.consume_front(AnalyzerPrefix))
continue;
// Analyzer options are always local options so we can ignore priority.
- AnalyzerOptions.Config[OptName] = Opt.getValue().Value;
+ AnalyzerOptions.Config[OptName.str()] = Opt.getValue().Value;
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
