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

Fixed #7233 (Fasle negative 'unknownEvaluationOrder in case of macro)

This commit is contained in:
Daniel Marjamäki 2015-12-25 18:31:21 +01:00
parent 27af1bcfd8
commit 24438c326e
5 changed files with 38 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
lib/astutils.cpp
View File

@ -98,7 +98,7 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
return ret;
}
bool isSameExpression(bool cpp, const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions)
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions)
{
if (tok1 == nullptr && tok2 == nullptr)
return true;
@ -113,14 +113,14 @@ bool isSameExpression(bool cpp, const Token *tok1, const Token *tok2, const std:
if (tok1->varId() != tok2->varId() || tok1->str() != tok2->str()) {
if ((Token::Match(tok1,"<|>") && Token::Match(tok2,"<|>")) ||
(Token::Match(tok1,"<=|>=") && Token::Match(tok2,"<=|>="))) {
return isSameExpression(cpp, tok1->astOperand1(), tok2->astOperand2(), constFunctions) &&
isSameExpression(cpp, tok1->astOperand2(), tok2->astOperand1(), constFunctions);
return isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), constFunctions) &&
isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), constFunctions);
}
return false;
}
if (tok1->str() == "." && tok1->originalName() != tok2->originalName())
return false;
if (tok1->isExpandedMacro() || tok2->isExpandedMacro())
if (macro && (tok1->isExpandedMacro() || tok2->isExpandedMacro()))
return false;
if (tok1->isName() && tok1->next()->str() == "(" && tok1->str() != "sizeof") {
if (!tok1->function() && !Token::Match(tok1->previous(), ".|::") && constFunctions.find(tok1->str()) == constFunctions.end() && !tok1->isAttributeConst() && !tok1->isAttributePure())
@ -168,18 +168,18 @@ bool isSameExpression(bool cpp, const Token *tok1, const Token *tok2, const std:
return false;
}
bool noncommuative_equals =
isSameExpression(cpp, tok1->astOperand1(), tok2->astOperand1(), constFunctions);
isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand1(), constFunctions);
noncommuative_equals = noncommuative_equals &&
isSameExpression(cpp, tok1->astOperand2(), tok2->astOperand2(), constFunctions);
isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand2(), constFunctions);
if (noncommuative_equals)
return true;
const bool commutative = tok1->astOperand1() && tok1->astOperand2() && Token::Match(tok1, "%or%|%oror%|+|*|&|&&|^|==|!=");
bool commuative_equals = commutative &&
isSameExpression(cpp, tok1->astOperand2(), tok2->astOperand1(), constFunctions);
isSameExpression(cpp, macro, tok1->astOperand2(), tok2->astOperand1(), constFunctions);
commuative_equals = commuative_equals &&
isSameExpression(cpp, tok1->astOperand1(), tok2->astOperand2(), constFunctions);
isSameExpression(cpp, macro, tok1->astOperand1(), tok2->astOperand2(), constFunctions);
// in c++, "a"+b might be different to b+"a"
if (cpp && commuative_equals && tok1->str() == "+" &&
@ -199,11 +199,11 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
if (cond1->str() == "!") {
if (cond2->str() == "!=") {
if (cond2->astOperand1() && cond2->astOperand1()->str() == "0")
return isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand2(), constFunctions);
return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), constFunctions);
if (cond2->astOperand2() && cond2->astOperand2()->str() == "0")
return isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand1(), constFunctions);
return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), constFunctions);
}
return isSameExpression(cpp, cond1->astOperand1(), cond2, constFunctions);
return isSameExpression(cpp, true, cond1->astOperand1(), cond2, constFunctions);
}
if (cond2->str() == "!")
@ -216,11 +216,11 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
// condition found .. get comparator
std::string comp2;
if (isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand1(), constFunctions) &&
isSameExpression(cpp, cond1->astOperand2(), cond2->astOperand2(), constFunctions)) {
if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), constFunctions) &&
isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand2(), constFunctions)) {
comp2 = cond2->str();
} else if (isSameExpression(cpp, cond1->astOperand1(), cond2->astOperand2(), constFunctions) &&
isSameExpression(cpp, cond1->astOperand2(), cond2->astOperand1(), constFunctions)) {
} else if (isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand2(), constFunctions) &&
isSameExpression(cpp, true, cond1->astOperand2(), cond2->astOperand1(), constFunctions)) {
comp2 = cond2->str();
if (comp2[0] == '>')
comp2[0] = '<';

2
lib/astutils.h
View File

@ -48,7 +48,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, const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions);
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions);
/**
* Are two conditions opposite

12
lib/checkcondition.cpp
View File

@ -326,7 +326,7 @@ bool CheckCondition::isOverlappingCond(const Token * const cond1, const Token *
return false;
// same expressions
if (isSameExpression(_tokenizer->isCPP(), cond1,cond2,constFunctions))
if (isSameExpression(_tokenizer->isCPP(), true, cond1, cond2, constFunctions))
return true;
// bitwise overlap for example 'x&7' and 'x==1'
@ -349,7 +349,7 @@ bool CheckCondition::isOverlappingCond(const Token * const cond1, const Token *
if (!num2->isNumber() || MathLib::isNegative(num2->str()))
return false;
if (!isSameExpression(_tokenizer->isCPP(), expr1,expr2,constFunctions))
if (!isSameExpression(_tokenizer->isCPP(), true, expr1, expr2, constFunctions))
return false;
const MathLib::bigint value1 = MathLib::toLongNumber(num1->str());
@ -720,9 +720,9 @@ void CheckCondition::checkIncorrectLogicOperator()
if (!parseComparison(comp2, &not2, &op2, &value2, &expr2))
continue;
if (isSameExpression(_tokenizer->isCPP(), comp1, comp2, _settings->library.functionpure))
if (isSameExpression(_tokenizer->isCPP(), true, 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 (!isSameExpression(_tokenizer->isCPP(), true, expr1, expr2, _settings->library.functionpure))
continue;
const bool isfloat = astIsFloat(expr1, true) || MathLib::isFloat(value1) || astIsFloat(expr2, true) || MathLib::isFloat(value2);
@ -1049,9 +1049,9 @@ void CheckCondition::checkInvalidTestForOverflow()
continue;
const Token *termToken;
if (isSameExpression(_tokenizer->isCPP(), exprToken, calcToken->astOperand1(), _settings->library.functionpure))
if (isSameExpression(_tokenizer->isCPP(), true, exprToken, calcToken->astOperand1(), _settings->library.functionpure))
termToken = calcToken->astOperand2();
else if (isSameExpression(_tokenizer->isCPP(), exprToken, calcToken->astOperand2(), _settings->library.functionpure))
else if (isSameExpression(_tokenizer->isCPP(), true, exprToken, calcToken->astOperand2(), _settings->library.functionpure))
termToken = calcToken->astOperand1();
else
continue;

17
lib/checkother.cpp
View File

@ -1890,7 +1890,7 @@ void CheckOther::checkDuplicateExpression()
if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>|+=|*=|<<=|>>=")) {
if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true))
continue;
if (isSameExpression(_tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
if (isSameExpression(_tokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
if (isWithoutSideEffects(_tokenizer->isCPP(), tok->astOperand1())) {
const bool assignment = tok->str() == "=";
if (assignment && warningEnabled)
@ -1910,16 +1910,16 @@ void CheckOther::checkDuplicateExpression()
}
}
} else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative
if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(_tokenizer->isCPP(), tok->astOperand2(), tok->astOperand1()->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer->isCPP(), tok->astOperand2()))
if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(_tokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer->isCPP(), tok->astOperand2()))
duplicateExpressionError(tok->astOperand2(), tok->astOperand2(), tok->str());
else if (tok->astOperand2()) {
const Token *ast1 = tok->astOperand1();
while (ast1 && tok->str() == ast1->str()) {
if (isSameExpression(_tokenizer->isCPP(), ast1->astOperand1(), tok->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer->isCPP(), ast1->astOperand1()))
if (isSameExpression(_tokenizer->isCPP(), true, ast1->astOperand1(), tok->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer->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(_tokenizer->isCPP(), ast1->astOperand2(), tok->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer->isCPP(), ast1->astOperand2()))
else if (styleEnabled && isSameExpression(_tokenizer->isCPP(), true, ast1->astOperand2(), tok->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer->isCPP(), ast1->astOperand2()))
duplicateExpressionError(ast1->astOperand2(), tok->astOperand2(), tok->str());
if (!isConstExpression(ast1->astOperand2(), _settings->library.functionpure))
break;
@ -1928,7 +1928,7 @@ void CheckOther::checkDuplicateExpression()
}
}
} else if (styleEnabled && tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") {
if (isSameExpression(_tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), temp))
if (isSameExpression(_tokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), temp))
duplicateExpressionTernaryError(tok);
}
}
@ -2515,8 +2515,9 @@ void CheckOther::checkEvaluationOrder()
if (tok2 == tok &&
tok->str() == "=" &&
parent->str() == "=" &&
isSameExpression(_tokenizer->isCPP(), tok->astOperand1(), parent->astOperand1(), _settings->library.functionpure)) {
if (_settings->isEnabled("warning"))
isSameExpression(_tokenizer->isCPP(), false, tok->astOperand1(), parent->astOperand1(), _settings->library.functionpure)) {
if (_settings->isEnabled("warning") &&
isSameExpression(_tokenizer->isCPP(), true, tok->astOperand1(), parent->astOperand1(), _settings->library.functionpure))
selfAssignmentError(parent, tok->astOperand1()->expressionString());
break;
}
@ -2532,7 +2533,7 @@ void CheckOther::checkEvaluationOrder()
continue;
tokens.push(tok3->astOperand1());
tokens.push(tok3->astOperand2());
if (isSameExpression(_tokenizer->isCPP(), tok->astOperand1(), tok3, _settings->library.functionpure)) {
if (isSameExpression(_tokenizer->isCPP(), false, tok->astOperand1(), tok3, _settings->library.functionpure)) {
foundError = true;
}
}

7
test/testother.cpp
View File

@ -6118,6 +6118,13 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Redundant assignment of 'x' to itself.\n", errout.str());
// macro, dont bailout (#7233)
check((std::string("void f(int x) {\n"
" return x + ") + Preprocessor::macroChar + "x++;\n"
"}").c_str());
ASSERT_EQUALS("[test.cpp:2]: (error) Expression 'x+x++' depends on order of evaluation of side effects\n", errout.str());
// sequence points
{
// FP