Permalink
Browse files

Refactorized postfix operator check:

- Support class members
- Support references (removed wrong bailout)
- Removed wrong unit tests and wrong messages for std::cout << k-- << std::endl;
  • Loading branch information...
1 parent ac0df71 commit 12f5ccfb4e2817bb73e0b18e4f298d8bfd6dd880 @PKEuS PKEuS committed Mar 31, 2013
Showing with 72 additions and 57 deletions.
  1. +30 −23 lib/checkpostfixoperator.cpp
  2. +42 −34 test/testpostfixoperator.cpp
@@ -43,37 +43,44 @@ void CheckPostfixOperator::postfixOperator()
for (std::size_t i = 0; i < functions; ++i) {
const Scope * scope = symbolDatabase->functionScopes[i];
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
- if (tok->type() == Token::eIncDecOp) {
- bool result = false;
- if (Token::Match(tok->tokAt(-2), ";|{|}") && Token::Match(tok->next(), ";|)|,")) {
+ bool result = false;
+ const Variable *var = tok->variable();
+ if (var && Token::Match(tok, "%var% ++|--")) {
+ if (Token::Match(tok->previous(), ";|{|}") && Token::Match(tok->tokAt(2), ";|,|)")) {
result = true;
- } else if (tok->strAt(-2) == ",") {
- int ii(1);
- while (tok->strAt(ii) != ")" && tok->tokAt(ii) != 0) {
- if (tok->strAt(ii) == ";") {
+ } else if (tok->strAt(-1) == ",") {
+ for (const Token* tok2 = tok->tokAt(2); tok2 != 0 && tok2->str() != ")"; tok2 = tok2->next()) {
+ if (tok2->str() == ";") {
result = true;
break;
- }
- ++ii;
+ } else if (tok2->str() == "(")
+ tok2 = tok2->link();
+ }
+ } else if (tok->strAt(-1) == ".") {
+ for (const Token* tok2 = tok->tokAt(-2); tok2 != 0; tok2 = tok2->previous()) {
+ if (Token::Match(tok2, ";|{|}")) {
+ result = true;
+ break;
+ } else if (Token::Match(tok2, ")|]|>") && tok2->link())
+ tok2 = tok2->link();
+ else if (tok2->isAssignmentOp() || Token::Match(tok2, "(|["))
+ break;
}
- } else if (tok->strAt(-2) == "<<" && tok->strAt(1) == "<<") {
- result = true;
}
+ }
- if (result && tok->previous()->varId()) {
- const Variable *var = tok->previous()->variable();
- if (!var || var->isPointer() || var->isArray() || var->isReference())
- continue;
+ if (result) {
+ if (var->isPointer() || var->isArray())
+ continue;
- const Token *decltok = var->nameToken();
+ const Token *decltok = var->nameToken();
- if (Token::Match(decltok->previous(), "iterator|const_iterator|reverse_iterator|const_reverse_iterator")) {
- // the variable is an iterator
- postfixOperatorError(tok);
- } else if (var->type()) {
- // the variable is an instance of class
- postfixOperatorError(tok);
- }
+ if (Token::Match(decltok->previous(), "iterator|const_iterator|reverse_iterator|const_reverse_iterator")) {
+ // the variable is an iterator
+ postfixOperatorError(tok);
+ } else if (var->type()) {
+ // the variable is an instance of class
+ postfixOperatorError(tok);
}
}
}
@@ -56,13 +56,14 @@ class TestPostfixOperator : public TestFixture {
void run() {
TEST_CASE(testsimple);
TEST_CASE(testfor);
- TEST_CASE(teststream);
TEST_CASE(testvolatile);
TEST_CASE(testiterator);
TEST_CASE(test2168);
TEST_CASE(pointer); // #2321 - postincrement of pointer is OK
TEST_CASE(testHangWithInvalidCode); // #2847 - cppcheck hangs with 100% cpu load
TEST_CASE(testtemplate); // #4686
+ TEST_CASE(testmember);
+ TEST_CASE(testcomma);
}
void testHangWithInvalidCode() {
@@ -106,6 +107,13 @@ class TestPostfixOperator : public TestFixture {
"}");
ASSERT_EQUALS("[test.cpp:4]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
+ check("struct K {};\n"
+ "void foo(K& k)\n"
+ "{\n"
+ " k++;\n"
+ "}");
+ ASSERT_EQUALS("[test.cpp:4]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
+
check("union K {};"
"void foo()\n"
"{\n"
@@ -230,39 +238,6 @@ class TestPostfixOperator : public TestFixture {
}
- void teststream() {
- check("int main()\n"
- "{\n"
- " int k(0);\n"
- " std::cout << k << std::endl;\n"
- " std::cout << k++ << std::endl;\n"
- " std::cout << k-- << std::endl;\n"
- " return 0;\n"
- "}");
- ASSERT_EQUALS("", errout.str());
-
- check("class K {};\n"
- "int main()\n"
- "{\n"
- " K k(0);\n"
- " std::cout << k << std::endl;\n"
- " std::cout << k-- << std::endl;\n"
- " return 0;\n"
- "}");
- ASSERT_EQUALS("[test.cpp:6]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
-
- check("class K {};\n"
- "int main()\n"
- "{\n"
- " K k(0);\n"
- " std::cout << k << std::endl;\n"
- " std::cout << ++k << std::endl;\n"
- " std::cout << --k << std::endl;\n"
- " return 0;\n"
- "}");
- ASSERT_EQUALS("", errout.str());
- }
-
void testvolatile() {
check("class K {};\n"
"int main()\n"
@@ -354,6 +329,39 @@ class TestPostfixOperator : public TestFixture {
"}");
ASSERT_EQUALS("[test.cpp:3]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
}
+
+ void testmember() {
+ check("bool foo() {\n"
+ " class A {}; class B {A a;};\n"
+ " B b;\n"
+ " b.a++;\n"
+ "}");
+ ASSERT_EQUALS("[test.cpp:4]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
+
+ check("bool foo() {\n"
+ " class A {}; class B {A a;};\n"
+ " B b;\n"
+ " foo(b.a++);\n"
+ "}");
+ ASSERT_EQUALS("", errout.str());
+ }
+
+ void testcomma() {
+ check("bool foo(int i) {\n"
+ " class A {};\n"
+ " A a;\n"
+ " i++, a++;\n"
+ "}");
+ ASSERT_EQUALS("[test.cpp:4]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
+
+ check("bool foo(int i) {\n"
+ " class A {};\n"
+ " A a;\n"
+ " foo(i, a++);\n"
+ " foo(a++, i);\n"
+ "}");
+ ASSERT_EQUALS("", errout.str());
+ }
};
REGISTER_TEST(TestPostfixOperator)

0 comments on commit 12f5ccf

Please sign in to comment.