Skip to content

Commit

Permalink
[clang-tidy] Merge options inplace instead of copying
Browse files Browse the repository at this point in the history
Changed `ClangTidyOptions::mergeWith` to operate on the instance instead of returning a copy. The old mergeWith method has been renamed to merge and marked as nodiscard, to aid in disambiguating which one is which.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D91184
  • Loading branch information
njames93 committed Nov 12, 2020
1 parent b336826 commit 06db8f9
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 26 deletions.
Expand Up @@ -205,7 +205,7 @@ const ClangTidyOptions &ClangTidyContext::getOptions() const {
ClangTidyOptions ClangTidyContext::getOptionsForFile(StringRef File) const {
// Merge options on top of getDefaults() as a safeguard against options with
// unset values.
return ClangTidyOptions::getDefaults().mergeWith(
return ClangTidyOptions::getDefaults().merge(
OptionsProvider->getOptions(File), 0);
}

Expand Down
40 changes: 22 additions & 18 deletions clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
Expand Up @@ -116,7 +116,7 @@ ClangTidyOptions ClangTidyOptions::getDefaults() {
Options.User = llvm::None;
for (const ClangTidyModuleRegistry::entry &Module :
ClangTidyModuleRegistry::entries())
Options = Options.mergeWith(Module.instantiate()->getModuleOptions(), 0);
Options.mergeWith(Module.instantiate()->getModuleOptions(), 0);
return Options;
}

Expand All @@ -142,27 +142,31 @@ static void overrideValue(Optional<T> &Dest, const Optional<T> &Src) {
Dest = Src;
}

ClangTidyOptions ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
unsigned Priority) const {
ClangTidyOptions Result = *this;

mergeCommaSeparatedLists(Result.Checks, Other.Checks);
mergeCommaSeparatedLists(Result.WarningsAsErrors, Other.WarningsAsErrors);
overrideValue(Result.HeaderFilterRegex, Other.HeaderFilterRegex);
overrideValue(Result.SystemHeaders, Other.SystemHeaders);
overrideValue(Result.FormatStyle, Other.FormatStyle);
overrideValue(Result.User, Other.User);
overrideValue(Result.UseColor, Other.UseColor);
mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
mergeVectors(Result.ExtraArgsBefore, Other.ExtraArgsBefore);
ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
unsigned Order) {
mergeCommaSeparatedLists(Checks, Other.Checks);
mergeCommaSeparatedLists(WarningsAsErrors, Other.WarningsAsErrors);
overrideValue(HeaderFilterRegex, Other.HeaderFilterRegex);
overrideValue(SystemHeaders, Other.SystemHeaders);
overrideValue(FormatStyle, Other.FormatStyle);
overrideValue(User, Other.User);
overrideValue(UseColor, Other.UseColor);
mergeVectors(ExtraArgs, Other.ExtraArgs);
mergeVectors(ExtraArgsBefore, Other.ExtraArgsBefore);

for (const auto &KeyValue : Other.CheckOptions) {
Result.CheckOptions.insert_or_assign(
CheckOptions.insert_or_assign(
KeyValue.getKey(),
ClangTidyValue(KeyValue.getValue().Value,
KeyValue.getValue().Priority + Priority));
KeyValue.getValue().Priority + Order));
}
return *this;
}

ClangTidyOptions ClangTidyOptions::merge(const ClangTidyOptions &Other,
unsigned Order) const {
ClangTidyOptions Result = *this;
Result.mergeWith(Other, Order);
return Result;
}

Expand All @@ -178,8 +182,8 @@ ClangTidyOptions
ClangTidyOptionsProvider::getOptions(llvm::StringRef FileName) {
ClangTidyOptions Result;
unsigned Priority = 0;
for (const auto &Source : getRawOptions(FileName))
Result = Result.mergeWith(Source.first, ++Priority);
for (auto &Source : getRawOptions(FileName))
Result.mergeWith(Source.first, ++Priority);
return Result;
}

Expand Down
8 changes: 6 additions & 2 deletions clang-tools-extra/clang-tidy/ClangTidyOptions.h
Expand Up @@ -55,11 +55,15 @@ struct ClangTidyOptions {
/// of each registered \c ClangTidyModule.
static ClangTidyOptions getDefaults();

/// Overwrites all fields in here by the fields of \p Other that have a value.
/// \p Order specifies precedence of \p Other option.
ClangTidyOptions &mergeWith(const ClangTidyOptions &Other, unsigned Order);

/// Creates a new \c ClangTidyOptions instance combined from all fields
/// of this instance overridden by the fields of \p Other that have a value.
/// \p Order specifies precedence of \p Other option.
ClangTidyOptions mergeWith(const ClangTidyOptions &Other,
unsigned Order) const;
LLVM_NODISCARD ClangTidyOptions merge(const ClangTidyOptions &Other,
unsigned Order) const;

/// Checks filter.
llvm::Optional<std::string> Checks;
Expand Down
7 changes: 3 additions & 4 deletions clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
Expand Up @@ -319,7 +319,7 @@ static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
if (ParsedConfig)
return std::make_unique<ConfigOptionsProvider>(
GlobalOptions,
ClangTidyOptions::getDefaults().mergeWith(DefaultOptions, 0),
ClangTidyOptions::getDefaults().merge(DefaultOptions, 0),
*ParsedConfig, OverrideOptions, std::move(FS));
llvm::errs() << "Error: invalid configuration specified.\n"
<< ParsedConfig.getError().message() << "\n";
Expand Down Expand Up @@ -455,9 +455,8 @@ int clangTidyMain(int argc, const char **argv) {
if (DumpConfig) {
EffectiveOptions.CheckOptions =
getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
llvm::outs() << configurationAsText(
ClangTidyOptions::getDefaults().mergeWith(
EffectiveOptions, 0))
llvm::outs() << configurationAsText(ClangTidyOptions::getDefaults().merge(
EffectiveOptions, 0))
<< "\n";
return 0;
}
Expand Down
Expand Up @@ -103,7 +103,7 @@ TEST(ParseConfiguration, MergeConfigurations) {
UseColor: true
)");
ASSERT_TRUE(!!Options2);
ClangTidyOptions Options = Options1->mergeWith(*Options2, 0);
ClangTidyOptions Options = Options1->merge(*Options2, 0);
EXPECT_EQ("check1,check2,check3,check4", *Options.Checks);
EXPECT_EQ("filter2", *Options.HeaderFilterRegex);
EXPECT_EQ("user2", *Options.User);
Expand Down

0 comments on commit 06db8f9

Please sign in to comment.