Fixed #5132 (False negative: incorrectLogicOperator in simple if-clause)
This commit is contained in:
parent
5cdbe0f42d
commit
f7f44f24c7
|
@ -157,7 +157,16 @@ bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::
|
||||||
|
|
||||||
static bool isOppositeCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
|
static bool isOppositeCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
|
||||||
{
|
{
|
||||||
if (!cond1 || !cond1->isComparisonOp() || !cond2 || !cond2->isComparisonOp())
|
if (!cond1 || !cond2)
|
||||||
|
return false;
|
||||||
|
|
||||||
|
if (cond1->str() == "!")
|
||||||
|
return isSameExpression(cond1->astOperand1(), cond2, constFunctions);
|
||||||
|
|
||||||
|
if (cond2->str() == "!")
|
||||||
|
return isSameExpression(cond1, cond2->astOperand1(), constFunctions);
|
||||||
|
|
||||||
|
if (!cond1->isComparisonOp() || !cond2->isComparisonOp())
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
const std::string &comp1 = cond1->str();
|
const std::string &comp1 = cond1->str();
|
||||||
|
@ -1260,7 +1269,16 @@ void CheckOther::checkIncorrectLogicOperator()
|
||||||
const Scope * scope = symbolDatabase->functionScopes[ii];
|
const Scope * scope = symbolDatabase->functionScopes[ii];
|
||||||
|
|
||||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||||
if (Token::Match(tok, "&&|%oror%")) {
|
// Opposite comparisons
|
||||||
|
if (Token::Match(tok, "%oror%|&&") &&
|
||||||
|
(tok->astOperand1()->isName() || tok->astOperand2()->isName()) &&
|
||||||
|
isOppositeCond(tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
|
||||||
|
|
||||||
|
const bool alwaysTrue(tok->str() == "||");
|
||||||
|
incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue);
|
||||||
|
}
|
||||||
|
|
||||||
|
else if (Token::Match(tok, "&&|%oror%")) {
|
||||||
// Comparison #1 (LHS)
|
// Comparison #1 (LHS)
|
||||||
const Token *comp1 = tok->astOperand1();
|
const Token *comp1 = tok->astOperand1();
|
||||||
if (comp1 && comp1->str() == tok->str())
|
if (comp1 && comp1->str() == tok->str())
|
||||||
|
|
|
@ -134,6 +134,7 @@ private:
|
||||||
TEST_CASE(incorrectLogicOperator4);
|
TEST_CASE(incorrectLogicOperator4);
|
||||||
TEST_CASE(incorrectLogicOperator5); // complex expressions
|
TEST_CASE(incorrectLogicOperator5); // complex expressions
|
||||||
TEST_CASE(incorrectLogicOperator6); // char literals
|
TEST_CASE(incorrectLogicOperator6); // char literals
|
||||||
|
TEST_CASE(incorrectLogicOperator7); // opposite expressions: (expr || !expr)
|
||||||
TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError);
|
TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError);
|
||||||
TEST_CASE(incorrectLogicOp_condSwapping);
|
TEST_CASE(incorrectLogicOp_condSwapping);
|
||||||
|
|
||||||
|
@ -4199,6 +4200,13 @@ private:
|
||||||
TODO_ASSERT_EQUALS("error", "", errout.str());
|
TODO_ASSERT_EQUALS("error", "", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void incorrectLogicOperator7() { // opposite expressions
|
||||||
|
check("void f(int i) {\n"
|
||||||
|
" if (i || !i) {}\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: i||!i.\n", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
void secondAlwaysTrueFalseWhenFirstTrueError() {
|
void secondAlwaysTrueFalseWhenFirstTrueError() {
|
||||||
check("void f(int x) {\n"
|
check("void f(int x) {\n"
|
||||||
" if (x > 5 && x != 1)\n"
|
" if (x > 5 && x != 1)\n"
|
||||||
|
|
Loading…
Reference in New Issue