Enable followVar for opposite expressions (#1404)

Enable followVar for opposite expressions
This commit is contained in:
Paul Fultz II 2018-10-04 14:17:47 -05:00 committed by Daniel Marjamäki
parent e9ddf4ddeb
commit e170a45230
5 changed files with 36 additions and 21 deletions

View File

@ -450,7 +450,7 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
}
if (cond2->str() == "!")
return isOppositeCond(isNot, cpp, cond2, cond1, library, pure, followVar);
return isOppositeCond(isNot, cpp, cond2, cond1, library, pure, followVar, errors);
if (!isNot) {
if (cond1->str() == "==" && cond2->str() == "==") {
@ -553,7 +553,7 @@ bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * cons
{
if (!tok1 || !tok2)
return false;
if (isOppositeCond(true, cpp, tok1, tok2, library, pure, followVar))
if (isOppositeCond(true, cpp, tok1, tok2, library, pure, followVar, errors))
return true;
if (tok1->isUnaryOp("-"))
return isSameExpression(cpp, true, tok1->astOperand1(), tok2, library, pure, followVar, errors);

View File

@ -2005,10 +2005,10 @@ void CheckOther::checkDuplicateExpression()
}
}
} else if (styleEnabled &&
isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false, false, &errorPath) &&
isOppositeExpression(mTokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), mSettings->library, false, true, &errorPath) &&
!Token::Match(tok, "=|-|-=|/|/=") &&
isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand1())) {
oppositeExpressionError(tok, tok, tok->str(), errorPath);
oppositeExpressionError(tok->astOperand1(), tok->astOperand2(), tok, errorPath);
} 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, false, &errorPath) && isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2()))
duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath);
@ -2037,12 +2037,11 @@ void CheckOther::checkDuplicateExpression()
}
}
void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op, ErrorPath errors)
void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors)
{
if (tok1)
errors.emplace_back(tok1, "");
if (tok2)
errors.emplace_back(tok2, "");
errors.emplace_back(opTok, "");
const std::string& op = opTok ? opTok->str() : "&&";
reportError(errors, Severity::style, "oppositeExpression", "Opposite expression on both sides of \'" + op + "\'.\n"
"Finding the opposite expression on both sides of an operator is suspicious and might "

View File

@ -239,7 +239,7 @@ private:
void misusedScopeObjectError(const Token *tok, const std::string &varname);
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, ErrorPath errors);
void oppositeExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors);
void duplicateValueTernaryError(const Token *tok);
void duplicateExpressionTernaryError(const Token *tok, ErrorPath errors);
@ -302,7 +302,7 @@ private:
c.clarifyCalculationError(nullptr, "+");
c.clarifyStatementError(nullptr);
c.duplicateBranchError(nullptr, nullptr);
c.oppositeExpressionError(nullptr, nullptr, "&&", errorPath);
c.oppositeExpressionError(nullptr, nullptr, nullptr, errorPath);
c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath);
c.duplicateValueTernaryError(nullptr);
c.duplicateExpressionTernaryError(nullptr, errorPath);

View File

@ -1601,7 +1601,7 @@ private:
" if(!b) {}\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Opposite inner 'if' condition leads to a dead code block.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [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"

View File

@ -4232,34 +4232,50 @@ private:
void oppositeExpression() {
check("void f(bool a) { if(a && !a) {} }");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '&&'.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '&&'.\n", errout.str());
check("void f(bool a) { if(a != !a) {} }");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
check("void f(bool a) { if( a == !(a) ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
check("void f(bool a) { if( a != !(a) ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
check("void f(bool a) { if( !(a) == a ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
check("void f(bool a) { if( !(a) != a ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
check("void f(bool a) { if( !(!a) == !(a) ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
check("void f(bool a) { if( !(!a) != !(a) ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
check("void f1(bool a) {\n"
" const bool b = a;\n"
" if( a == !(b) ) {}\n"
" if( b == !(a) ) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Opposite expression on both sides of '=='.\n"
"[test.cpp:2] -> [test.cpp:4]: (style) Opposite expression on both sides of '=='.\n", errout.str());
check("void f2(bool *a) {\n"
" const bool b = *a;\n"
" if( *a == !(b) ) {}\n"
" if( b == !(*a) ) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Opposite expression on both sides of '=='.\n"
"[test.cpp:2] -> [test.cpp:4]: (style) Opposite expression on both sides of '=='.\n", errout.str());
check("void f(bool a) { a = !a; }");
ASSERT_EQUALS("", errout.str());
check("void f(int a) { if( a < -a ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '<'.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1]: (style) Opposite expression on both sides of '<'.\n", errout.str());
check("void f(int a) { a -= -a; }");
ASSERT_EQUALS("", errout.str());