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

Fixed #6764 (False positive redundantCondition - !(i>1) is not i<1)

This commit is contained in:
Daniel Marjamäki 2015-06-19 19:49:05 +02:00
parent 02df692b0b
commit af4a4663e2
2 changed files with 28 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
lib/checkcondition.cpp
View File

@ -358,7 +358,13 @@ void CheckCondition::multiConditionError(const Token *tok, unsigned int line1)
// Detect oppositing inner and outer conditions // Detect oppositing inner and outer conditions
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
static bool isOppositeCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions) /**
* Are two conditions opposite
* @param isNot do you want to know if cond1 is !cond2 or if cond1 and cond2 are non-overlapping. true: cond1==!cond2 false: cond1==true => cond2==false
* @param cond1 condition1
* @param cond2 condition2
*/
static bool isOppositeCond(bool isNot, const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
{ {
if (!cond1 || !cond2) if (!cond1 || !cond2)
return false; return false;
@ -392,11 +398,11 @@ static bool isOppositeCond(const Token * const cond1, const Token * const cond2,
return ((comp1 == "==" && comp2 == "!=") || return ((comp1 == "==" && comp2 == "!=") ||
(comp1 == "!=" && comp2 == "==") || (comp1 == "!=" && comp2 == "==") ||
(comp1 == "<" && comp2 == ">=") || (comp1 == "<" && comp2 == ">=") ||
(comp1 == "<" && comp2 == ">") ||
(comp1 == "<=" && comp2 == ">") || (comp1 == "<=" && comp2 == ">") ||
(comp1 == ">" && comp2 == "<=") || (comp1 == ">" && comp2 == "<=") ||
(comp1 == ">" && comp2 == "<") || (comp1 == ">=" && comp2 == "<") ||
(comp1 == ">=" && comp2 == "<")); (!isNot && ((comp1 == "<" && comp2 == ">") ||
(comp1 == ">" && comp2 == "<"))));
} }
void CheckCondition::oppositeInnerCondition() void CheckCondition::oppositeInnerCondition()
@ -488,7 +494,7 @@ void CheckCondition::oppositeInnerCondition()
const Token *cond1 = scope->classDef->next()->astOperand2(); const Token *cond1 = scope->classDef->next()->astOperand2();
const Token *cond2 = ifToken->next()->astOperand2(); const Token *cond2 = ifToken->next()->astOperand2();
if (isOppositeCond(cond1, cond2, _settings->library.functionpure)) if (isOppositeCond(false, cond1, cond2, _settings->library.functionpure))
oppositeInnerConditionError(scope->classDef, cond2); oppositeInnerConditionError(scope->classDef, cond2);
} }
} }
@ -624,7 +630,7 @@ void CheckCondition::checkIncorrectLogicOperator()
tok->astOperand1() && tok->astOperand1() &&
tok->astOperand2() && tok->astOperand2() &&
(tok->astOperand1()->isName() || tok->astOperand2()->isName()) && (tok->astOperand1()->isName() || tok->astOperand2()->isName()) &&
isOppositeCond(tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) { isOppositeCond(true, tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
const bool alwaysTrue(tok->str() == "||"); const bool alwaysTrue(tok->str() == "||");
incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue); incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue);
@ -633,7 +639,7 @@ void CheckCondition::checkIncorrectLogicOperator()
else if (Token::Match(tok, "&&|%oror%")) { else if (Token::Match(tok, "&&|%oror%")) {
if (printStyle && (tok->str() == "||") && tok->astOperand1() && tok->astOperand2() && tok->astOperand2()->str() == "&&") { if (printStyle && (tok->str() == "||") && tok->astOperand1() && tok->astOperand2() && tok->astOperand2()->str() == "&&") {
const Token* tok2 = tok->astOperand2()->astOperand1(); const Token* tok2 = tok->astOperand2()->astOperand1();
if (isOppositeCond(tok->astOperand1(), tok2, _settings->library.functionpure)) { if (isOppositeCond(true, tok->astOperand1(), tok2, _settings->library.functionpure)) {
redundantConditionError(tok, tok2->expressionString() + ". 'A && (!A || B)' is equivalent to 'A || B'"); redundantConditionError(tok, tok2->expressionString() + ". 'A && (!A || B)' is equivalent to 'A || B'");
} }
} }

15
test/testcondition.cpp
View File

@ -897,6 +897,16 @@ private:
" if (i || !i) {}\n" " if (i || !i) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: i||!i.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: i||!i.\n", errout.str());
check("void f(int a, int b) {\n"
" if (a>b || a<=b) {}\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: a>b||a<=b.\n", "", errout.str());
check("void f(int a, int b) {\n"
" if (a>b || a<b) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
} }
void secondAlwaysTrueFalseWhenFirstTrueError() { void secondAlwaysTrueFalseWhenFirstTrueError() {
@ -1343,6 +1353,11 @@ private:
" if (y==0 || y!=0 && z);\n" " if (y==0 || y!=0 && z);\n"
"}", false); "}", false);
ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition: y. 'A && (!A || B)' is equivalent to 'A || B'\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition: y. 'A && (!A || B)' is equivalent to 'A || B'\n", errout.str());
check("void f() {\n"
" if (x>0 || (x<0 && y)) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
} }
// clarify conditions with bitwise operator and comparison // clarify conditions with bitwise operator and comparison