[clang-tidy] Merge options inplace instead of copying

[njames93]

Summary

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.

[aaron.ballman]

LGTM!

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

 
 const ClangTidyOptions &ClangTidyContext::getOptions() const {
   return CurrentOptions;
 }
 
 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);
 }
 
 void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }
 
 void ClangTidyContext::setProfileStoragePrefix(StringRef Prefix) {
   ProfilePrefix = std::string(Prefix);
 }

clang-tools-extra/clang-tidy/ClangTidyOptions.h

struct ClangTidyOptions {
   /// These options are used for all settings that haven't been
   /// overridden by the \c OptionsProvider.
   ///
   /// Allow no checks and no headers by default. This method initializes
   /// check-specific options by calling \c ClangTidyModule::getModuleOptions()
   /// 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,
+  LLVM_NODISCARD ClangTidyOptions merge(const ClangTidyOptions &Other,
                                         unsigned Order) const;
 
   /// Checks filter.
   llvm::Optional<std::string> Checks;
 
   /// WarningsAsErrors filter.
   llvm::Optional<std::string> WarningsAsErrors;
 
   /// Output warnings from headers matching this filter. Warnings from

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
+#include <iterator>
 #include <utility>
 
 #define DEBUG_TYPE "clang-tidy-options"
 
 using clang::tidy::ClangTidyOptions;
 using clang::tidy::FileFilter;
 using OptionsSource = clang::tidy::ClangTidyOptionsProvider::OptionsSource;
 
ClangTidyOptions ClangTidyOptions::getDefaults() {
   Options.Checks = "";
   Options.WarningsAsErrors = "";
   Options.HeaderFilterRegex = "";
   Options.SystemHeaders = false;
   Options.FormatStyle = "none";
   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;
 }
 
 template <typename T>
 static void mergeVectors(Optional<T> &Dest, const Optional<T> &Src) {
   if (Src) {
     if (Dest)
       Dest->insert(Dest->end(), Src->begin(), Src->end());

 }
 
 template <typename T>
 static void overrideValue(Optional<T> &Dest, const Optional<T> &Src) {
   if (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;
 }
 
 const char ClangTidyOptionsProvider::OptionsSourceTypeDefaultBinary[] =
     "clang-tidy binary";
 const char ClangTidyOptionsProvider::OptionsSourceTypeCheckCommandLineOption[] =
     "command-line option '-checks'";
 const char
     ClangTidyOptionsProvider::OptionsSourceTypeConfigCommandLineOption[] =
         "command-line option '-config'";
 
 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;
 }
 
 std::vector<OptionsSource>
 DefaultOptionsProvider::getRawOptions(llvm::StringRef FileName) {
   std::vector<OptionsSource> Result;
   Result.emplace_back(DefaultOptions, OptionsSourceTypeDefaultBinary);
   return Result;

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
 
   auto LoadConfig = [&](StringRef Configuration)
       -> std::unique_ptr<ClangTidyOptionsProvider> {
     llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
         parseConfiguration(Configuration);
     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";
     return nullptr;
   };
 
   if (ConfigFile.getNumOccurrences() > 0) {
     if (Config.getNumOccurrences() > 0) {
for (const auto &CheckName : EnabledChecks)
       llvm::outs() << "\n    " << CheckName;
     llvm::outs() << "\n\n";
     return 0;
   }
 
   if (DumpConfig) {
     EffectiveOptions.CheckOptions =
         getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
-    llvm::outs() << configurationAsText(
-                        ClangTidyOptions::getDefaults().mergeWith(
-                            EffectiveOptions, 0))
+    llvm::outs() << configurationAsText(ClangTidyOptions::getDefaults().merge(
                  << "\n";
     return 0;
   }
 
   if (EnabledChecks.empty()) {
     llvm::errs() << "Error: no checks enabled.\n";
     llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
     return 1;

clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

llvm::ErrorOr<ClangTidyOptions> Options2 = parseConfiguration(R"(
       HeaderFilterRegex: "filter2"
       AnalyzeTemporaryDtors: false
       User: user2
       ExtraArgs: ['arg3', 'arg4']
       ExtraArgsBefore: ['arg-before3', 'arg-before4']
       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);
   ASSERT_TRUE(Options.ExtraArgs.hasValue());
   EXPECT_EQ("arg1,arg2,arg3,arg4", llvm::join(Options.ExtraArgs->begin(),
                                               Options.ExtraArgs->end(), ","));
   ASSERT_TRUE(Options.ExtraArgsBefore.hasValue());
   EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",