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
This commit is contained in:
Ben Boeckel
2020-04-13 11:26:02 -04:00
parent 4e9e7c713e
commit f2a33107be
20 changed files with 59 additions and 87 deletions
@@ -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;
+2
View File
@@ -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;
+13 -27
View File
@@ -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])) {
} else if (vgMPK1.find(line) || vgMPK2.find(line)) {
failure = cmCTestMemCheckHandler::MPK;
} else if (vgMPK2.find(lines[cc])) {
failure = cmCTestMemCheckHandler::MPK;
} else if (vgUMC.find(lines[cc])) {
} else if (vgUMC.find(line)) {
failure = cmCTestMemCheckHandler::UMC;
} else if (vgUMR1.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 (vgUMR2.find(lines[cc])) {
failure = cmCTestMemCheckHandler::UMR;
} else if (vgUMR3.find(lines[cc])) {
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]++;
+3 -4
View File
@@ -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);
+5 -7
View File
@@ -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)) {
-1
View File
@@ -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;
+1 -3
View File
@@ -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);
+1 -3
View File
@@ -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;
-1
View File
@@ -981,7 +981,6 @@ void cmExtraEclipseCDT4Generator::CreateCProjectFile() const
}
} break;
case cmStateEnums::INTERFACE_LIBRARY:
break;
default:
break;
}
+2 -3
View File
@@ -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;
}
}
+6
View File
@@ -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);
+5 -3
View File
@@ -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")) {
-1
View File
@@ -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.
+4 -6
View File
@@ -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
+1 -4
View File
@@ -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;
}
+2
View File
@@ -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:
+2
View File
@@ -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);
+2
View File
@@ -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;
+8 -17
View File
@@ -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++;
+2 -3
View File
@@ -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;