From 8f27cec9919955f5683caf9927d1b54d3e9c6be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 14 Dec 2015 20:29:29 +0100 Subject: [PATCH] Revert "minor tweak of comment" This reverts commit b1d1869f22fe24423ee740cb87179028e130bfce. --- addons/cppcheckdata.py | 2 +- lib/astutils.cpp | 45 ---------- lib/astutils.h | 1 - lib/checkcondition.cpp | 182 ----------------------------------------- lib/checkcondition.h | 3 - 5 files changed, 1 insertion(+), 232 deletions(-) diff --git a/addons/cppcheckdata.py b/addons/cppcheckdata.py index 3fedc11f4..0c5c3acf9 100644 --- a/addons/cppcheckdata.py +++ b/addons/cppcheckdata.py @@ -352,7 +352,7 @@ class ValueFlow: # http://cppcheck.sourceforge.net/devinfo/doxyoutput/classValueFlow_1_1Value.html class Value: - ## integer value + # #integer value intvalue = None ## token value tokvalue = None diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 889c91eb6..f7389d749 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -239,51 +239,6 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token (comp1 == ">" && comp2 == "<")))); } - -bool isIncorrectCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const std::set &constFunctions) -{ - if (!cond1 || !cond2) - return false; - - if (cond1->str() == "!") { - if (cond2->str() == "!=") { - if (cond2->astOperand1() && cond2->astOperand1()->str() == "0") - return isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand2(), constFunctions); - if (cond2->astOperand2() && cond2->astOperand2()->str() == "0") - return isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand1(), constFunctions); - } - return isSameExpression(cpp, cond1->astOperand1(), cond2, constFunctions); - } - - if (cond2->str() == "!") - return isIncorrectCond(isNot, cpp, cond2, cond1, constFunctions); - - if (!cond1->isComparisonOp() || !cond2->isComparisonOp()) - return false; - - const std::string &comp1 = cond1->str(); - - // condition found .. get comparator - std::string comp2; - if (isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand1(), constFunctions) && - isSameExpression(cpp, cond1->astOperand2(), cond2->astOperand2(), constFunctions)) { - comp2 = cond2->str(); - } else if (isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand2(), constFunctions) && - isSameExpression(cpp, cond1->astOperand2(), cond2->astOperand1(), constFunctions)) { - comp2 = cond2->str(); - if (comp2[0] == '>') - comp2[0] = '<'; - else if (comp2[0] == '<') - comp2[0] = '>'; - } - - // is condition incorrect? - return ((comp1 == "<" && comp2 == "==") || - (comp1 == ">" && comp2 == "==") || - (comp1 == "==" && comp2 == "<") || - (comp1 == "==" && comp2 == ">")); -} - bool isConstExpression(const Token *tok, const std::set &constFunctions) { if (!tok) diff --git a/lib/astutils.h b/lib/astutils.h index f3a68dcfd..32296a5fa 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -59,7 +59,6 @@ bool isSameExpression(bool cpp, const Token *tok1, const Token *tok2, const std: * @param constFunctions constFunctions */ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const std::set &constFunctions); -bool isIncorrectCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const std::set &constFunctions); bool isConstExpression(const Token *tok, const std::set &constFunctions); diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index f7dd40705..0d6f1da6e 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -641,187 +641,6 @@ static std::string conditionString(bool not1, const Token *expr1, const std::str (expr1->isName() ? expr1->str() : std::string("EXPR")); } -void CheckCondition::checkIncorrectCondition() -{ - const bool printStyle = _settings->isEnabled("style"); - const bool printWarning = _settings->isEnabled("warning"); - if (!printWarning && !printStyle) - return; - - const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); - const std::size_t functions = symbolDatabase->functionScopes.size(); - for (std::size_t ii = 0; ii < functions; ++ii) { - const Scope * scope = symbolDatabase->functionScopes[ii]; - - for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { - if (!Token::Match(tok, "%oror%|&&") || !tok->astOperand1() || !tok->astOperand2()) - continue; - - // Opposite comparisons around || or && => always true or always false - if ((tok->astOperand1()->isName() || tok->astOperand2()->isName()) && - isIncorrectCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) { - - const bool alwaysTrue(tok->str() == "||"); - incorrectConditionError(tok, tok->expressionString(), alwaysTrue); - continue; - } - - - // 'A && (!A || B)' is equivalent with 'A && B' - // 'A || (!A && B)' is equivalent with 'A || B' - if (printStyle && - ((tok->str() == "||" && tok->astOperand2()->str() == "&&") || - (tok->str() == "&&" && tok->astOperand2()->str() == "||"))) { - const Token* tok2 = tok->astOperand2()->astOperand1(); - if (isIncorrectCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok2, _settings->library.functionpure)) { - std::string expr1(tok->astOperand1()->expressionString()); - std::string expr2(tok->astOperand2()->astOperand1()->expressionString()); - std::string expr3(tok->astOperand2()->astOperand2()->expressionString()); - - if (expr1.length() + expr2.length() + expr3.length() > 50U) { - if (expr1[0] == '!' && expr2[0] != '!') { - expr1 = "!A"; - expr2 = "A"; - } else { - expr1 = "A"; - expr2 = "!A"; - } - - expr3 = "B"; - } - - const std::string cond1 = expr1 + " " + tok->str() + " (" + expr2 + " " + tok->astOperand2()->str() + " " + expr3 + ")"; - const std::string cond2 = expr1 + " " + tok->str() + " " + expr3; - - redundantConditionError(tok, tok2->expressionString() + ". '" + cond1 + "' is equivalent to '" + cond2 + "'"); - continue; - } - } - - // Comparison #1 (LHS) - const Token *comp1 = tok->astOperand1(); - if (comp1 && comp1->str() == tok->str()) - comp1 = comp1->astOperand2(); - - // Comparison #2 (RHS) - const Token *comp2 = tok->astOperand2(); - - // Parse LHS - bool not1; - std::string op1, value1; - const Token *expr1; - if (!parseComparison(comp1, ¬1, &op1, &value1, &expr1)) - continue; - - // Parse RHS - bool not2; - std::string op2, value2; - const Token *expr2; - if (!parseComparison(comp2, ¬2, &op2, &value2, &expr2)) - continue; - - if (isSameExpression(_tokenizer->isCPP(), comp1, comp2, _settings->library.functionpure)) - continue; // same expressions => only report that there are same expressions - if (!isSameExpression(_tokenizer->isCPP(), expr1, expr2, _settings->library.functionpure)) { - if ((tok->astOperand1()->isConstOp() && tok->astOperand2()->isConstOp()) && - isIncorrectCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) { - - const bool alwaysTrue(tok->str() == "||"); - incorrectConditionError(tok, tok->expressionString(), alwaysTrue); - } - continue; - } - - const bool isfloat = astIsFloat(expr1, true) || MathLib::isFloat(value1) || astIsFloat(expr2, true) || MathLib::isFloat(value2); - - // don't check floating point equality comparisons. that is bad - // and deserves different warnings. - if (isfloat && (op1 == "==" || op1 == "!=" || op2 == "==" || op2 == "!=")) - continue; - - const double d1 = (isfloat) ? MathLib::toDoubleNumber(value1) : 0; - const double d2 = (isfloat) ? MathLib::toDoubleNumber(value2) : 0; - const MathLib::bigint i1 = (isfloat) ? 0 : MathLib::toLongNumber(value1); - const MathLib::bigint i2 = (isfloat) ? 0 : MathLib::toLongNumber(value2); - const bool useUnsignedInt = (std::numeric_limits::max()==i1)||(std::numeric_limits::max()==i2); - const MathLib::biguint u1 = (useUnsignedInt) ? MathLib::toLongNumber(value1) : 0; - const MathLib::biguint u2 = (useUnsignedInt) ? MathLib::toLongNumber(value2) : 0; - // evaluate if expression is always true/false - bool alwaysTrue = true, alwaysFalse = true; - bool firstTrue = true, secondTrue = true; - for (int test = 1; test <= 5; ++test) { - // test: - // 1 => testvalue is less than both value1 and value2 - // 2 => testvalue is value1 - // 3 => testvalue is between value1 and value2 - // 4 => testvalue value2 - // 5 => testvalue is larger than both value1 and value2 - bool result1, result2; - if (isfloat) { - const double testvalue = getvalue(test, d1, d2); - result1 = checkFloatRelation(op1, testvalue, d1); - result2 = checkFloatRelation(op2, testvalue, d2); - } else if (useUnsignedInt) { - const MathLib::biguint testvalue = getvalue(test, u1, u2); - result1 = checkIntRelation(op1, testvalue, u1); - result2 = checkIntRelation(op2, testvalue, u2); - } else { - const MathLib::bigint testvalue = getvalue(test, i1, i2); - result1 = checkIntRelation(op1, testvalue, i1); - result2 = checkIntRelation(op2, testvalue, i2); - } - if (not1) - result1 = !result1; - if (not2) - result2 = !result2; - if (tok->str() == "&&") { - alwaysTrue &= (result1 && result2); - alwaysFalse &= !(result1 && result2); - } else { - alwaysTrue &= (result1 || result2); - alwaysFalse &= !(result1 || result2); - } - firstTrue &= !(!result1 && result2); - secondTrue &= !(result1 && !result2); - } - - const std::string cond1str = conditionString(not1, expr1, op1, value1); - const std::string cond2str = conditionString(not2, expr2, op2, value2); - if (printWarning && (alwaysTrue || alwaysFalse)) { - const std::string text = cond1str + " " + tok->str() + " " + cond2str; - incorrectConditionError(tok, text, alwaysTrue); - } else if (printStyle && secondTrue) { - const std::string text = "If '" + cond1str + "', the comparison '" + cond2str + - "' is always " + (secondTrue ? "true" : "false") + "."; - redundantConditionError(tok, text); - } else if (printStyle && firstTrue) { - //const std::string text = "The comparison " + cond1str + " is always " + - // (firstTrue ? "true" : "false") + " when " + - // cond2str + "."; - const std::string text = "If '" + cond2str + "', the comparison '" + cond1str + - "' is always " + (firstTrue ? "true" : "false") + "."; - redundantConditionError(tok, text); - } - } - } -} - -void CheckCondition::incorrectConditionError(const Token *tok, const std::string &condition, bool always) -{ - if (always) - reportError(tok, Severity::warning, "IncorrectCondition", - "Condition always evaluates to true: " + condition + ".\n" - "Condition always evaluates to true: " + condition + ". " - "Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?"); - else - reportError(tok, Severity::warning, "IncorrectCondition", - "Condition always evaluates to false: " + condition + ".\n" - "Condition always evaluates to false: " + condition + ". " - "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?"); -} - - - void CheckCondition::checkIncorrectLogicOperator() { const bool printStyle = _settings->isEnabled("style"); @@ -994,7 +813,6 @@ void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::st "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?"); } - void CheckCondition::redundantConditionError(const Token *tok, const std::string &text) { reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text); diff --git a/lib/checkcondition.h b/lib/checkcondition.h index 8d48dcb77..acc4e313b 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -50,7 +50,6 @@ public: checkCondition.clarifyCondition(); // not simplified because ifAssign checkCondition.oppositeInnerCondition(); checkCondition.checkIncorrectLogicOperator(); - checkCondition.checkIncorrectCondition(); checkCondition.checkInvalidTestForOverflow(); } @@ -89,7 +88,6 @@ public: /** @brief %Check for testing for mutual exclusion over ||*/ void checkIncorrectLogicOperator(); - void checkIncorrectCondition(); /** @brief %Check for suspicious usage of modulo (e.g. "if(var % 4 == 4)") */ void checkModuloAlwaysTrueFalse(); @@ -120,7 +118,6 @@ private: void oppositeInnerConditionError(const Token *tok1, const Token* tok2); void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always); - void incorrectConditionError(const Token *tok, const std::string &condition, bool always); void redundantConditionError(const Token *tok, const std::string &text); void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal);