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

Fixed #3296 (false positive (inconclusive): 'C::operator=' should return 'C &')

This commit is contained in:
PKEuS 2011-12-14 15:37:43 +01:00 committed by Daniel Marjamäki
parent ba463295c2
commit 8ed8206b44
2 changed files with 24 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
lib/checkclass.cpp
View File

@ -26,7 +26,6 @@
#include <cstring> #include <cstring>
#include <string> #include <string>
#include <sstream>
#include <algorithm> #include <algorithm>
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
@ -742,10 +741,6 @@ void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const
void CheckClass::operatorEq() void CheckClass::operatorEq()
{ {
// See #3296
if (!_settings->inconclusive)
return;
if (!_settings->isEnabled("style")) if (!_settings->isEnabled("style"))
return; return;
@ -762,7 +757,7 @@ void CheckClass::operatorEq()
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (func->type == Function::eOperatorEqual && func->access != Private) { if (func->type == Function::eOperatorEqual && func->access != Private) {
// use definition for check so we don't have to deal with qualification // use definition for check so we don't have to deal with qualification
if (!(Token::Match(func->tokenDef->tokAt(-3), ";|}|{|public:|protected:|private: %type% &") && if (!(Token::Match(func->tokenDef->tokAt(-3), ";|}|{|public:|protected:|private:|virtual %type% &") &&
func->tokenDef->strAt(-2) == scope->className)) { func->tokenDef->strAt(-2) == scope->className)) {
// make sure we really have a copy assignment operator // make sure we really have a copy assignment operator
if (Token::Match(func->tokenDef->tokAt(2), "const| %var% &")) { if (Token::Match(func->tokenDef->tokAt(2), "const| %var% &")) {
@ -780,7 +775,7 @@ void CheckClass::operatorEq()
void CheckClass::operatorEqReturnError(const Token *tok, const std::string &className) void CheckClass::operatorEqReturnError(const Token *tok, const std::string &className)
{ {
reportInconclusiveError(tok, Severity::style, "operatorEq", "Inconclusive: \'" + className + "::operator=' should return \'" + className + " &\'"); reportInconclusiveError(tok, Severity::style, "operatorEq", "\'" + className + "::operator=' should return \'" + className + " &\'");
} }
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
@ -805,12 +800,10 @@ void CheckClass::operatorEqRetRefThis()
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (func->type == Function::eOperatorEqual && func->hasBody) { if (func->type == Function::eOperatorEqual && func->hasBody) {
// make sure return signature is correct // make sure return signature is correct
if (Token::Match(func->tokenDef->tokAt(-3), ";|}|{|public:|protected:|private: %type% &") && if (Token::Match(func->tokenDef->tokAt(-3), ";|}|{|public:|protected:|private:|virtual %type% &") &&
func->tokenDef->strAt(-2) == scope->className) { func->tokenDef->strAt(-2) == scope->className) {
// find the ')'
const Token *tok = func->token->next()->link();
checkReturnPtrThis(&(*scope), &(*func), tok->tokAt(2), tok->next()->link()); checkReturnPtrThis(&(*scope), &(*func), func->start->next(), func->start->link());
} }
} }
} }
@ -844,7 +837,7 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co
if (it->tokenDef->previous()->str() == "&" && if (it->tokenDef->previous()->str() == "&" &&
it->tokenDef->strAt(-2) == scope->className) { it->tokenDef->strAt(-2) == scope->className) {
// make sure it's not a const function // make sure it's not a const function
if (it->arg->link()->next()->str() != "const") { if (!it->isConst) {
/** @todo make sure argument types match */ /** @todo make sure argument types match */
// make sure it's not the same function // make sure it's not the same function
if (&*it != func) if (&*it != func)
@ -860,8 +853,7 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co
} }
// check if *this is returned // check if *this is returned
else if (!(Token::Match(tok->next(), "(| * this ;|=") || else if (!(Token::Match(tok->next(), "(| * this ;|=|+=") ||
Token::Match(tok->next(), "(| * this +=") ||
Token::simpleMatch(tok->next(), "operator= (") || Token::simpleMatch(tok->next(), "operator= (") ||
Token::simpleMatch(tok->next(), "this . operator= (") || Token::simpleMatch(tok->next(), "this . operator= (") ||
(Token::Match(tok->next(), "%type% :: operator= (") && (Token::Match(tok->next(), "%type% :: operator= (") &&

27
test/testclass.cpp
View File

@ -110,6 +110,7 @@ private:
TEST_CASE(operatorEq2); TEST_CASE(operatorEq2);
TEST_CASE(operatorEq3); // ticket #3051 TEST_CASE(operatorEq3); // ticket #3051
TEST_CASE(operatorEq4); // ticket #3114 TEST_CASE(operatorEq4); // ticket #3114
TEST_CASE(operatorEq5); // ticket #3296
TEST_CASE(operatorEqRetRefThis1); TEST_CASE(operatorEqRetRefThis1);
TEST_CASE(operatorEqRetRefThis2); // ticket #1323 TEST_CASE(operatorEqRetRefThis2); // ticket #1323
TEST_CASE(operatorEqRetRefThis3); // ticket #1405 TEST_CASE(operatorEqRetRefThis3); // ticket #1405
@ -238,7 +239,7 @@ private:
" void goo() {}" " void goo() {}"
" void operator=(const A&);\n" " void operator=(const A&);\n"
"};\n"); "};\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Inconclusive: 'A::operator=' should return 'A &'\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'\n", errout.str());
checkOpertorEq("class A\n" checkOpertorEq("class A\n"
"{\n" "{\n"
@ -272,14 +273,14 @@ private:
"public:\n" "public:\n"
" void operator=(const B&);\n" " void operator=(const B&);\n"
"};\n"); "};\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Inconclusive: 'A::operator=' should return 'A &'\n" ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'\n"
"[test.cpp:9]: (style) Inconclusive: 'B::operator=' should return 'B &'\n", errout.str()); "[test.cpp:9]: (style) 'B::operator=' should return 'B &'\n", errout.str());
checkOpertorEq("struct A\n" checkOpertorEq("struct A\n"
"{\n" "{\n"
" void operator=(const A&);\n" " void operator=(const A&);\n"
"};\n"); "};\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Inconclusive: 'A::operator=' should return 'A &'\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (style) 'A::operator=' should return 'A &'\n", errout.str());
} }
void operatorEq2() { void operatorEq2() {
@ -288,28 +289,28 @@ private:
"public:\n" "public:\n"
" void * operator=(const A&);\n" " void * operator=(const A&);\n"
"};\n"); "};\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Inconclusive: 'A::operator=' should return 'A &'\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'\n", errout.str());
checkOpertorEq("class A\n" checkOpertorEq("class A\n"
"{\n" "{\n"
"public:\n" "public:\n"
" A * operator=(const A&);\n" " A * operator=(const A&);\n"
"};\n"); "};\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Inconclusive: 'A::operator=' should return 'A &'\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'\n", errout.str());
checkOpertorEq("class A\n" checkOpertorEq("class A\n"
"{\n" "{\n"
"public:\n" "public:\n"
" const A & operator=(const A&);\n" " const A & operator=(const A&);\n"
"};\n"); "};\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Inconclusive: 'A::operator=' should return 'A &'\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'\n", errout.str());
checkOpertorEq("class A\n" checkOpertorEq("class A\n"
"{\n" "{\n"
"public:\n" "public:\n"
" B & operator=(const A&);\n" " B & operator=(const A&);\n"
"};\n"); "};\n");
ASSERT_EQUALS("[test.cpp:4]: (style) Inconclusive: 'A::operator=' should return 'A &'\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'\n", errout.str());
} }
void operatorEq3() { // ticket #3051 void operatorEq3() { // ticket #3051
@ -329,6 +330,14 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void operatorEq5() { // ticket #3296 (virtual operator)
checkOpertorEq(
"class A {\n"
" virtual A& operator=(const A &a) {return *this};\n"
"};");
ASSERT_EQUALS("", errout.str());
}
// Check that operator Equal returns reference to this // Check that operator Equal returns reference to this
void checkOpertorEqRetRefThis(const char code[]) { void checkOpertorEqRetRefThis(const char code[]) {
// Clear the error log // Clear the error log
@ -6346,7 +6355,7 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void constFriend() { // ticket #1921 void constFriend() { // ticket #1921
const char code[] = "class foo {\n" const char code[] = "class foo {\n"
" friend void f() { }\n" " friend void f() { }\n"
"};"; "};";