Fixed #7230 (Confusing code snippet in error message)

This commit is contained in:
Daniel Marjamäki 2015-12-31 01:15:49 +01:00
parent fae9c2159c
commit 8171154e12
11 changed files with 173 additions and 100 deletions

View File

@ -275,7 +275,7 @@ bool isVariableChanged(const Token *start, const Token *end, const unsigned int
{
for (const Token *tok = start; tok != end; tok = tok->next()) {
if (tok->varId() == varid) {
if (Token::Match(tok, "%name% =|++|--"))
if (Token::Match(tok, "%name% %assign%|++|--"))
return true;
if (Token::Match(tok->previous(), "++|-- %name%"))

View File

@ -442,13 +442,13 @@ void CheckCondition::oppositeInnerCondition()
break;
else if ((tok->varId() && vars.find(tok->varId()) != vars.end()) ||
(!tok->varId() && nonlocal)) {
if (Token::Match(tok, "%name% ++|--|="))
if (Token::Match(tok, "%name% %assign%|++|--"))
break;
if (Token::Match(tok, "%name% [")) {
const Token *tok2 = tok->linkAt(1);
while (Token::simpleMatch(tok2, "] ["))
tok2 = tok2->linkAt(1);
if (Token::simpleMatch(tok2, "] ="))
if (Token::Match(tok2, "] %assign%|++|--"))
break;
}
if (Token::Match(tok->previous(), "++|--|& %name%"))

View File

@ -540,7 +540,7 @@ void CheckOther::checkRedundantAssignment()
}
std::map<unsigned int, const Token*>::iterator it = varAssignments.find(tok->varId());
if (tok->next() && tok->next()->isAssignmentOp() && Token::Match(startToken, "[;{}]")) { // Assignment
if (Token::simpleMatch(tok->next(), "=") && Token::Match(startToken, "[;{}]")) { // Assignment
if (it != varAssignments.end()) {
bool error = true; // Ensure that variable is not used on right side
for (const Token* tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) {
@ -733,6 +733,32 @@ void CheckOther::checkRedundantAssignmentInSwitch()
// Bitwise operation. Report an error if it's performed twice before a break. E.g.:
// case 3: b |= 1; // <== redundant
// case 4: b |= 1;
else if (Token::Match(tok2->previous(), ";|{|}|: %var% %assign% %num% ;") &&
(tok2->strAt(1) == "|=" || tok2->strAt(1) == "&=") &&
Token::Match(tok2->next()->astOperand2(), "%num%")) {
std::string bitOp = tok2->strAt(1)[0] + tok2->strAt(2);
std::map<unsigned int, const Token*>::const_iterator i2 = varsWithBitsSet.find(tok2->varId());
// This variable has not had a bit operation performed on it yet, so just make a note of it
if (i2 == varsWithBitsSet.end()) {
varsWithBitsSet[tok2->varId()] = tok2;
bitOperations[tok2->varId()] = bitOp;
}
// The same bit operation has been performed on the same variable twice, so report an error
else if (bitOperations[tok2->varId()] == bitOp)
redundantBitwiseOperationInSwitchError(i2->second, i2->second->str());
// A different bit operation was performed on the variable, so clear it
else {
varsWithBitsSet.erase(tok2->varId());
bitOperations.erase(tok2->varId());
}
}
// Bitwise operation. Report an error if it's performed twice before a break. E.g.:
// case 3: b = b | 1; // <== redundant
// case 4: b = b | 1;
else if (Token::Match(tok2->previous(), ";|{|}|: %var% = %name% %or%|& %num% ;") &&
tok2->varId() == tok2->tokAt(2)->varId()) {
std::string bitOp = tok2->strAt(3) + tok2->strAt(4);
@ -2186,7 +2212,7 @@ static bool isNegative(const Token *tok, const Settings *settings)
void CheckOther::checkNegativeBitwiseShift()
{
for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (tok->str() != "<<" && tok->str() != ">>")
if (!Token::Match(tok, "<<|>>|<<=|>>="))
continue;
if (!tok->astOperand1() || !tok->astOperand2())

View File

@ -417,6 +417,9 @@ static const Token* doAssignment(Variables &variables, const Token *tok, bool de
return tok->tokAt(2);
}
if (Token::Match(tok, "%var% %assign%") && tok->strAt(1) != "=")
return tok->next();
const Token* const tokOld = tok;
// check for aliased variable
@ -426,7 +429,7 @@ static const Token* doAssignment(Variables &variables, const Token *tok, bool de
if (var1) {
// jump behind '='
tok = tok->next();
while (tok->str() != "=") {
while (!tok->isAssignmentOp()) {
if (tok->varId())
variables.read(tok->varId(), tok);
tok = tok->next();
@ -862,8 +865,8 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
variables.use(tok->tokAt(2)->varId(), tok);
}
// assignment
else if (Token::Match(tok, "*| ++|--| %name% ++|--| =") ||
Token::Match(tok, "*| ( const| %type% *| ) %name% =")) {
else if (Token::Match(tok, "*| ++|--| %name% ++|--| %assign%") ||
Token::Match(tok, "*| ( const| %type% *| ) %name% %assign%")) {
bool dereference = false;
bool pre = false;
bool post = false;
@ -873,7 +876,7 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
tok = tok->next();
}
if (Token::Match(tok, "( const| %type% *| ) %name% ="))
if (Token::Match(tok, "( const| %type% *| ) %name% %assign%"))
tok = tok->link()->next();
else if (tok->str() == "(")
@ -892,6 +895,12 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
tok = doAssignment(variables, tok, dereference, scope);
if (tok && tok->isAssignmentOp() && tok->str() != "=") {
variables.use(varid1, tok);
if (Token::Match(tok, "%assign% %name%"))
tok = tok->next();
}
if (pre || post)
variables.use(varid1, tok);
@ -939,19 +948,19 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const
} else {
variables.write(varid1, tok);
}
}
Variables::VariableUsage *var2 = variables.find(tok->varId());
if (var2) {
if (var2->_type == Variables::reference) {
variables.writeAliases(tok->varId(), tok);
variables.read(tok->varId(), tok);
} else if (tok->varId() != varid1 && Token::Match(tok, "%name% ."))
variables.read(tok->varId(), tok);
else if (tok->varId() != varid1 &&
var2->_type == Variables::standard &&
tok->strAt(-1) != "&")
variables.use(tok->varId(), tok);
}
Variables::VariableUsage *var2 = variables.find(tok->varId());
if (var2) {
if (var2->_type == Variables::reference) {
variables.writeAliases(tok->varId(), tok);
variables.read(tok->varId(), tok);
} else if (tok->varId() != varid1 && Token::Match(tok, "%name% .|["))
variables.read(tok->varId(), tok);
else if (tok->varId() != varid1 &&
var2->_type == Variables::standard &&
tok->strAt(-1) != "&")
variables.use(tok->varId(), tok);
}
const Token * const equal = skipBracketsAndMembers(tok->next());

View File

@ -362,11 +362,18 @@ static int multiComparePercent(const Token *tok, const char*& haystack, bool emp
}
break;
case 'a':
// Accept any token (%any%)
// Accept any token (%any%) or assign (%assign%)
{
haystack += 4;
return 1;
if (haystack[3] == '%') { // %any%
haystack += 4;
return 1;
} else { // %assign%
haystack += 7;
if (tok->isAssignmentOp())
return 1;
}
}
break;
case 'n':
// Number (%num%) or name (%name%)
{

View File

@ -3480,9 +3480,6 @@ bool Tokenizer::simplifyTokenList1(const char FileName[])
// simplify '[;{}] * & ( %any% ) =' to '%any% ='
simplifyMulAndParens();
// ";a+=b;" => ";a=a+b;"
simplifyCompoundAssignment();
if (!isC() && !_settings->library.markupFile(FileName)) {
findComplicatedSyntaxErrorsInTemplates();
}
@ -3720,6 +3717,9 @@ bool Tokenizer::simplifyTokenList2()
// f(x=g()) => x=g(); f(x)
simplifyAssignmentInFunctionCall();
// ";a+=b;" => ";a=a+b;"
simplifyCompoundAssignment();
simplifyCharAt();
// simplify references

View File

@ -50,6 +50,9 @@ private:
// foo(p = new char[10]); => p = new char[10]; foo(p);
TEST_CASE(simplifyAssignmentInFunctionCall);
// ";a+=b;" => ";a=a+b;"
TEST_CASE(simplifyCompoundAssignment);
TEST_CASE(cast);
TEST_CASE(iftruefalse);
TEST_CASE(combine_strings);
@ -343,12 +346,73 @@ private:
tok(";while((x=f())==-1 && errno==EINTR){}",true));
}
void simplifyAssignmentInFunctionCall() {
ASSERT_EQUALS("; x = g ( ) ; f ( x ) ;", tok(";f(x=g());"));
ASSERT_EQUALS("; hs = ( xyz_t ) { h . centerX , h . centerY , 1 + index } ; putInput ( hs , 1 ) ;", tok(";putInput(hs = (xyz_t) { h->centerX, h->centerY, 1 + index }, 1);"));
}
void simplifyCompoundAssignment() {
ASSERT_EQUALS("; x = x + y ;", tok("; x += y;"));
ASSERT_EQUALS("; x = x - y ;", tok("; x -= y;"));
ASSERT_EQUALS("; x = x * y ;", tok("; x *= y;"));
ASSERT_EQUALS("; x = x / y ;", tok("; x /= y;"));
ASSERT_EQUALS("; x = x % y ;", tok("; x %= y;"));
ASSERT_EQUALS("; x = x & y ;", tok("; x &= y;"));
ASSERT_EQUALS("; x = x | y ;", tok("; x |= y;"));
ASSERT_EQUALS("; x = x ^ y ;", tok("; x ^= y;"));
ASSERT_EQUALS("; x = x << y ;", tok("; x <<= y;"));
ASSERT_EQUALS("; x = x >> y ;", tok("; x >>= y;"));
ASSERT_EQUALS("{ x = x + y ; }", tok("{ x += y;}"));
ASSERT_EQUALS("{ x = x - y ; }", tok("{ x -= y;}"));
ASSERT_EQUALS("{ x = x * y ; }", tok("{ x *= y;}"));
ASSERT_EQUALS("{ x = x / y ; }", tok("{ x /= y;}"));
ASSERT_EQUALS("{ x = x % y ; }", tok("{ x %= y;}"));
ASSERT_EQUALS("{ x = x & y ; }", tok("{ x &= y;}"));
ASSERT_EQUALS("{ x = x | y ; }", tok("{ x |= y;}"));
ASSERT_EQUALS("{ x = x ^ y ; }", tok("{ x ^= y;}"));
ASSERT_EQUALS("{ x = x << y ; }", tok("{ x <<= y;}"));
ASSERT_EQUALS("{ x = x >> y ; }", tok("{ x >>= y;}"));
ASSERT_EQUALS("; * p = * p + y ;", tok("; *p += y;"));
ASSERT_EQUALS("; ( * p ) = ( * p ) + y ;", tok("; (*p) += y;"));
ASSERT_EQUALS("; * ( p [ 0 ] ) = * ( p [ 0 ] ) + y ;", tok("; *(p[0]) += y;"));
ASSERT_EQUALS("; p [ { 1 , 2 } ] = p [ { 1 , 2 } ] + y ;", tok("; p[{1,2}] += y;"));
ASSERT_EQUALS("void foo ( ) { switch ( n ) { case 0 : ; x = x + y ; break ; } }", tok("void foo() { switch (n) { case 0: x += y; break; } }"));
ASSERT_EQUALS("; x . y = x . y + 1 ;", tok("; x.y += 1;"));
ASSERT_EQUALS("; x [ 0 ] = x [ 0 ] + 1 ;", tok("; x[0] += 1;"));
ASSERT_EQUALS("; x [ y - 1 ] = x [ y - 1 ] + 1 ;", tok("; x[y-1] += 1;"));
ASSERT_EQUALS("; x [ y ] = x [ y ++ ] + 1 ;", tok("; x[y++] += 1;"));
ASSERT_EQUALS("; x [ ++ y ] = x [ y ] + 1 ;", tok("; x[++y] += 1;"));
ASSERT_EQUALS(";", tok(";x += 0;"));
ASSERT_EQUALS(";", tok(";x += '\\0';"));
ASSERT_EQUALS(";", tok(";x -= 0;"));
ASSERT_EQUALS(";", tok(";x |= 0;"));
ASSERT_EQUALS(";", tok(";x *= 1;"));
ASSERT_EQUALS(";", tok(";x /= 1;"));
ASSERT_EQUALS("; a . x ( ) = a . x ( ) + 1 ;", tok("; a.x() += 1;"));
ASSERT_EQUALS("; x ( 1 ) = x ( 1 ) + 1 ;", tok("; x(1) += 1;"));
// #2368
ASSERT_EQUALS("{ j = j - i ; }", tok("if (false) {} else { j -= i; }"));
// #2714 - wrong simplification of "a += b?c:d;"
ASSERT_EQUALS("; a = a + ( b ? c : d ) ;", tok("; a+=b?c:d;"));
ASSERT_EQUALS("; a = a * ( b + 1 ) ;", tok("; a*=b+1;"));
ASSERT_EQUALS("; a = a + ( b && c ) ;", tok("; a+=b&&c;"));
ASSERT_EQUALS("; a = a * ( b || c ) ;", tok("; a*=b||c;"));
ASSERT_EQUALS("; a = a | ( b == c ) ;", tok("; a|=b==c;"));
// #3469
ASSERT_EQUALS("; a = a + ( b = 1 ) ;", tok("; a += b = 1;"));
}
void cast() {
ASSERT_EQUALS("if ( p == 0 ) { ; }", tok("if (p == (char *)0);"));
@ -1715,7 +1779,7 @@ private:
}
void cAlternativeTokens() {
ASSERT_EQUALS("void f ( ) { err = err | ( ( r & s ) && ! t ) ; }",
ASSERT_EQUALS("void f ( ) { err |= ( ( r & s ) && ! t ) ; }",
tok("void f() { err or_eq ((r bitand s) and not t); }", "test.c", false));
ASSERT_EQUALS("void f ( ) const { r = f ( a [ 4 ] | 15 , ~ c , ! d ) ; }",
tok("void f() const { r = f(a[4] bitor 0x0F, compl c, not d) ; }", "test.c", false));

View File

@ -609,6 +609,15 @@ private:
ASSERT_EQUALS("[test.cpp:4]: (portability) 'p1' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.\n"
"[test.cpp:5]: (portability) 'p2' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.\n", errout.str());
check("void f() {\n"
" void* p1 = malloc(10);\n"
" void* p2 = malloc(5);\n"
" p1-=4;\n"
" p2+=4;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (portability) 'p1' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.\n"
"[test.cpp:5]: (portability) 'p2' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.\n", errout.str());
check("void f() {\n"
" void* p = malloc(10);\n"
" int* p2 = &p + 4;\n"

View File

@ -363,9 +363,6 @@ private:
TEST_CASE(simplifyCalculations);
// "x += .." => "x = x + .."
TEST_CASE(simplifyCompoundAssignment);
// x = ({ 123; }); => { x = 123; }
TEST_CASE(simplifyRoundCurlyParentheses);
@ -3490,7 +3487,7 @@ private:
" n12 = open ( ) ;"
" n13 = open ( ) ;"
" n14 = open ( ) ;"
" n15 = n15 + 10 ; "
" n15 += 10 ; "
"}";
ASSERT_EQUALS(expected, tokenizeAndStringify(code));
}
@ -5662,68 +5659,6 @@ private:
ASSERT_EQUALS("dostuff ( 1 ) ;", tokenizeAndStringify("dostuff(9&&8);", true));
}
void simplifyCompoundAssignment() {
ASSERT_EQUALS("; x = x + y ;", tokenizeAndStringify("; x += y;"));
ASSERT_EQUALS("; x = x - y ;", tokenizeAndStringify("; x -= y;"));
ASSERT_EQUALS("; x = x * y ;", tokenizeAndStringify("; x *= y;"));
ASSERT_EQUALS("; x = x / y ;", tokenizeAndStringify("; x /= y;"));
ASSERT_EQUALS("; x = x % y ;", tokenizeAndStringify("; x %= y;"));
ASSERT_EQUALS("; x = x & y ;", tokenizeAndStringify("; x &= y;"));
ASSERT_EQUALS("; x = x | y ;", tokenizeAndStringify("; x |= y;"));
ASSERT_EQUALS("; x = x ^ y ;", tokenizeAndStringify("; x ^= y;"));
ASSERT_EQUALS("; x = x << y ;", tokenizeAndStringify("; x <<= y;"));
ASSERT_EQUALS("; x = x >> y ;", tokenizeAndStringify("; x >>= y;"));
ASSERT_EQUALS("{ x = x + y ; }", tokenizeAndStringify("{ x += y;}"));
ASSERT_EQUALS("{ x = x - y ; }", tokenizeAndStringify("{ x -= y;}"));
ASSERT_EQUALS("{ x = x * y ; }", tokenizeAndStringify("{ x *= y;}"));
ASSERT_EQUALS("{ x = x / y ; }", tokenizeAndStringify("{ x /= y;}"));
ASSERT_EQUALS("{ x = x % y ; }", tokenizeAndStringify("{ x %= y;}"));
ASSERT_EQUALS("{ x = x & y ; }", tokenizeAndStringify("{ x &= y;}"));
ASSERT_EQUALS("{ x = x | y ; }", tokenizeAndStringify("{ x |= y;}"));
ASSERT_EQUALS("{ x = x ^ y ; }", tokenizeAndStringify("{ x ^= y;}"));
ASSERT_EQUALS("{ x = x << y ; }", tokenizeAndStringify("{ x <<= y;}"));
ASSERT_EQUALS("{ x = x >> y ; }", tokenizeAndStringify("{ x >>= y;}"));
ASSERT_EQUALS("; * p = * p + y ;", tokenizeAndStringify("; *p += y;"));
ASSERT_EQUALS("; ( * p ) = ( * p ) + y ;", tokenizeAndStringify("; (*p) += y;"));
ASSERT_EQUALS("; * ( p [ 0 ] ) = * ( p [ 0 ] ) + y ;", tokenizeAndStringify("; *(p[0]) += y;"));
ASSERT_EQUALS("; p [ { 1 , 2 } ] = p [ { 1 , 2 } ] + y ;", tokenizeAndStringify("; p[{1,2}] += y;"));
ASSERT_EQUALS("void foo ( ) { switch ( n ) { case 0 : ; x = x + y ; break ; } }", tokenizeAndStringify("void foo() { switch (n) { case 0: x += y; break; } }"));
ASSERT_EQUALS("; x . y = x . y + 1 ;", tokenizeAndStringify("; x.y += 1;"));
ASSERT_EQUALS("; x [ 0 ] = x [ 0 ] + 1 ;", tokenizeAndStringify("; x[0] += 1;"));
ASSERT_EQUALS("; x [ y - 1 ] = x [ y - 1 ] + 1 ;", tokenizeAndStringify("; x[y-1] += 1;"));
ASSERT_EQUALS("; x [ y ] = x [ y ++ ] + 1 ;", tokenizeAndStringify("; x[y++] += 1;"));
ASSERT_EQUALS("; x [ ++ y ] = x [ y ] + 1 ;", tokenizeAndStringify("; x[++y] += 1;"));
ASSERT_EQUALS(";", tokenizeAndStringify(";x += 0;"));
ASSERT_EQUALS(";", tokenizeAndStringify(";x += '\\0';"));
ASSERT_EQUALS(";", tokenizeAndStringify(";x -= 0;"));
ASSERT_EQUALS(";", tokenizeAndStringify(";x |= 0;"));
ASSERT_EQUALS(";", tokenizeAndStringify(";x *= 1;"));
ASSERT_EQUALS(";", tokenizeAndStringify(";x /= 1;"));
ASSERT_EQUALS("; a . x ( ) = a . x ( ) + 1 ;", tokenizeAndStringify("; a.x() += 1;"));
ASSERT_EQUALS("; x ( 1 ) = x ( 1 ) + 1 ;", tokenizeAndStringify("; x(1) += 1;"));
// #2368
ASSERT_EQUALS("if ( false ) { } else { j = j - i ; }", tokenizeAndStringify("if (false) {} else { j -= i; }"));
// #2714 - wrong simplification of "a += b?c:d;"
ASSERT_EQUALS("; a = a + ( b ? c : d ) ;", tokenizeAndStringify("; a+=b?c:d;"));
ASSERT_EQUALS("; a = a * ( b + 1 ) ;", tokenizeAndStringify("; a*=b+1;"));
ASSERT_EQUALS("; a = a + ( b && c ) ;", tokenizeAndStringify("; a+=b&&c;"));
ASSERT_EQUALS("; a = a * ( b || c ) ;", tokenizeAndStringify("; a*=b||c;"));
ASSERT_EQUALS("; a = a | ( b == c ) ;", tokenizeAndStringify("; a|=b==c;"));
// #3469
ASSERT_EQUALS("; a = a + ( b = 1 ) ;", tokenizeAndStringify("; a += b = 1;"));
}
void simplifyRoundCurlyParentheses() {
ASSERT_EQUALS("; x = 123 ;", tokenizeAndStringify(";x=({123;});"));
ASSERT_EQUALS("; x = y ;", tokenizeAndStringify(";x=({y;});"));

View File

@ -746,7 +746,7 @@ private:
" d += code;\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:7]: (style) Variable 'd' is assigned a value that is never used.\n", errout.str());
TODO_ASSERT_EQUALS("[test.cpp:7]: (style) Variable 'd' is assigned a value that is never used.\n", "", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
@ -805,7 +805,7 @@ private:
" d += code;\n"
" } while(code < 20);\n"
"}");
ASSERT_EQUALS("[test.cpp:7]: (style) Variable 'd' is assigned a value that is never used.\n", errout.str());
TODO_ASSERT_EQUALS("[test.cpp:7]: (style) Variable 'd' is assigned a value that is never used.\n", "", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
@ -1708,9 +1708,7 @@ private:
" C c;\n"
" if (c >>= x) {}\n"
"}", "test.c");
TODO_ASSERT_EQUALS("[test.c:2]: (style) Variable 'x' is not assigned a value.\n",
"[test.c:2]: (style) Variable 'x' is not assigned a value.\n"
"[test.c:3]: (style) Variable 'c' is not assigned a value.\n", errout.str());
ASSERT_EQUALS("[test.c:4]: (style) Variable 'c' is assigned a value that is never used.\n", errout.str());
functionVariableUsage("void f(int c) {\n"
" int x;\n"
@ -3132,6 +3130,29 @@ private:
" (b).x += a;\n"
"}");
ASSERT_EQUALS("", errout.str());
functionVariableUsage("void foo() {\n"
" int a=1, b[10];\n"
" b[0] = x;\n"
" a += b[0];\n"
" return a;\n"
"}");
ASSERT_EQUALS("", errout.str());
functionVariableUsage("void f(int *start, int *stop) {\n"
" int length = *start - *stop;\n"
" if (length < 10000)\n"
" length = 10000;\n"
" *stop -= length;\n"
"}");
ASSERT_EQUALS("", errout.str());
functionVariableUsage("void f(int a) {\n"
" int x = 3;\n"
" a &= ~x;\n"
" return a;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void localvarFor() {

View File

@ -83,6 +83,8 @@ class MatchCompiler:
def _compileCmd(self, tok):
if tok == '%any%':
return 'true'
elif tok == '%assign%':
return 'tok->isAssignmentOp()'
elif tok == '%bool%':
return 'tok->isBoolean()'
elif tok == '%char%':