From b10110b5aca56584bf232f6d4139b936dbfcd706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 13 Nov 2015 12:48:26 +0100 Subject: [PATCH] CheckClass: Removed noExplicitCopyMoveConstructorError after discussion in http://sourceforge.net/p/cppcheck/discussion/general/thread/b2ce9d3d/. --- lib/checkclass.cpp | 23 +++++++---------------- lib/checkclass.h | 4 +--- test/testclass.cpp | 6 +++--- 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 5fb703360..89d6a4b3f 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -241,8 +241,8 @@ void CheckClass::checkExplicitConstructors() } } - // Abstract classes can't be instantiated. But if there is "misuse" by derived - // classes then these constructors must be explicit. + // Abstract classes can't be instantiated. But if there is C++11 + // "misuse" by derived classes then these constructors must be explicit. if (isAbstractClass && _settings->standards.cpp != Standards::CPP11) continue; @@ -256,13 +256,11 @@ void CheckClass::checkExplicitConstructors() if (!func->isConstructor() || func->isDelete() || (!func->hasBody() && func->access == Private)) continue; - if (!func->isExplicit() && func->argCount() == 1) { - // We must decide, if it is not a copy/move constructor, or it is a copy/move constructor of abstract class. - if (func->type != Function::eCopyConstructor && func->type != Function::eMoveConstructor) { - noExplicitConstructorError(func->tokenDef, scope->className, scope->type == Scope::eStruct); - } else if (isAbstractClass) { - noExplicitCopyMoveConstructorError(func->tokenDef, scope->className, scope->type == Scope::eStruct); - } + if (!func->isExplicit() && + func->argCount() == 1 && + func->type != Function::eCopyConstructor && + func->type != Function::eMoveConstructor) { + noExplicitConstructorError(func->tokenDef, scope->className, scope->type == Scope::eStruct); } } } @@ -788,13 +786,6 @@ void CheckClass::noExplicitConstructorError(const Token *tok, const std::string reportError(tok, Severity::style, "noExplicitConstructor", message + "\n" + verbose); } -void CheckClass::noExplicitCopyMoveConstructorError(const Token *tok, const std::string &classname, bool isStruct) -{ - const std::string message(std::string(isStruct ? "Abstract struct" : "Abstract class") + " '" + classname + "' has a copy/move constructor that is not explicit."); - const std::string verbose(message + " For abstract classes, even copy/move constructors may be declared explicit, as, by definition, abstract classes cannot be instantiated, and so objects of such type should never be passed by value."); - reportError(tok, Severity::style, "noExplicitCopyMoveConstructor", message + "\n" + verbose); -} - void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) { reportError(tok, Severity::warning, "uninitMemberVar", "Member variable '" + classname + "::" + varname + "' is not initialized in the constructor.", 0U, inconclusive); diff --git a/lib/checkclass.h b/lib/checkclass.h index cdeed2472..ffc533aa8 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -142,7 +142,6 @@ private: // Reporting errors.. void noConstructorError(const Token *tok, const std::string &classname, bool isStruct); void noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct); - void noExplicitCopyMoveConstructorError(const Token *tok, const std::string &classname, bool isStruct); //void copyConstructorMallocError(const Token *cctor, const Token *alloc, const std::string& var_name); void copyConstructorShallowCopyError(const Token *tok, const std::string& varname); void noCopyConstructorError(const Token *tok, const std::string &classname, bool isStruct); @@ -173,7 +172,6 @@ private: CheckClass c(0, settings, errorLogger); c.noConstructorError(0, "classname", false); c.noExplicitConstructorError(0, "classname", false); - c.noExplicitCopyMoveConstructorError(0, "classname", false); //c.copyConstructorMallocError(0, 0, "var"); c.copyConstructorShallowCopyError(0, "var"); c.noCopyConstructorError(0, "class", false); @@ -208,7 +206,7 @@ private: return "Check the code for each class.\n" "- Missing constructors and copy constructors\n" //"- Missing allocation of memory in copy constructor\n" - "- Constructors which should be explicit are explicit\n" + "- Constructors which should be explicit\n" "- Are all variables initialized by the constructors?\n" "- Are all variables assigned by 'operator='?\n" "- Warn if memset, memcpy etc are used on a class\n" diff --git a/test/testclass.cpp b/test/testclass.cpp index 5a31f98a0..6e762c57d 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -237,13 +237,13 @@ private: " Class(const Class& other) { }\n" " virtual int i() = 0;\n" "};"); - ASSERT_EQUALS("[test.cpp:2]: (style) Abstract class 'Class' has a copy/move constructor that is not explicit.\n", errout.str()); + ASSERT_EQUALS("", errout.str()); checkExplicitConstructors("class Class {\n" " Class(Class&& other) { }\n" " virtual int i() = 0;\n" "};"); - ASSERT_EQUALS("[test.cpp:2]: (style) Abstract class 'Class' has a copy/move constructor that is not explicit.\n", errout.str()); + ASSERT_EQUALS("", errout.str()); // #6585 checkExplicitConstructors("class Class {\n" @@ -256,7 +256,7 @@ private: " public: Class(const Class&);\n" " virtual int i() = 0;\n" "};"); - ASSERT_EQUALS("[test.cpp:2]: (style) Abstract class 'Class' has a copy/move constructor that is not explicit.\n", errout.str()); + ASSERT_EQUALS("", errout.str()); } void checkDuplInheritedMembers(const char code[]) {