clang-tidy: address bugprone-branch-clone lints

Arguably, many of these are bugs in `clang-tidy`. An if/else tree with
other conditionals between cloned blocks may be relying on the
intermediate logic to fall out of the case and inverting this logic may
be non-trivial.

See: https://bugs.llvm.org/show_bug.cgi?id=44165

20 files changed, 59 insertions, 87 deletions

diff --git a/Source/CPack/IFW/cmCPackIFWRepository.cxx b/Source/CPack/IFW/cmCPackIFWRepository.cxx
index a696549..f5e8744 100644
--- a/Source/CPack/IFW/cmCPackIFWRepository.cxx
+++ b/Source/CPack/IFW/cmCPackIFWRepository.cxx
@@ -21,11 +21,7 @@ bool cmCPackIFWRepository::IsValid() const
 
   switch (this->Update) {
     case cmCPackIFWRepository::None:
-      valid = !this->Url.empty();
-      break;
     case cmCPackIFWRepository::Add:
-      valid = !this->Url.empty();
-      break;
     case cmCPackIFWRepository::Remove:
       valid = !this->Url.empty();
       break;
diff --git a/Source/CTest/cmCTestCVS.cxx b/Source/CTest/cmCTestCVS.cxx
index 45ec390..75b534b 100644
--- a/Source/CTest/cmCTestCVS.cxx
+++ b/Source/CTest/cmCTestCVS.cxx
@@ -152,6 +152,8 @@ private:
         this->FinishRevision();
       }
     } else if (this->Section == SectionRevisions) {
+      // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+      // NOLINTNEXTLINE(bugprone-branch-clone)
       if (!this->Rev.Log.empty()) {
         // Continue the existing log.
         this->Rev.Log += this->Line;
diff --git a/Source/CTest/cmCTestMemCheckHandler.cxx b/Source/CTest/cmCTestMemCheckHandler.cxx
index c1ecaf1..7ad87f5 100644
--- a/Source/CTest/cmCTestMemCheckHandler.cxx
+++ b/Source/CTest/cmCTestMemCheckHandler.cxx
@@ -957,35 +957,25 @@ bool cmCTestMemCheckHandler::ProcessMemCheckValgrindOutput(
                          "valgrind  line " << lines[cc] << std::endl,
                          this->Quiet);
       int failure = cmCTestMemCheckHandler::NO_MEMORY_FAULT;
-      if (vgFIM.find(lines[cc])) {
+      auto& line = lines[cc];
+      if (vgFIM.find(line)) {
         failure = cmCTestMemCheckHandler::FIM;
-      } else if (vgFMM.find(lines[cc])) {
+      } else if (vgFMM.find(line)) {
         failure = cmCTestMemCheckHandler::FMM;
-      } else if (vgMLK1.find(lines[cc])) {
+      } else if (vgMLK1.find(line) || vgMLK2.find(line)) {
         failure = cmCTestMemCheckHandler::MLK;
-      } else if (vgMLK2.find(lines[cc])) {
-        failure = cmCTestMemCheckHandler::MLK;
-      } else if (vgPAR.find(lines[cc])) {
+      } else if (vgPAR.find(line)) {
         failure = cmCTestMemCheckHandler::PAR;
-      } else if (vgMPK1.find(lines[cc])) {
-        failure = cmCTestMemCheckHandler::MPK;
-      } else if (vgMPK2.find(lines[cc])) {
+      } else if (vgMPK1.find(line) || vgMPK2.find(line)) {
         failure = cmCTestMemCheckHandler::MPK;
-      } else if (vgUMC.find(lines[cc])) {
+      } else if (vgUMC.find(line)) {
         failure = cmCTestMemCheckHandler::UMC;
-      } else if (vgUMR1.find(lines[cc])) {
-        failure = cmCTestMemCheckHandler::UMR;
-      } else if (vgUMR2.find(lines[cc])) {
-        failure = cmCTestMemCheckHandler::UMR;
-      } else if (vgUMR3.find(lines[cc])) {
+      } else if (vgUMR1.find(line) || vgUMR2.find(line) || vgUMR3.find(line) ||
+                 vgUMR4.find(line) || vgUMR5.find(line)) {
         failure = cmCTestMemCheckHandler::UMR;
-      } else if (vgUMR4.find(lines[cc])) {
-        failure = cmCTestMemCheckHandler::UMR;
-      } else if (vgUMR5.find(lines[cc])) {
-        failure = cmCTestMemCheckHandler::UMR;
-      } else if (vgIPW.find(lines[cc])) {
+      } else if (vgIPW.find(line)) {
         failure = cmCTestMemCheckHandler::IPW;
-      } else if (vgABR.find(lines[cc])) {
+      } else if (vgABR.find(line)) {
         failure = cmCTestMemCheckHandler::ABR;
       }
 
@@ -1049,13 +1039,9 @@ bool cmCTestMemCheckHandler::ProcessMemCheckDrMemoryOutput(
     ostr << l << std::endl;
     if (drMemoryError.find(l)) {
       defects++;
-      if (unaddressableAccess.find(l)) {
+      if (unaddressableAccess.find(l) || uninitializedRead.find(l)) {
         results[cmCTestMemCheckHandler::UMR]++;
-      } else if (uninitializedRead.find(l)) {
-        results[cmCTestMemCheckHandler::UMR]++;
-      } else if (leak.find(l)) {
-        results[cmCTestMemCheckHandler::MLK]++;
-      } else if (handleLeak.find(l)) {
+      } else if (leak.find(l) || handleLeak.find(l)) {
         results[cmCTestMemCheckHandler::MLK]++;
       } else if (invalidHeapArgument.find(l)) {
         results[cmCTestMemCheckHandler::FMM]++;
diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx
index f9850ef..ca65946 100644
--- a/Source/CTest/cmCTestTestHandler.cxx
+++ b/Source/CTest/cmCTestTestHandler.cxx
@@ -2402,10 +2402,9 @@ bool cmCTestTestHandler::AddTest(const std::vector<std::string>& args)
   test.SkipReturnCode = -1;
   test.PreviousRuns = 0;
   if (this->UseIncludeRegExpFlag &&
-      !this->IncludeTestsRegularExpression.find(testname)) {
-    test.IsInBasedOnREOptions = false;
-  } else if (this->UseExcludeRegExpFlag && !this->UseExcludeRegExpFirst &&
-             this->ExcludeTestsRegularExpression.find(testname)) {
+      (!this->IncludeTestsRegularExpression.find(testname) ||
+       (!this->UseExcludeRegExpFirst &&
+        this->ExcludeTestsRegularExpression.find(testname)))) {
     test.IsInBasedOnREOptions = false;
   }
   this->TestList.push_back(test);
diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx
index c5505f9..35a8952 100644
--- a/Source/cmCTest.cxx
+++ b/Source/cmCTest.cxx
@@ -1941,13 +1941,11 @@ bool cmCTest::HandleCommandLineArguments(size_t& i,
   else if (this->CheckArgument(arg, "--debug"_s)) {
     this->Impl->Debug = true;
     this->Impl->ShowLineNumbers = true;
-  } else if (this->CheckArgument(arg, "--group"_s) && i < args.size() - 1) {
-    i++;
-    this->Impl->SpecificGroup = args[i];
-  }
-  // This is an undocumented / deprecated option.
-  // "Track" has been renamed to "Group".
-  else if (this->CheckArgument(arg, "--track"_s) && i < args.size() - 1) {
+  } else if ((this->CheckArgument(arg, "--group"_s) ||
+              // This is an undocumented / deprecated option.
+              // "Track" has been renamed to "Group".
+              this->CheckArgument(arg, "--track"_s)) &&
+             i < args.size() - 1) {
     i++;
     this->Impl->SpecificGroup = args[i];
   } else if (this->CheckArgument(arg, "--show-line-numbers"_s)) {
diff --git a/Source/cmComputeLinkInformation.cxx b/Source/cmComputeLinkInformation.cxx
index 8d27699..de81f13 100644
--- a/Source/cmComputeLinkInformation.cxx
+++ b/Source/cmComputeLinkInformation.cxx
@@ -1440,7 +1440,6 @@ void cmComputeLinkInformation::HandleBadFullItem(std::string const& item,
     }
     case cmPolicies::OLD:
       // OLD behavior does not warn.
-      break;
     case cmPolicies::NEW:
       // NEW behavior will not get here.
       break;
diff --git a/Source/cmDependsFortran.cxx b/Source/cmDependsFortran.cxx
index 666306c..c45cd1c 100644
--- a/Source/cmDependsFortran.cxx
+++ b/Source/cmDependsFortran.cxx
@@ -29,12 +29,10 @@ static void cmFortranModuleAppendUpperLower(std::string const& mod,
                                             std::string& mod_lower)
 {
   std::string::size_type ext_len = 0;
-  if (cmHasLiteralSuffix(mod, ".mod")) {
+  if (cmHasLiteralSuffix(mod, ".mod") || cmHasLiteralSuffix(mod, ".sub")) {
     ext_len = 4;
   } else if (cmHasLiteralSuffix(mod, ".smod")) {
     ext_len = 5;
-  } else if (cmHasLiteralSuffix(mod, ".sub")) {
-    ext_len = 4;
   }
   std::string const& name = mod.substr(0, mod.size() - ext_len);
   std::string const& ext = mod.substr(mod.size() - ext_len);
diff --git a/Source/cmExtraCodeLiteGenerator.cxx b/Source/cmExtraCodeLiteGenerator.cxx
index de40c77..bf7555d 100644
--- a/Source/cmExtraCodeLiteGenerator.cxx
+++ b/Source/cmExtraCodeLiteGenerator.cxx
@@ -204,9 +204,7 @@ std::string cmExtraCodeLiteGenerator::CollectSourceFiles(
     case cmStateEnums::STATIC_LIBRARY: {
       projectType = "Static Library";
     } break;
-    case cmStateEnums::SHARED_LIBRARY: {
-      projectType = "Dynamic Library";
-    } break;
+    case cmStateEnums::SHARED_LIBRARY:
     case cmStateEnums::MODULE_LIBRARY: {
       projectType = "Dynamic Library";
     } break;
diff --git a/Source/cmExtraEclipseCDT4Generator.cxx b/Source/cmExtraEclipseCDT4Generator.cxx
index 1c5bcaa..b65f097 100644
--- a/Source/cmExtraEclipseCDT4Generator.cxx
+++ b/Source/cmExtraEclipseCDT4Generator.cxx
@@ -981,7 +981,6 @@ void cmExtraEclipseCDT4Generator::CreateCProjectFile() const
           }
         } break;
         case cmStateEnums::INTERFACE_LIBRARY:
-          break;
         default:
           break;
       }
diff --git a/Source/cmExtraKateGenerator.cxx b/Source/cmExtraKateGenerator.cxx
index 271bbee..de979e2 100644
--- a/Source/cmExtraKateGenerator.cxx
+++ b/Source/cmExtraKateGenerator.cxx
@@ -129,9 +129,8 @@ void cmExtraKateGenerator::WriteTargets(const cmLocalGenerator& lg,
             if (targetName == "edit_cache") {
               const char* editCommand =
                 localGen->GetMakefile()->GetDefinition("CMAKE_EDIT_COMMAND");
-              if (editCommand == nullptr) {
-                insertTarget = false;
-              } else if (strstr(editCommand, "ccmake") != nullptr) {
+              if (editCommand == nullptr ||
+                  strstr(editCommand, "ccmake") != nullptr) {
                 insertTarget = false;
               }
             }
diff --git a/Source/cmFindPackageCommand.cxx b/Source/cmFindPackageCommand.cxx
index e996327..d1517fe 100644
--- a/Source/cmFindPackageCommand.cxx
+++ b/Source/cmFindPackageCommand.cxx
@@ -279,9 +279,13 @@ bool cmFindPackageCommand::InitialPass(std::vector<std::string> const& args)
     } else if (args[i] == "MODULE") {
       moduleArgs.insert(i);
       doing = DoingNone;
+      // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+      // NOLINTNEXTLINE(bugprone-branch-clone)
     } else if (args[i] == "CONFIG") {
       configArgs.insert(i);
       doing = DoingNone;
+      // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+      // NOLINTNEXTLINE(bugprone-branch-clone)
     } else if (args[i] == "NO_MODULE") {
       configArgs.insert(i);
       doing = DoingNone;
@@ -318,6 +322,8 @@ bool cmFindPackageCommand::InitialPass(std::vector<std::string> const& args)
       this->NoSystemRegistry = true;
       configArgs.insert(i);
       doing = DoingNone;
+      // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+      // NOLINTNEXTLINE(bugprone-branch-clone)
     } else if (args[i] == "NO_CMAKE_BUILDS_PATH") {
       // Ignore legacy option.
       configArgs.insert(i);
diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx
index 611f7b0..93eb8fb 100644
--- a/Source/cmGeneratorTarget.cxx
+++ b/Source/cmGeneratorTarget.cxx
@@ -1227,7 +1227,6 @@ std::string cmGeneratorTarget::EvaluateInterfaceProperty(
       return result;
     case cmGeneratorExpressionDAGChecker::CYCLIC_REFERENCE:
       // No error. We just skip cyclic references.
-      return result;
     case cmGeneratorExpressionDAGChecker::ALREADY_SEEN:
       // No error. We have already seen this transitive property.
       return result;
@@ -1295,10 +1294,9 @@ std::string AddSwiftInterfaceIncludeDirectories(
       dag.ReportError(nullptr,
                       "$<TARGET_PROPERTY:" + target->GetName() +
                         ",Swift_MODULE_DIRECTORY>");
-      return "";
+      CM_FALLTHROUGH;
     case cmGeneratorExpressionDAGChecker::CYCLIC_REFERENCE:
       // No error. We just skip cyclic references.
-      return "";
     case cmGeneratorExpressionDAGChecker::ALREADY_SEEN:
       // No error. We have already seen this transitive property.
       return "";
@@ -1689,10 +1687,14 @@ void cmGeneratorTarget::ComputeKindedSources(KindedSources& files,
     std::string ext = cmSystemTools::LowerCase(sf->GetExtension());
     if (sf->GetCustomCommand()) {
       kind = SourceKindCustomCommand;
+      // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+      // NOLINTNEXTLINE(bugprone-branch-clone)
     } else if (this->Target->GetType() == cmStateEnums::UTILITY) {
       kind = SourceKindExtra;
     } else if (this->IsSourceFilePartOfUnityBatch(sf->ResolveFullPath())) {
       kind = SourceKindUnityBatched;
+      // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+      // NOLINTNEXTLINE(bugprone-branch-clone)
     } else if (sf->GetPropertyAsBool("HEADER_FILE_ONLY")) {
       kind = SourceKindHeader;
     } else if (sf->GetPropertyAsBool("EXTERNAL_OBJECT")) {
diff --git a/Source/cmInstallCommand.cxx b/Source/cmInstallCommand.cxx
index 704db66..c9b22b6 100644
--- a/Source/cmInstallCommand.cxx
+++ b/Source/cmInstallCommand.cxx
@@ -661,7 +661,6 @@ bool HandleTargetsMode(std::vector<std::string> const& args,
         // Nothing to do. An INTERFACE_LIBRARY can be installed, but the
         // only effect of that is to make it exportable. It installs no
         // other files itself.
-        break;
       default:
         // This should never happen due to the above type check.
         // Ignore the case.
diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index f5ca5f4..e6083d3 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -2070,11 +2070,9 @@ bool cmLocalGenerator::GetRealDependency(const std::string& inName,
       case cmStateEnums::OBJECT_LIBRARY:
         // An object library has no single file on which to depend.
         // This was listed to get the target-level dependency.
-        return false;
       case cmStateEnums::INTERFACE_LIBRARY:
         // An interface library has no file on which to depend.
         // This was listed to get the target-level dependency.
-        return false;
       case cmStateEnums::UTILITY:
       case cmStateEnums::GLOBAL_TARGET:
         // A utility target has no file on which to depend.  This was listed
@@ -3389,11 +3387,12 @@ std::string cmLocalGenerator::GetObjectFileNameWithoutTarget(
   // Select a nice-looking reference to the source file to construct
   // the object file name.
   std::string objectName;
+  // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+  // NOLINTNEXTLINE(bugprone-branch-clone)
   if ((relSource && !relBinary) || (subSource && !subBinary)) {
     objectName = relFromSource;
-  } else if ((relBinary && !relSource) || (subBinary && !subSource)) {
-    objectName = relFromBinary;
-  } else if (relFromBinary.length() < relFromSource.length()) {
+  } else if ((relBinary && !relSource) || (subBinary && !subSource) ||
+             relFromBinary.length() < relFromSource.length()) {
     objectName = relFromBinary;
   } else {
     objectName = relFromSource;
@@ -3552,7 +3551,6 @@ bool cmLocalGenerator::NeedBackwardsCompatibility_2_4()
       break;
     case cmPolicies::NEW:
       // New behavior is to ignore the variable.
-      return false;
     case cmPolicies::REQUIRED_IF_USED:
     case cmPolicies::REQUIRED_ALWAYS:
       // This will never be the case because the only way to require
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 69f316d..0b532e7 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -2513,10 +2513,7 @@ void cmMakefile::ExpandVariablesCMP0019()
 
     for (auto l = linkLibs.begin(); l != linkLibs.end(); ++l) {
       std::string libName = *l;
-      if (libName == "optimized") {
-        ++l;
-        libName = *l;
-      } else if (libName == "debug") {
+      if (libName == "optimized" || libName == "debug") {
         ++l;
         libName = *l;
       }
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index 5c8bc98..dea3f8a 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -72,6 +72,7 @@ static const char* idToVersion(cmPolicies::PolicyID id)
 #define POLICY_CASE(ID, V_MAJOR, V_MINOR, V_PATCH)                            \
   case cmPolicies::ID:                                                        \
     return #V_MAJOR "." #V_MINOR "." #V_PATCH;
+    // NOLINTNEXTLINE(bugprone-branch-clone)
     CM_FOR_EACH_POLICY_ID_VERSION(POLICY_CASE)
 #undef POLICY_CASE
     case cmPolicies::CMPCOUNT:
@@ -90,6 +91,7 @@ static bool isPolicyNewerThan(cmPolicies::PolicyID id, unsigned int majorV,
             (majorV == (V_MAJOR) && minorV + 1 < (V_MINOR) + 1) ||            \
             (majorV == (V_MAJOR) && minorV == (V_MINOR) &&                    \
              patchV + 1 < (V_PATCH) + 1));
+    // NOLINTNEXTLINE(bugprone-branch-clone)
     CM_FOR_EACH_POLICY_ID_VERSION(POLICY_CASE)
 #undef POLICY_CASE
     case cmPolicies::CMPCOUNT:
diff --git a/Source/cmRST.cxx b/Source/cmRST.cxx
index c39d162..26e93bb 100644
--- a/Source/cmRST.cxx
+++ b/Source/cmRST.cxx
@@ -166,6 +166,8 @@ void cmRST::ProcessLine(std::string const& line)
     this->Markup =
       (line.find_first_not_of(" \t", 2) == std::string::npos ? MarkupEmpty
                                                              : MarkupNormal);
+    // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+    // NOLINTNEXTLINE(bugprone-branch-clone)
     if (this->CMakeDirective.find(line)) {
       // Output cmake domain directives and their content normally.
       this->NormalLine(line);
diff --git a/Source/cmSeparateArgumentsCommand.cxx b/Source/cmSeparateArgumentsCommand.cxx
index 52bde7c..cfe3087 100644
--- a/Source/cmSeparateArgumentsCommand.cxx
+++ b/Source/cmSeparateArgumentsCommand.cxx
@@ -39,6 +39,8 @@ bool cmSeparateArgumentsCommand(std::vector<std::string> const& args,
     if (doing == DoingVariable) {
       var = arg;
       doing = DoingMode;
+      // This will always clone one of the other blocks.
+      // NOLINTNEXTLINE(bugprone-branch-clone)
     } else if (doing == DoingMode && arg == "NATIVE_COMMAND") {
 #ifdef _WIN32
       mode = ModeWindows;
diff --git a/Source/cmake.cxx b/Source/cmake.cxx
index 2ec893f..ea1d8ca 100644
--- a/Source/cmake.cxx
+++ b/Source/cmake.cxx
@@ -642,6 +642,8 @@ void cmake::SetArgs(const std::vector<std::string>& args)
       path = cmSystemTools::CollapseFullPath(path);
       cmSystemTools::ConvertToUnixSlashes(path);
       this->SetHomeDirectory(path);
+      // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+      // NOLINTNEXTLINE(bugprone-branch-clone)
     } else if (cmHasLiteralPrefix(arg, "-O")) {
       // There is no local generate anymore.  Ignore -O option.
     } else if (cmHasLiteralPrefix(arg, "-B")) {
@@ -681,27 +683,16 @@ void cmake::SetArgs(const std::vector<std::string>& args)
       this->VSSolutionFile = args[++i];
     }
 #endif
-    else if (cmHasLiteralPrefix(arg, "-D")) {
+    else if (cmHasLiteralPrefix(arg, "-D") || cmHasLiteralPrefix(arg, "-U") ||
+             cmHasLiteralPrefix(arg, "-C")) {
       // skip for now
-      // in case '-D var=val' is given, also skip the next
-      // in case '-Dvar=val' is given, don't skip the next
-      if (arg.size() == 2) {
-        ++i;
-      }
-    } else if (cmHasLiteralPrefix(arg, "-U")) {
-      // skip for now
-      // in case '-U var' is given, also skip the next
-      // in case '-Uvar' is given, don't skip the next
-      if (arg.size() == 2) {
-        ++i;
-      }
-    } else if (cmHasLiteralPrefix(arg, "-C")) {
-      // skip for now
-      // in case '-C path' is given, also skip the next
-      // in case '-Cpath' is given, don't skip the next
+      // in case '-[DUC] argval' var' is given, also skip the next
+      // in case '-[DUC]argval' is given, don't skip the next
       if (arg.size() == 2) {
         ++i;
       }
+      // XXX(clang-tidy): https://bugs.llvm.org/show_bug.cgi?id=44165
+      // NOLINTNEXTLINE(bugprone-branch-clone)
     } else if (cmHasLiteralPrefix(arg, "-P")) {
       // skip for now
       i++;
diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx
index c3bd160..49d04a6 100644
--- a/Source/cmcmd.cxx
+++ b/Source/cmcmd.cxx
@@ -1972,9 +1972,8 @@ bool cmVSLink::Parse(std::vector<std::string>::const_iterator argBeg,
 
   // Parse the link command to extract information we need.
   for (; arg != argEnd; ++arg) {
-    if (cmSystemTools::Strucmp(arg->c_str(), "/INCREMENTAL:YES") == 0) {
-      this->Incremental = true;
-    } else if (cmSystemTools::Strucmp(arg->c_str(), "/INCREMENTAL") == 0) {
+    if (cmSystemTools::Strucmp(arg->c_str(), "/INCREMENTAL:YES") == 0 ||
+        cmSystemTools::Strucmp(arg->c_str(), "/INCREMENTAL") == 0) {
       this->Incremental = true;
     } else if (cmSystemTools::Strucmp(arg->c_str(), "/MANIFEST:NO") == 0) {
       this->LinkGeneratesManifest = false;