[clang-tidy] Merge options inplace instead of copying
authorNathan James <n.james93@hotmail.co.uk>
Thu, 12 Nov 2020 18:19:11 +0000 (18:19 +0000)
committerNathan James <n.james93@hotmail.co.uk>
Thu, 12 Nov 2020 18:19:12 +0000 (18:19 +0000)
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

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/clang-tidy/ClangTidyOptions.h
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

index 2b137d0..e7b5b1d 100644 (file)
@@ -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);
 }
 
index 8c78f28..950a64f 100644 (file)
@@ -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;
 }
 
@@ -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;
 }
 
@@ -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;
 }
 
index 6bfcae0..11c2e65 100644 (file)
@@ -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;
index e046566..a4069da 100644 (file)
@@ -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";
@@ -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;
   }
index bfa5940..729dfa8 100644 (file)
@@ -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);