Skip to content

Commit 12f5ccf

Browse files
PKEuSPKEuS
committed
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;
1 parent ac0df71 commit 12f5ccf

2 files changed

Lines changed: 72 additions & 57 deletions

File tree

‎lib/checkpostfixoperator.cpp‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -43,37 +43,44 @@ void CheckPostfixOperator::postfixOperator()
4343
for (std::size_t i = 0; i < functions; ++i) {
4444
const Scope * scope = symbolDatabase->functionScopes[i];
4545
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
46-
if (tok->type() == Token::eIncDecOp) {
47-
bool result = false;
48-
if (Token::Match(tok->tokAt(-2), ";|{|}") && Token::Match(tok->next(), ";|)|,")) {
46+
bool result = false;
47+
const Variable *var = tok->variable();
48+
if (var && Token::Match(tok, "%var% ++|--")) {
49+
if (Token::Match(tok->previous(), ";|{|}") && Token::Match(tok->tokAt(2), ";|,|)")) {
4950
result = true;
50-
} else if (tok->strAt(-2) == ",") {
51-
int ii(1);
52-
while (tok->strAt(ii) != ")" && tok->tokAt(ii) != 0) {
53-
if (tok->strAt(ii) == ";") {
51+
} else if (tok->strAt(-1) == ",") {
52+
for (const Token* tok2 = tok->tokAt(2); tok2 != 0 && tok2->str() != ")"; tok2 = tok2->next()) {
53+
if (tok2->str() == ";") {
5454
result = true;
5555
break;
56-
}
57-
++ii;
56+
} else if (tok2->str() == "(")
57+
tok2 = tok2->link();
58+
}
59+
} else if (tok->strAt(-1) == ".") {
60+
for (const Token* tok2 = tok->tokAt(-2); tok2 != 0; tok2 = tok2->previous()) {
61+
if (Token::Match(tok2, ";|{|}")) {
62+
result = true;
63+
break;
64+
} else if (Token::Match(tok2, ")|]|>") && tok2->link())
65+
tok2 = tok2->link();
66+
else if (tok2->isAssignmentOp() || Token::Match(tok2, "(|["))
67+
break;
5868
}
59-
} else if (tok->strAt(-2) == "<<" && tok->strAt(1) == "<<") {
60-
result = true;
6169
}
70+
}
6271

63-
if (result && tok->previous()->varId()) {
64-
const Variable *var = tok->previous()->variable();
65-
if (!var || var->isPointer() || var->isArray() || var->isReference())
66-
continue;
72+
if (result) {
73+
if (var->isPointer() || var->isArray())
74+
continue;
6775

68-
const Token *decltok = var->nameToken();
76+
const Token *decltok = var->nameToken();
6977

70-
if (Token::Match(decltok->previous(), "iterator|const_iterator|reverse_iterator|const_reverse_iterator")) {
71-
// the variable is an iterator
72-
postfixOperatorError(tok);
73-
} else if (var->type()) {
74-
// the variable is an instance of class
75-
postfixOperatorError(tok);
76-
}
78+
if (Token::Match(decltok->previous(), "iterator|const_iterator|reverse_iterator|const_reverse_iterator")) {
79+
// the variable is an iterator
80+
postfixOperatorError(tok);
81+
} else if (var->type()) {
82+
// the variable is an instance of class
83+
postfixOperatorError(tok);
7784
}
7885
}
7986
}

‎test/testpostfixoperator.cpp‎

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,14 @@ class TestPostfixOperator : public TestFixture {
5656
void run() {
5757
TEST_CASE(testsimple);
5858
TEST_CASE(testfor);
59-
TEST_CASE(teststream);
6059
TEST_CASE(testvolatile);
6160
TEST_CASE(testiterator);
6261
TEST_CASE(test2168);
6362
TEST_CASE(pointer); // #2321 - postincrement of pointer is OK
6463
TEST_CASE(testHangWithInvalidCode); // #2847 - cppcheck hangs with 100% cpu load
6564
TEST_CASE(testtemplate); // #4686
65+
TEST_CASE(testmember);
66+
TEST_CASE(testcomma);
6667
}
6768

6869
void testHangWithInvalidCode() {
@@ -106,6 +107,13 @@ class TestPostfixOperator : public TestFixture {
106107
"}");
107108
ASSERT_EQUALS("[test.cpp:4]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
108109

110+
check("struct K {};\n"
111+
"void foo(K& k)\n"
112+
"{\n"
113+
" k++;\n"
114+
"}");
115+
ASSERT_EQUALS("[test.cpp:4]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
116+
109117
check("union K {};"
110118
"void foo()\n"
111119
"{\n"
@@ -230,39 +238,6 @@ class TestPostfixOperator : public TestFixture {
230238

231239
}
232240

233-
void teststream() {
234-
check("int main()\n"
235-
"{\n"
236-
" int k(0);\n"
237-
" std::cout << k << std::endl;\n"
238-
" std::cout << k++ << std::endl;\n"
239-
" std::cout << k-- << std::endl;\n"
240-
" return 0;\n"
241-
"}");
242-
ASSERT_EQUALS("", errout.str());
243-
244-
check("class K {};\n"
245-
"int main()\n"
246-
"{\n"
247-
" K k(0);\n"
248-
" std::cout << k << std::endl;\n"
249-
" std::cout << k-- << std::endl;\n"
250-
" return 0;\n"
251-
"}");
252-
ASSERT_EQUALS("[test.cpp:6]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
253-
254-
check("class K {};\n"
255-
"int main()\n"
256-
"{\n"
257-
" K k(0);\n"
258-
" std::cout << k << std::endl;\n"
259-
" std::cout << ++k << std::endl;\n"
260-
" std::cout << --k << std::endl;\n"
261-
" return 0;\n"
262-
"}");
263-
ASSERT_EQUALS("", errout.str());
264-
}
265-
266241
void testvolatile() {
267242
check("class K {};\n"
268243
"int main()\n"
@@ -354,6 +329,39 @@ class TestPostfixOperator : public TestFixture {
354329
"}");
355330
ASSERT_EQUALS("[test.cpp:3]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
356331
}
332+
333+
void testmember() {
334+
check("bool foo() {\n"
335+
" class A {}; class B {A a;};\n"
336+
" B b;\n"
337+
" b.a++;\n"
338+
"}");
339+
ASSERT_EQUALS("[test.cpp:4]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
340+
341+
check("bool foo() {\n"
342+
" class A {}; class B {A a;};\n"
343+
" B b;\n"
344+
" foo(b.a++);\n"
345+
"}");
346+
ASSERT_EQUALS("", errout.str());
347+
}
348+
349+
void testcomma() {
350+
check("bool foo(int i) {\n"
351+
" class A {};\n"
352+
" A a;\n"
353+
" i++, a++;\n"
354+
"}");
355+
ASSERT_EQUALS("[test.cpp:4]: (performance) Prefer prefix ++/-- operators for non-primitive types.\n", errout.str());
356+
357+
check("bool foo(int i) {\n"
358+
" class A {};\n"
359+
" A a;\n"
360+
" foo(i, a++);\n"
361+
" foo(a++, i);\n"
362+
"}");
363+
ASSERT_EQUALS("", errout.str());
364+
}
357365
};
358366

359367
REGISTER_TEST(TestPostfixOperator)

0 commit comments

Comments
 (0)