walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix issue 8413: Condition is always false 'i=expr; if (i != expr) {}' (#1295) 

* Follow variables when comparing same expression

* Remove assert include

* Dont follow function arguments

* Improve the checking to check more cases

* Add more tests

* Check if the variable is used inside a loop

* Follow both variables

* Only skip loops when variable is modified in scope

* Fix FP when followed variable is modified

* Dont follow arrays

* Skip pointer indirection

* Make recursive

* Improve checking more variables

* Fix test with sizeof

* Skip following operators

* Fix test when using sizeof

* Dont check every step

* Use early returns

* Update test to use a loop instead of conditional

* Add static

* Check variables are global

* Check local variables in another scope

* Fix issue with const pointers

* Distinguish between pointer indirection and multiply

* Use simple match

* Prevent crash with uniform initialization

* Use unary op and ast to detect pointer indirection

* Expand error message when expression do not match exactly

* Add errorpath to issameexpression

* Revert "Clarify warning message for 'Same expression on both sides of operator'"

This reverts commit 0e491b41a8.

* Check if the tokens are the same

* Report the operator and not the expressions
This commit is contained in:
Paul Fultz II 2018-08-07 02:32:16 -05:00 committed by Daniel Marjamäki
parent 20da3d2b46
commit f603b529df
6 changed files with 352 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

137
lib/astutils.cpp
View File

@ -133,9 +133,96 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
return ret; return ret;
} }
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure) static const Token * getVariableInitExpression(const Variable * var)
{ {
if (tok1 == nullptr && tok2 == nullptr) if(!var || !var->declEndToken())
return nullptr;
if(Token::Match(var->declEndToken(), "; %varid% =", var->declarationId()))
return var->declEndToken()->tokAt(2)->astOperand2();
return var->declEndToken()->astOperand2();
}
static bool isInLoopCondition(const Token * tok) {
return Token::Match(tok->astTop()->previous(), "for|while (");
}
/// This takes a token that refers to a variable and it will return the token
/// to the expression that the variable is assigned to. If its not valid to
/// make such substitution then it will return the original token.
static const Token * followVariableExpression(const Token * tok, bool cpp)
{
if(!tok)
return tok;
// Skip array access
if(Token::Match(tok, "%var% ["))
return tok;
// Skip pointer indirection
if(tok->astParent() && tok->isUnaryOp("*"))
return tok;
// Skip following variables if it is used in an assignment
if(Token::Match(tok->astParent(), "%assign%") || Token::Match(tok->next(), "%assign%"))
return tok;
const Variable * var = tok->variable();
const Token * varTok = getVariableInitExpression(var);
if(!varTok)
return tok;
// Skip array access
if(Token::simpleMatch(varTok, "["))
return tok;
if(!var->isLocal() && !var->isConst())
return tok;
if(var->isStatic() && !var->isConst())
return tok;
if(var->isArgument())
return tok;
// If this is in a loop then check if variables are modified in the entire scope
const Token * endToken = (isInLoopCondition(tok) || var->scope() != tok->scope()) ? var->scope()->bodyEnd : tok;
if (!var->isConst() && isVariableChanged(varTok, endToken, tok->varId(), false, nullptr, cpp))
return tok;
// Start at begining of initialization
const Token * startToken = varTok;
while(Token::Match(startToken, "%op%|.|(|{") && startToken->astOperand1())
startToken = startToken->astOperand1();
// Skip if the variable its referring to is modified
for(const Token * tok2 = startToken;tok2 != endToken;tok2 = tok2->next()) {
if(Token::simpleMatch(tok2, ";"))
break;
if(tok2->astParent() && tok2->isUnaryOp("*"))
return tok;
if (tok2->tokType() == Token::eIncDecOp ||
tok2->isAssignmentOp() ||
Token::Match(tok2, "%name% .|[|++|--|%assign%")) {
return tok;
}
if(const Variable * var2 = tok2->variable()) {
const Token * endToken2 = var2->scope() != tok->scope() ? var2->scope()->bodyEnd : endToken;
if(!var2->isLocal() && !var2->isConst() && !var2->isArgument())
return tok;
if(var2->isStatic() && !var2->isConst())
return tok;
if(!var2->isConst() && isVariableChanged(tok2, endToken2, tok2->varId(), false, nullptr, cpp))
return tok;
}
}
return varTok;
}
static void followVariableExpressionError(const Token *tok1, const Token *tok2, ErrorPath* errors)
{
if(!errors)
return;
if(!tok1)
return;
if(!tok2)
return;
errors->push_back(std::make_pair(tok2, "'" + tok1->str() + "' is assigned value '" + tok2->expressionString() + "' here."));
}
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, ErrorPath* errors)
{
if (tok1 == nullptr && tok2 == nullptr)
return true; return true;
if (tok1 == nullptr || tok2 == nullptr) if (tok1 == nullptr || tok2 == nullptr)
return false; return false;
@ -145,17 +232,36 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
if (tok2->str() == "." && tok2->astOperand1() && tok2->astOperand1()->str() == "this") if (tok2->str() == "." && tok2->astOperand1() && tok2->astOperand1()->str() == "this")
tok2 = tok2->astOperand2(); tok2 = tok2->astOperand2();
} }
// Skip double not
if (Token::simpleMatch(tok1, "!") && Token::simpleMatch(tok1->astOperand1(), "!") && !Token::simpleMatch(tok1->astParent(), "=")) { if (Token::simpleMatch(tok1, "!") && Token::simpleMatch(tok1->astOperand1(), "!") && !Token::simpleMatch(tok1->astParent(), "=")) {
return isSameExpression(cpp, macro, tok1->astOperand1()->astOperand1(), tok2, library, pure); return isSameExpression(cpp, macro, tok1->astOperand1()->astOperand1(), tok2, library, pure, errors);
} }
if (Token::simpleMatch(tok2, "!") && Token::simpleMatch(tok2->astOperand1(), "!") && !Token::simpleMatch(tok2->astParent(), "=")) { if (Token::simpleMatch(tok2, "!") && Token::simpleMatch(tok2->astOperand1(), "!") && !Token::simpleMatch(tok2->astParent(), "=")) {
return isSameExpression(cpp, macro, tok1, tok2->astOperand1()->astOperand1(), library, pure); return isSameExpression(cpp, macro, tok1, tok2->astOperand1()->astOperand1(), library, pure, errors);
}
// Follow variables if possible
if(tok1->str() != tok2->str() && (Token::Match(tok1, "%var%") || Token::Match(tok2, "%var%"))) {
const Token * varTok1 = followVariableExpression(tok1, cpp);
if (varTok1->str() == tok2->str()) {
followVariableExpressionError(tok1, varTok1, errors);
return isSameExpression(cpp, macro, varTok1, tok2, library, pure, errors);
}
const Token * varTok2 = followVariableExpression(tok2, cpp);
if(tok1->str() == varTok2->str()) {
followVariableExpressionError(tok2, varTok2, errors);
return isSameExpression(cpp, macro, tok1, varTok2, library, pure, errors);
}
if(varTok1->str() == varTok2->str()) {
followVariableExpressionError(tok1, varTok1, errors);
followVariableExpressionError(tok2, varTok2, errors);
return isSameExpression(cpp, macro, varTok1, varTok2, library, pure, errors);
}
} }
if (tok1->varId() != tok2->varId() || tok1->str() != tok2->str() || tok1->originalName() != tok2->originalName()) { if (tok1->varId() != tok2->varId() || tok1->str() != tok2->str() || tok1->originalName() != tok2->originalName()) {
if ((Token::Match(tok1,"<|>") && Token::Match(tok2,"<|>")) || if ((Token::Match(tok1,"<|>") && Token::Match(tok2,"<|>")) ||
(Token::Match(tok1,"<=|>=") && Token::Match(tok2,"<=|>="))) { (Token::Match(tok1,"<=|>=") && Token::Match(tok2,"<=|>="))) {
return isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure) && return isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, errors) &&
isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure); isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, errors);
} }
return false; return false;
} }
@ -233,9 +339,9 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
return false; return false;
} }
bool noncommutativeEquals = bool noncommutativeEquals =
isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand1(), library, pure); isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand1(), library, pure, errors);
noncommutativeEquals = noncommutativeEquals && noncommutativeEquals = noncommutativeEquals &&
isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand2(), library, pure); isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand2(), library, pure, errors);
if (noncommutativeEquals) if (noncommutativeEquals)
return true; return true;
@ -250,9 +356,9 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
const bool commutative = tok1->isBinaryOp() && Token::Match(tok1, "%or%|%oror%|+|*|&|&&|^|==|!="); const bool commutative = tok1->isBinaryOp() && Token::Match(tok1, "%or%|%oror%|+|*|&|&&|^|==|!=");
bool commutativeEquals = commutative && bool commutativeEquals = commutative &&
isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure); isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, errors);
commutativeEquals = commutativeEquals && commutativeEquals = commutativeEquals &&
isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure); isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, errors);
return commutativeEquals; return commutativeEquals;
@ -626,7 +732,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
if (!tok->function()) { if (!tok->function()) {
// if the library says 0 is invalid // if the library says 0 is invalid
// => it is assumed that parameter is an in parameter (TODO: this is a bad heuristic) // => it is assumed that parameter is an in parameter (TODO: this is a bad heuristic)
if (!addressOf && settings->library.isnullargbad(tok, 1+argnr)) if (!addressOf && settings && settings->library.isnullargbad(tok, 1+argnr))
return false; return false;
// addressOf => inconclusive // addressOf => inconclusive
if (!addressOf) { if (!addressOf) {
@ -639,8 +745,13 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
const Variable *arg = tok->function()->getArgumentVar(argnr); const Variable *arg = tok->function()->getArgumentVar(argnr);
if (addressOf && !(arg && arg->isConst())) if (addressOf) {
return true; if(!(arg && arg->isConst()))
return true;
// If const is applied to the pointer, then the value can still be modified
if(arg && Token::simpleMatch(arg->typeEndToken(), "* const"))
return true;
}
return arg && !arg->isConst() && arg->isReference(); return arg && !arg->isConst() && arg->isReference();
} }

4
lib/astutils.h
View File

@ -25,6 +25,8 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "errorlogger.h"
class Library; class Library;
class Settings; class Settings;
class Token; class Token;
@ -54,7 +56,7 @@ std::string astCanonicalType(const Token *expr);
/** Is given syntax tree a variable comparison against value */ /** Is given syntax tree a variable comparison against value */
const Token * astIsVariableComparison(const Token *tok, const std::string &comp, const std::string &rhs, const Token **vartok=nullptr); const Token * astIsVariableComparison(const Token *tok, const std::string &comp, const std::string &rhs, const Token **vartok=nullptr);
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure); bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, ErrorPath* errors=nullptr);
bool isEqualKnownValue(const Token * const tok1, const Token * const tok2); bool isEqualKnownValue(const Token * const tok1, const Token * const tok2);

