From f603b529dfa4a440b464d805530bf21bb334d26f Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Tue, 7 Aug 2018 02:32:16 -0500 Subject: [PATCH] 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 0e491b41a8686a2b767ed1a116e6e3b6ff66f534. * Check if the tokens are the same * Report the operator and not the expressions --- lib/astutils.cpp | 137 ++++++++++++++++++++++++--- lib/astutils.h | 4 +- lib/checkother.cpp | 33 ++++--- lib/checkother.h | 6 +- test/testcondition.cpp | 2 +- test/testother.cpp | 205 ++++++++++++++++++++++++++++++++++++++++- 6 files changed, 352 insertions(+), 35 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index f6e6bc6a7..18313d7fa 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -133,9 +133,96 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp, 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; if (tok1 == nullptr || tok2 == nullptr) 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") tok2 = tok2->astOperand2(); } + // Skip double not 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(), "=")) { - 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 ((Token::Match(tok1,"<|>") && Token::Match(tok2,"<|>")) || (Token::Match(tok1,"<=|>=") && Token::Match(tok2,"<=|>="))) { - return isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure) && - isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure); + return isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, errors) && + isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, errors); } return false; } @@ -233,9 +339,9 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2 return false; } bool noncommutativeEquals = - isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand1(), library, pure); + isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand1(), library, pure, errors); noncommutativeEquals = noncommutativeEquals && - isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand2(), library, pure); + isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand2(), library, pure, errors); if (noncommutativeEquals) 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%|+|*|&|&&|^|==|!="); bool commutativeEquals = commutative && - isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure); + isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), library, pure, errors); commutativeEquals = commutativeEquals && - isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure); + isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), library, pure, errors); return commutativeEquals; @@ -626,7 +732,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, if (!tok->function()) { // if the library says 0 is invalid // => 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; // addressOf => inconclusive if (!addressOf) { @@ -639,8 +745,13 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, const Variable *arg = tok->function()->getArgumentVar(argnr); - if (addressOf && !(arg && arg->isConst())) - return true; + if (addressOf) { + 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(); } diff --git a/lib/astutils.h b/lib/astutils.h index 97e65bd6a..83f40d7a2 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -25,6 +25,8 @@ #include #include +#include "errorlogger.h" + class Library; class Settings; class Token; @@ -54,7 +56,7 @@ std::string astCanonicalType(const Token *expr); /** 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); -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); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 517a64866..91154f8e4 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1981,10 +1981,11 @@ void CheckOther::checkDuplicateExpression() } } } + ErrorPath errorPath; if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) { if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true)) 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())) { const bool assignment = tok->str() == "="; if (assignment && warningEnabled) @@ -1999,7 +2000,7 @@ void CheckOther::checkDuplicateExpression() continue; } } - duplicateExpressionError(tok, tok, tok->str()); + duplicateExpressionError(tok->astOperand1(), tok->astOperand2(), tok, errorPath); } } } else if (styleEnabled && @@ -2008,17 +2009,17 @@ void CheckOther::checkDuplicateExpression() isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) { oppositeExpressionError(tok, tok, tok->str()); } 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())) - duplicateExpressionError(tok->astOperand2(), tok->astOperand2(), tok->str()); + 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->astOperand1()->astOperand2(), tok, errorPath); else if (tok->astOperand2()) { const Token *ast1 = tok->astOperand1(); 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 // Probably the message should be changed to 'duplicate expressions X in condition or something like that'. - ;//duplicateExpressionError(ast1->astOperand1(), tok->astOperand2(), tok->str()); - else if (styleEnabled && isSameExpression(mTokenizer->isCPP(), true, ast1->astOperand2(), tok->astOperand2(), mSettings->library, true) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2())) - duplicateExpressionError(ast1->astOperand2(), 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, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), ast1->astOperand2())) + duplicateExpressionError(ast1->astOperand2(), tok->astOperand2(), tok, errorPath); if (!isConstExpression(ast1->astOperand2(), mSettings->library, true)) break; ast1 = ast1->astOperand1(); @@ -2045,13 +2046,19 @@ void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, c "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 toks = { tok1 }; - if (tok1 != tok2) - toks.push_front(tok2); + errors.emplace_back(opTok, ""); - 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 " "indicate a cut and paste or logic error. Please examine this code carefully to " "determine if it is correct.", CWE398, false); diff --git a/lib/checkother.h b/lib/checkother.h index 09e210b3e..b6fe0632b 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -240,7 +240,7 @@ private: void duplicateBranchError(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 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 duplicateExpressionTernaryError(const Token *tok); void duplicateBreakError(const Token *tok, bool inconclusive); @@ -267,6 +267,8 @@ private: void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override { CheckOther c(nullptr, settings, errorLogger); + ErrorPath errorPath; + // error c.zerodivError(nullptr, nullptr); c.misusedScopeObjectError(nullptr, "varname"); @@ -301,7 +303,7 @@ private: c.clarifyStatementError(nullptr); c.duplicateBranchError(nullptr, nullptr); c.oppositeExpressionError(nullptr, nullptr, "&&"); - c.duplicateExpressionError(nullptr, nullptr, "&&"); + c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath); c.duplicateValueTernaryError(nullptr); c.duplicateExpressionTernaryError(nullptr); c.duplicateBreakError(nullptr, false); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index a251c8769..3c62b53d5 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -1555,7 +1555,7 @@ private: " if(!b) {}\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" " if (u != 0) {\n" diff --git a/test/testother.cpp b/test/testother.cpp index e9ac16a85..becc90311 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -130,6 +130,9 @@ private: TEST_CASE(duplicateExpression4); // ticket #3354 (++) TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values) TEST_CASE(duplicateExpression6); // ticket #4639 + TEST_CASE(duplicateExpression7); + TEST_CASE(duplicateExpression8); + TEST_CASE(duplicateExpressionLoop); TEST_CASE(duplicateValueTernary); TEST_CASE(duplicateExpressionTernary); // #6391 TEST_CASE(duplicateExpressionTemplate); // #6930 @@ -483,7 +486,7 @@ private: " double fStepHelp = 0;\n" " if( (rOuterValue >>= fStepHelp) ) {\n" " if( fStepHelp != 0.0) {\n" - " double fStepMain = 0;\n" + " double fStepMain = 1;\n" " sal_Int32 nIntervalCount = static_cast< sal_Int32 >(fStepMain / fStepHelp);\n" " }\n" " }\n" @@ -3530,12 +3533,12 @@ private: check("void foo(int a, int b) {\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 'aa' are the same.\n", errout.str()); check("void foo(int a, int b) {\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" " if (x!=2 || y!=3 || x!=2) {}\n" @@ -3659,7 +3662,7 @@ private: check("void foo(int a, int 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" " return a.find(b+\"&\") || a.find(\"&\"+b);\n" @@ -3674,7 +3677,7 @@ private: check("void foo(double a, double 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" " if ((x == 1) && (x == 0x00000001))\n" @@ -3908,6 +3911,198 @@ private: 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 check("void f() {\n" " return A ? x : x;\n"