From c89979223225f966912292578e7a7f54d34ef6d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 19 Nov 2018 21:23:36 +0100 Subject: [PATCH] Fixed #7619 (False positive: Redundant assignment) --- lib/checkother.cpp | 58 ++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 71f38deae..a03fe8596 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -473,6 +473,7 @@ static bool checkExceptionHandling(const Token* tok) void CheckOther::checkRedundantAssignment() { + // TODO: Rewrite this messy checker. const bool printPerformance = mSettings->isEnabled(Settings::PERFORMANCE); const bool printStyle = mSettings->isEnabled(Settings::STYLE); const bool printWarning = mSettings->isEnabled(Settings::WARNING); @@ -491,6 +492,7 @@ void CheckOther::checkRedundantAssignment() std::map memAssignments; std::map > membervars; std::set initialized; + std::map> dependencies; const Token* writtenArgumentsEnd = nullptr; for (const Token* tok = scope.bodyStart->next(); tok && tok != scope.bodyEnd; tok = tok->next()) { @@ -503,15 +505,18 @@ void CheckOther::checkRedundantAssignment() break; varAssignments.clear(); memAssignments.clear(); + dependencies.clear(); } else if (tok->str() == "{" && tok->strAt(-1) != "{" && tok->strAt(-1) != "=" && tok->strAt(-4) != "case" && tok->strAt(-3) != "default") { // conditional or non-executable inner scope: Skip it and reset status tok = tok->link(); varAssignments.clear(); memAssignments.clear(); + dependencies.clear(); } else if (Token::Match(tok, "for|if|while (")) { tok = tok->linkAt(1); } else if (Token::Match(tok, "break|return|continue|throw|goto|asm")) { varAssignments.clear(); memAssignments.clear(); + dependencies.clear(); } else if (Token::Match(tok, "%var% = [ & ] (")) { const unsigned int lambdaId = tok->varId(); const Token *lambdaParams = tok->tokAt(5); @@ -523,29 +528,38 @@ void CheckOther::checkRedundantAssignment() usedByLambda[lambdaId].insert(tok2->varId()); } } - } else if (tok->tokType() == Token::eVariable && !Token::Match(tok, "%name% (")) { - const Token *eq = nullptr; - for (const Token *tok2 = tok; tok2; tok2 = tok2->next()) { - if (Token::Match(tok2, "[([]")) { - // bail out if there is a variable in rhs - we only track 1 variable - bool bailout = false; - for (const Token *tok3 = tok2->link(); tok3 != tok2; tok3 = tok3->previous()) { - if (tok3->varId()) { - const Variable *var = tok3->variable(); - if (!var || !var->isConst() || var->isReference() || var->isPointer()) { - bailout = true; - break; - } - } - } - if (bailout) + } else if (tok->tokType() == Token::eVariable && !Token::Match(tok, "%var% (")) { + const Token *eq = tok; + while (Token::Match(eq->astParent(), ".|::|[")) + eq = eq->astParent(); + if (Token::simpleMatch(eq->astParent(), "=") && eq == eq->astParent()->astOperand1()) + eq = eq->astParent(); + else + eq = nullptr; + + if (eq) { + for (const Token *tok2 = tok->next(); tok2 != eq; tok2 = tok2->next()) { + if (tok2->str() == "[") + tok2 = tok2->link(); + else if (tok2->str() == "]") { + eq = nullptr; break; - tok2 = tok2->link(); - } else if (Token::Match(tok2, "[)];,]")) - break; - else if (tok2->str() == "=") { - eq = tok2; - break; + } + } + } + + if (eq) { + bool arrayIndex = false; + for (const Token *tok2 = tok->next(); tok2 != eq; tok2 = tok2->next()) { + if (tok2->str() == "[") + arrayIndex = true; + if (arrayIndex && tok2->varId() != 0) + dependencies[tok2->varId()].insert(tok->varId()); + } + + for (unsigned int varId : dependencies[tok->varId()]) { + varAssignments.erase(varId); + memAssignments.erase(varId); } }