33
lib/checkother.cpp
View File

@ -1981,10 +1981,11 @@ void CheckOther::checkDuplicateExpression()
} }
} }
} }
ErrorPath errorPath;
if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) { if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) {
if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true)) if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true))
continue; continue;
if (isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true)) { if (isSameExpression(mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true, &errorPath)) {
if (isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { if (isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) {
const bool assignment = tok->str() == "="; const bool assignment = tok->str() == "=";
if (assignment && warningEnabled) if (assignment && warningEnabled)
@ -1999,7 +2000,7 @@ void CheckOther::checkDuplicateExpression()
continue; continue;
} }
} }
duplicateExpressionError(tok, tok, tok->str()); duplicateExpressionError(tok->astOperand1(), tok->astOperand2(), tok, errorPath);
} }
} }
} else if (styleEnabled && } else if (styleEnabled &&
@ -2008,17 +2009,17 @@ void CheckOther::checkDuplicateExpression()
isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) {
oppositeExpressionError(tok, tok, tok->str()); oppositeExpressionError(tok, tok, tok->str());
} else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative } else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative
if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), mSettings->library, true) && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2())) if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), mSettings->library, true, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2()))
duplicateExpressionError(tok->astOperand2(), tok->astOperand2(), tok->str()); duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath);
else if (tok->astOperand2()) { else if (tok->astOperand2()) {
const Token *ast1 = tok->astOperand1(); const Token *ast1 = tok->astOperand1();
while (ast1 && tok->str() == ast1->str()) { while (ast1 && tok->str() == ast1->str()) {
if (isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand1(), tok->astOperand2(), mSettings->library, true) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand1())) if (isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand1(), tok->astOperand2(), mSettings->library, true, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand1()))
// TODO: warn if variables are unchanged. See #5683 // TODO: warn if variables are unchanged. See #5683
// Probably the message should be changed to 'duplicate expressions X in condition or something like that'. // Probably the message should be changed to 'duplicate expressions X in condition or something like that'.
;//duplicateExpressionError(ast1->astOperand1(), tok->astOperand2(), tok->str()); ;//duplicateExpressionError(ast1->astOperand1(), tok->astOperand2(), tok, errorPath);
else if (styleEnabled && isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand2(), tok->astOperand2(), mSettings->library, true) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2())) else if (styleEnabled && isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand2(), tok->astOperand2(), mSettings->library, true, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2()))
duplicateExpressionError(ast1->astOperand2(), tok->astOperand2(), tok->str()); duplicateExpressionError(ast1->astOperand2(), tok->astOperand2(), tok, errorPath);
if (!isConstExpression(ast1->astOperand2(), mSettings->library, true)) if (!isConstExpression(ast1->astOperand2(), mSettings->library, true))
break; break;
ast1 = ast1->astOperand1(); ast1 = ast1->astOperand1();
@ -2045,13 +2046,19 @@ void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, c
"determine if it is correct.", CWE398, false); "determine if it is correct.", CWE398, false);
} }
void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op) void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors)
{ {
std::list<const Token *> toks = { tok1 }; errors.emplace_back(opTok, "");
if (tok1 != tok2)
toks.push_front(tok2);
reportError(toks, Severity::style, "duplicateExpression", "Same expression on both sides of \'" + op + "\'.\n" const std::string& expr1 = tok1 ? tok1->expressionString() : "x";
const std::string& expr2 = tok2 ? tok2->expressionString() : "x";
std::string endingPhrase = expr1 == expr2 ? "." :
" because the value of '" + expr1 + "' and '" + expr2 + "' are the same.";
const std::string& op = opTok ? opTok->str() : "&&";
reportError(errors, Severity::style, "duplicateExpression", "Same expression on both sides of \'" + op + "\'" + endingPhrase + "\n"
"Finding the same expression on both sides of an operator is suspicious and might " "Finding the same expression on both sides of an operator is suspicious and might "
"indicate a cut and paste or logic error. Please examine this code carefully to " "indicate a cut and paste or logic error. Please examine this code carefully to "
"determine if it is correct.", CWE398, false); "determine if it is correct.", CWE398, false);

6
lib/checkother.h
View File

@ -240,7 +240,7 @@ private:
void duplicateBranchError(const Token *tok1, const Token *tok2); void duplicateBranchError(const Token *tok1, const Token *tok2);
void duplicateAssignExpressionError(const Token *tok1, const Token *tok2); void duplicateAssignExpressionError(const Token *tok1, const Token *tok2);
void oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op); void oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op); void duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors);
void duplicateValueTernaryError(const Token *tok); void duplicateValueTernaryError(const Token *tok);
void duplicateExpressionTernaryError(const Token *tok); void duplicateExpressionTernaryError(const Token *tok);
void duplicateBreakError(const Token *tok, bool inconclusive); void duplicateBreakError(const Token *tok, bool inconclusive);
@ -267,6 +267,8 @@ private:
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override { void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override {
CheckOther c(nullptr, settings, errorLogger); CheckOther c(nullptr, settings, errorLogger);
ErrorPath errorPath;
// error // error
c.zerodivError(nullptr, nullptr); c.zerodivError(nullptr, nullptr);
c.misusedScopeObjectError(nullptr, "varname"); c.misusedScopeObjectError(nullptr, "varname");
@ -301,7 +303,7 @@ private:
c.clarifyStatementError(nullptr); c.clarifyStatementError(nullptr);
c.duplicateBranchError(nullptr, nullptr); c.duplicateBranchError(nullptr, nullptr);
c.oppositeExpressionError(nullptr, nullptr, "&&"); c.oppositeExpressionError(nullptr, nullptr, "&&");
c.duplicateExpressionError(nullptr, nullptr, "&&"); c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath);
c.duplicateValueTernaryError(nullptr); c.duplicateValueTernaryError(nullptr);
c.duplicateExpressionTernaryError(nullptr); c.duplicateExpressionTernaryError(nullptr);
c.duplicateBreakError(nullptr, false); c.duplicateBreakError(nullptr, false);

2
test/testcondition.cpp
View File

@ -1555,7 +1555,7 @@ private:
" if(!b) {}\n" " if(!b) {}\n"
" }\n" " }\n"
"}"); "}");
TODO_ASSERT_EQUALS("error", "", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
check("void foo(unsigned u) {\n" check("void foo(unsigned u) {\n"
" if (u != 0) {\n" " if (u != 0) {\n"

205
test/testother.cpp
View File

@ -130,6 +130,9 @@ private:
TEST_CASE(duplicateExpression4); // ticket #3354 (++) TEST_CASE(duplicateExpression4); // ticket #3354 (++)
TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values) TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values)
TEST_CASE(duplicateExpression6); // ticket #4639 TEST_CASE(duplicateExpression6); // ticket #4639
TEST_CASE(duplicateExpression7);
TEST_CASE(duplicateExpression8);
TEST_CASE(duplicateExpressionLoop);
TEST_CASE(duplicateValueTernary); TEST_CASE(duplicateValueTernary);
TEST_CASE(duplicateExpressionTernary); // #6391 TEST_CASE(duplicateExpressionTernary); // #6391
TEST_CASE(duplicateExpressionTemplate); // #6930 TEST_CASE(duplicateExpressionTemplate); // #6930
@ -483,7 +486,7 @@ private:
" double fStepHelp = 0;\n" " double fStepHelp = 0;\n"
" if( (rOuterValue >>= fStepHelp) ) {\n" " if( (rOuterValue >>= fStepHelp) ) {\n"
" if( fStepHelp != 0.0) {\n" " if( fStepHelp != 0.0) {\n"
" double fStepMain = 0;\n" " double fStepMain = 1;\n"
" sal_Int32 nIntervalCount = static_cast< sal_Int32 >(fStepMain / fStepHelp);\n" " sal_Int32 nIntervalCount = static_cast< sal_Int32 >(fStepMain / fStepHelp);\n"
" }\n" " }\n"
" }\n" " }\n"
@ -3530,12 +3533,12 @@ private:
check("void foo(int a, int b) {\n" check("void foo(int a, int b) {\n"
" if ((a < b) && (b > a)) { }\n" " if ((a < b) && (b > a)) { }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&' because the value of 'a<b' and 'b>a' are the same.\n", errout.str());
check("void foo(int a, int b) {\n" check("void foo(int a, int b) {\n"
" if ((a <= b) && (b >= a)) { }\n" " if ((a <= b) && (b >= a)) { }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&' because the value of 'a<=b' and 'b>=a' are the same.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (x!=2 || y!=3 || x!=2) {}\n" " if (x!=2 || y!=3 || x!=2) {}\n"
@ -3659,7 +3662,7 @@ private:
check("void foo(int a, int b) {\n" check("void foo(int a, int b) {\n"
" if ((b + a) | (a + b)) {}\n" " if ((b + a) | (a + b)) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '|'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '|' because the value of 'b+a' and 'a+b' are the same.\n", errout.str());
check("void foo(const std::string& a, const std::string& b) {\n" check("void foo(const std::string& a, const std::string& b) {\n"
" return a.find(b+\"&\") || a.find(\"&\"+b);\n" " return a.find(b+\"&\") || a.find(\"&\"+b);\n"
@ -3674,7 +3677,7 @@ private:
check("void foo(double a, double b) {\n" check("void foo(double a, double b) {\n"
" if ((b + a) > (a + b)) {}\n" " if ((b + a) > (a + b)) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '>'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '>' because the value of 'b+a' and 'a+b' are the same.\n", errout.str());
check("void f(int x) {\n" check("void f(int x) {\n"
" if ((x == 1) && (x == 0x00000001))\n" " if ((x == 1) && (x == 0x00000001))\n"
@ -3908,6 +3911,198 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void duplicateExpression7() {
check("void f() {\n"
" const int i = sizeof(int);\n"
" if ( i != sizeof (int)){}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'i' and 'sizeof(int)' are the same.\n", errout.str());
check("void f() {\n"
" const int i = sizeof(int);\n"
" if ( sizeof (int) != i){}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'sizeof(int)' and 'i' are the same.\n", errout.str());
check("void f(int a = 1) { if ( a != 1){}}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int a = 1;\n"
" if ( a != 1){} \n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str());
check("void f() {\n"
" int a = 1;\n"
" int b = 1;\n"
" if ( a != b){} \n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:4]: (style) Same expression on both sides of '!=' because the value of 'a' and 'b' are the same.\n", errout.str());
check("void f() {\n"
" int a = 1;\n"
" int b = a;\n"
" if ( a != b){} \n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Same expression on both sides of '!=' because the value of 'a' and 'b' are the same.\n", errout.str());
check("void use(int);\n"
"void f() {\n"
" int a = 1;\n"
" int b = 1;\n"
" use(b);\n"
" if ( a != 1){} \n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str());
check("void use(int);\n"
"void f() {\n"
" int a = 1;\n"
" use(a);\n"
" a = 2;\n"
" if ( a != 1){} \n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void use(int);\n"
"void f() {\n"
" int a = 2;\n"
" use(a);\n"
" a = 1;\n"
" if ( a != 1){} \n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("const int a = 1;\n"
" void f() {\n"
" if ( a != 1){} \n"
"}\n");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str());
check("int a = 1;\n"
" void f() {\n"
" if ( a != 1){} \n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" static const int a = 1;\n"
" if ( a != 1){} \n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str());
check("void f() {\n"
" static int a = 1;\n"
" if ( a != 1){} \n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" int a = 1;\n"
" if ( a != 1){\n"
" a++;\n"
" }}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str());
check("void f(int b) {\n"
" int a = 1;\n"
" while (b) {\n"
" if ( a != 1){}\n"
" a++;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void duplicateExpression8() {
check("void f() {\n"
" int a = 1;\n"
" int b = a;\n"
" a = 2;\n"
" if ( b != a){}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f(int * a, int i) { int b = a[i]; a[i] = 2; if ( b != a[i]){}}\n");
ASSERT_EQUALS("", errout.str());
check("void f(int * a, int i) { int b = *a; *a = 2; if ( b != *a){}}\n");
ASSERT_EQUALS("", errout.str());
check("struct A { int f() const; };\n"
"A g();\n"
"void foo() {\n"
" for (const A x = A();;) {\n"
" const int a = x.f();\n"
" x = g();\n"
" if (x.f() == a) break;\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("int f(int i);\n"
"struct A {\n"
" enum E { B, C };\n"
" bool f(E);\n"
"};\n"
"void foo() {\n"
" A a;\n"
" const bool x = a.f(A::B);\n"
" const bool y = a.f(A::C);\n"
" if(!x && !y) return;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void foo() { \n"
" const bool x = a.f(A::B);\n"
" const bool y = a.f(A::C);\n"
" if (!x && !y) return;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f(bool * const b);\n"
"void foo() { \n"
" bool x = true;\n"
" bool y = true;\n"
" f(&x);\n"
" if (!x && !y) return;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" const int a = {};\n"
" if(a == 1) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void duplicateExpressionLoop() {
check("void f() {\n"
" int a = 1;\n"
" while ( a != 1){}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str());
check("void f() { int a = 1; while ( a != 1){ a++; }}\n");
ASSERT_EQUALS("", errout.str());
check("void f() { int a = 1; for ( int i=0; i < 3 && a != 1; i++){ a++; }}\n");
ASSERT_EQUALS("", errout.str());
check("void f(int b) { int a = 1; while (b) { if ( a != 1){} b++; } a++; } \n");
ASSERT_EQUALS("", errout.str());
check("void f(int b) {\n"
" while (b) {\n"
" int a = 1;\n"
" if ( a != 1){}\n"
" b++;\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str());
}
void duplicateExpressionTernary() { // #6391 void duplicateExpressionTernary() { // #6391
check("void f() {\n" check("void f() {\n"
" return A ? x : x;\n" " return A ? x : x;\n"