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

Merge pull request #59 from richq/sameexpr 

Improved same expression check for ticket #3274
This commit is contained in:
Daniel Marjamäki 2011-11-08 23:56:36 -08:00
parent 3b11ae3fac d5c7c5d098
commit b7cc9779c4
4 changed files with 160 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

108
lib/checkother.cpp
View File

@ -2339,6 +2339,105 @@ void CheckOther::duplicateBranchError(const Token *tok1, const Token *tok2)
"carefully to determine if it is correct.");
}
namespace {
class Expressions {
public:
void endExpr() {
const std::string &e = _expression.str();
if (!e.empty()) {
std::map<std::string,int>::const_iterator it = _expressions.find(e);
if (it == _expressions.end())
_expressions[e] = 1;
else
_expressions[e] += 1;
}
_expression.str("");
}
void append(const std::string &tok) {
_expression << tok;
}
std::map<std::string,int> &getMap() {
return _expressions;
}
private:
std::map<std::string, int> _expressions;
std::ostringstream _expression;
};
}
void CheckOther::checkExpressionRange(const Token *start, const Token *end, const std::string &toCheck)
{
if (!start || !end)
return;
Expressions expressions;
std::string opName;
for (const Token *tok = start->next(); tok && tok != end; tok = tok->next()) {
if (Token::Match(tok, toCheck.c_str())) {
opName = tok->str();
expressions.endExpr();
} else {
expressions.append(tok->str());
}
}
expressions.endExpr();
std::map<std::string,int>::const_iterator it = expressions.getMap().begin();
for (; it != expressions.getMap().end(); ++it) {
if (it->second > 1)
duplicateExpressionError(start, start, opName);
}
}
void CheckOther::complexDuplicateExpressionCheck(const Token *classStart,
const std::string &toCheck,
const std::string &alt)
{
std::string statementStart(",|=|return");
if (!alt.empty())
statementStart += "|" + alt;
std::string statementEnd(";|,");
if (!alt.empty())
statementEnd += "|" + alt;
for (const Token *tok = classStart; tok && tok != classStart->link(); tok = tok->next()) {
if (!Token::Match(tok, toCheck.c_str()))
continue;
// look backward for the start of the statement
const Token *start = 0;
int level = 0;
for (const Token *tok1 = tok->previous(); tok1 && tok1 != classStart; tok1 = tok1->previous()) {
if (tok1->str() == ")")
level++;
else if (tok1->str() == "(")
level--;
if (level < 0 || Token::Match(tok1, statementStart.c_str())) {
start = tok1;
break;
}
}
const Token *end = 0;
level = 0;
// look for the end of the statement
for (const Token *tok1 = tok->next(); tok1 && tok1 != classStart->link(); tok1 = tok1->next()) {
if (tok1->str() == ")")
level--;
else if (tok1->str() == "(")
level++;
if (level < 0 || Token::Match(tok1, statementEnd.c_str())) {
end = tok1;
break;
}
}
checkExpressionRange(start, end, toCheck);
}
}
//---------------------------------------------------------------------------
// check for the same expression on both sides of an operator
// (x == x), (x && x), (x || x)
@ -2359,8 +2458,13 @@ void CheckOther::checkDuplicateExpression()
if (scope->type != Scope::eFunction)
continue;
complexDuplicateExpressionCheck(scope->classStart, "%or%", "");
complexDuplicateExpressionCheck(scope->classStart, "%oror%", "");
complexDuplicateExpressionCheck(scope->classStart, "&", "%oror%|%or%");
complexDuplicateExpressionCheck(scope->classStart, "&&", "%oror%|%or%");
for (const Token *tok = scope->classStart; tok && tok != scope->classStart->link(); tok = tok->next()) {
if (Token::Match(tok, ",|=|return|(|&&|%oror% %var% &&|%oror%|==|!=|<=|>=|<|>|-|%or% %var% )|&&|%oror%|;") &&
if (Token::Match(tok, ",|=|return|(|&&|%oror% %var% ==|!=|<=|>=|<|>|- %var% )|&&|%oror%|;|,") &&
tok->strAt(1) == tok->strAt(3)) {
// float == float and float != float are valid NaN checks
if (Token::Match(tok->tokAt(2), "==|!=") && tok->next()->varId()) {
@ -2372,7 +2476,7 @@ void CheckOther::checkDuplicateExpression()
}
duplicateExpressionError(tok->next(), tok->tokAt(3), tok->strAt(2));
} else if (Token::Match(tok, ",|=|return|(|&&|%oror% %var% . %var% &&|%oror%|==|!=|<=|>=|<|>|-|%or% %var% . %var% )|&&|%oror%") &&
} else if (Token::Match(tok, ",|=|return|(|&&|%oror% %var% . %var% ==|!=|<=|>=|<|>|- %var% . %var% )|&&|%oror%|;|,") &&
tok->strAt(1) == tok->strAt(5) && tok->strAt(3) == tok->strAt(7)) {
duplicateExpressionError(tok->next(), tok->tokAt(6), tok->strAt(4));
}

5
lib/checkother.h
View File

@ -426,6 +426,11 @@ private:
return varname;
}
void checkExpressionRange(const Token *start, const Token *end, const std::string &toCheck);
void complexDuplicateExpressionCheck(const Token *classStart,
const std::string &toCheck,
const std::string &alt);
};
/// @}
//---------------------------------------------------------------------------

4
lib/mathlib.cpp
View File

@ -316,10 +316,10 @@ bool MathLib::isNullValue(const std::string &str)
|| str == "-0E+00" || str == "+0E+00"
|| str == "+0E-00" || str == "+0"
|| str == "+0.0" || str == "+0."
|| str == "0.0" || str == "-0e-00"
|| str == "0.0"
|| str == "+0e+00" || str == "-0e+00"
|| str == "+0e-00" || str == "-0e-00"
|| str == "-0E-0" || str == "+0E-00");
|| str == "-0E-0");
}
bool MathLib::isOctalDigit(char c)

50
test/testother.cpp
View File

@ -2247,7 +2247,7 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
check("void f(int x) {\n"
" if (x == 1 && x == 3)\n"
@ -3433,9 +3433,53 @@ private:
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
check("void foo() {\n"
" f(a,b && b);\n"
" f(a,b == b);\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
check("void foo() {\n"
" f(b == b, a);\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
check("void foo() {\n"
" if (x!=2 || x!=2) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n"
" if (x!=2 || x!=3 || x!=2) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n"
" if (this->bar() || this->bar()) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n"
" if (a && b || a && b) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n"
" if (a && b || b && c) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void foo() {\n"
" if (a && b | b && c) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void foo() {\n"
" if ((a + b) | (a + b)) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '|'.\n", errout.str());
check("void foo() {\n"
" if ((a | b) & (a | b)) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&'.\n", errout.str());
}
void duplicateExpression2() { // ticket #2730