CheckClass: Better handling of defaulted and deleted functions in the noCopyConstructor/noOperatorEq/noDestructor

This commit is contained in:
Daniel Marjamäki 2018-05-04 14:58:38 +02:00
parent f2bb7397b3
commit 99003c2084
2 changed files with 70 additions and 11 deletions

View File

@ -327,25 +327,25 @@ void CheckClass::copyconstructors()
} }
if (!allocatedVars.empty()) { if (!allocatedVars.empty()) {
bool hasCopyCtor = false; const Function *funcCopyCtor = nullptr;
bool hasOperatorEq = false; const Function *funcOperatorEq = nullptr;
bool hasDestructor = false; const Function *funcDestructor = nullptr;
for (const Function &func : scope->functionList) { for (const Function &func : scope->functionList) {
if (func.type == Function::eCopyConstructor) if (func.type == Function::eCopyConstructor)
hasCopyCtor = true; funcCopyCtor = &func;
else if (func.type == Function::eOperatorEqual) else if (func.type == Function::eOperatorEqual)
hasOperatorEq = true; funcOperatorEq = &func;
else if (func.type == Function::eDestructor) else if (func.type == Function::eDestructor)
hasDestructor = true; funcDestructor = &func;
} }
if (!hasCopyCtor) { if (!funcCopyCtor || funcCopyCtor->isDefault()) {
bool unknown = false; bool unknown = false;
if (!isNonCopyable(scope, &unknown)) if (!isNonCopyable(scope, &unknown))
noCopyConstructorError(scope, allocatedVars.begin()->second, unknown); noCopyConstructorError(scope, allocatedVars.begin()->second, unknown);
} }
if (!hasOperatorEq) if (!funcOperatorEq || funcOperatorEq->isDefault())
noOperatorEqError(scope, allocatedVars.begin()->second); noOperatorEqError(scope, allocatedVars.begin()->second);
if (!hasDestructor) { if (!funcDestructor || funcDestructor->isDefault()) {
const Token * mustDealloc = nullptr; const Token * mustDealloc = nullptr;
for (std::map<unsigned int, const Token*>::const_iterator it = allocatedVars.begin(); it != allocatedVars.end(); ++it) { for (std::map<unsigned int, const Token*>::const_iterator it = allocatedVars.begin(); it != allocatedVars.end(); ++it) {
if (!Token::Match(it->second, "%var% [(=] new %type%")) { if (!Token::Match(it->second, "%var% [(=] new %type%")) {

View File

@ -68,6 +68,7 @@ private:
TEST_CASE(copyConstructor1); TEST_CASE(copyConstructor1);
TEST_CASE(copyConstructor2); // ticket #4458 TEST_CASE(copyConstructor2); // ticket #4458
TEST_CASE(copyConstructor3); // defaulted/deleted
TEST_CASE(noOperatorEq); // class with memory management should have operator eq TEST_CASE(noOperatorEq); // class with memory management should have operator eq
TEST_CASE(noDestructor); // class with memory management should have destructor TEST_CASE(noDestructor); // class with memory management should have destructor
@ -789,7 +790,6 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void copyConstructor2() { // ticket #4458 void copyConstructor2() { // ticket #4458
checkCopyConstructor("template <class _Tp>\n" checkCopyConstructor("template <class _Tp>\n"
"class Vector\n" "class Vector\n"
@ -807,6 +807,26 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void copyConstructor3() {
checkCopyConstructor("struct F {\n"
" char* c;\n"
" F() { c = malloc(100); }\n"
" F(const F &f) = delete;\n"
" F&operator=(const F &f);\n"
" ~F();\n"
"};");
ASSERT_EQUALS("", errout.str());
checkCopyConstructor("struct F {\n"
" char* c;\n"
" F() { c = malloc(100); }\n"
" F(const F &f) = default;\n"
" F&operator=(const F &f);\n"
" ~F();\n"
"};");
ASSERT_EQUALS("[test.cpp:3]: (style) Struct 'F' does not have a copy constructor but it has dynamic memory/resource allocation. It is recommended to delete or define the copy constructor.\n", errout.str());
}
void noOperatorEq() { void noOperatorEq() {
checkCopyConstructor("struct F {\n" checkCopyConstructor("struct F {\n"
" char* c;\n" " char* c;\n"
@ -815,6 +835,26 @@ private:
" ~F();\n" " ~F();\n"
"};"); "};");
ASSERT_EQUALS("[test.cpp:3]: (style) Struct 'F' does not have a assignment operator but it has dynamic memory/resource allocation. It is recommended to delete or define the assignment operator.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (style) Struct 'F' does not have a assignment operator but it has dynamic memory/resource allocation. It is recommended to delete or define the assignment operator.\n", errout.str());
// defaulted operator=
checkCopyConstructor("struct F {\n"
" char* c;\n"
" F() { c = malloc(100); }\n"
" F(const F &f);\n"
" F &operator=(const F &f) = default;\n"
" ~F();\n"
"};");
ASSERT_EQUALS("[test.cpp:3]: (style) Struct 'F' does not have a assignment operator but it has dynamic memory/resource allocation. It is recommended to delete or define the assignment operator.\n", errout.str());
// deleted operator=
checkCopyConstructor("struct F {\n"
" char* c;\n"
" F() { c = malloc(100); }\n"
" F(const F &f);\n"
" F &operator=(const F &f) = delete;\n"
" ~F();\n"
"};");
ASSERT_EQUALS("", errout.str());
} }
void noDestructor() { void noDestructor() {
@ -834,7 +874,6 @@ private:
"};"); "};");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
checkCopyConstructor("struct Data { int x; int y; };\n" checkCopyConstructor("struct Data { int x; int y; };\n"
"struct F {\n" "struct F {\n"
" Data* c;\n" " Data* c;\n"
@ -843,6 +882,26 @@ private:
" F&operator=(const F&);" " F&operator=(const F&);"
"};"); "};");
ASSERT_EQUALS("[test.cpp:4]: (style) Struct 'F' does not have a destructor which is recommended since the class has dynamic memory/resource allocation.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) Struct 'F' does not have a destructor which is recommended since the class has dynamic memory/resource allocation.\n", errout.str());
// defaulted destructor
checkCopyConstructor("struct F {\n"
" char* c;\n"
" F() { c = malloc(100); }\n"
" F(const F &f);\n"
" F &operator=(const F &f);\n"
" ~F() = default;\n"
"};");
ASSERT_EQUALS("[test.cpp:3]: (style) Struct 'F' does not have a destructor which is recommended since the class has dynamic memory/resource allocation.\n", errout.str());
// deleted destructor
checkCopyConstructor("struct F {\n"
" char* c;\n"
" F() { c = malloc(100); }\n"
" F(const F &f);\n"
" F &operator=(const F &f);\n"
" ~F() = delete;\n"
"};");
ASSERT_EQUALS("", errout.str());
} }
// Check the operator Equal // Check the operator Equal