From 3f8218af1bb4fec3b0367ca7a1f1a902b42e93ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 17 Jun 2020 20:35:30 +0200 Subject: [PATCH] Removed CheckClass::operatorEq: does not 'belong' --- lib/checkclass.cpp | 52 -------------- lib/checkclass.h | 7 -- test/testclass.cpp | 171 --------------------------------------------- 3 files changed, 230 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 8f1bc4598..049c020bf 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -1328,58 +1328,6 @@ void CheckClass::memsetErrorFloat(const Token *tok, const std::string &type) } -//--------------------------------------------------------------------------- -// ClassCheck: "void operator=(" and "const type & operator=(" -//--------------------------------------------------------------------------- - -void CheckClass::operatorEq() -{ - if (!mSettings->isEnabled(Settings::STYLE)) - return; - - for (const Scope * scope : mSymbolDatabase->classAndStructScopes) { - for (std::list::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { - if (func->type == Function::eOperatorEqual && func->access == AccessControl::Public) { - // skip "deleted" functions - cannot be called anyway - if (func->isDelete()) - continue; - // use definition for check so we don't have to deal with qualification - bool returnSelfRef = false; - if (func->retDef->str() == scope->className) { - if (Token::Match(func->retDef, "%type% &")) { - returnSelfRef = true; - } else { - // We might have "Self&"" - const Token * const tok = func->retDef->next(); - if (tok && tok->str() == "<" && tok->link() && tok->link()->next() && tok->link()->next()->str() == "&") - returnSelfRef = true; - } - } - if (!returnSelfRef) { - // make sure we really have a copy assignment operator - const Token *paramTok = func->tokenDef->tokAt(2); - if (Token::Match(paramTok, "const| %name% &")) { - if (paramTok->str() == "const" && - paramTok->strAt(1) == scope->className) - operatorEqReturnError(func->retDef, scope->className); - else if (paramTok->str() == scope->className) - operatorEqReturnError(func->retDef, scope->className); - } - } - } - } - } -} - -void CheckClass::operatorEqReturnError(const Token *tok, const std::string &className) -{ - reportError(tok, Severity::style, "operatorEq", - "$symbol:" + className +"\n" - "'$symbol::operator=' should return '$symbol &'.\n" - "The $symbol::operator= does not conform to standard C/C++ behaviour. To conform to standard C/C++ behaviour, return a reference to self (such as: '$symbol &$symbol::operator=(..) { .. return *this; }'. For safety reasons it might be better to not fix this message. If you think that safety is always more important than conformance then please ignore/suppress this message. For more details about this topic, see the book \"Effective C++\" by Scott Meyers." - , CWE398, false); -} - //--------------------------------------------------------------------------- // ClassCheck: "C& operator=(const C&) { ... return *this; }" // operator= should return a reference to *this diff --git a/lib/checkclass.h b/lib/checkclass.h index 20325223d..568ba5dac 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -62,7 +62,6 @@ public: // can't be a simplified check .. the 'sizeof' is used. checkClass.checkMemset(); checkClass.constructors(); - checkClass.operatorEq(); checkClass.privateFunctions(); checkClass.operatorEqRetRefThis(); checkClass.thisSubtraction(); @@ -104,9 +103,6 @@ public: void checkMemset(); void checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation, std::set parsedTypes); - /** @brief 'operator=' should return something and it should not be const. */ - void operatorEq(); - /** @brief 'operator=' should return reference to *this */ void operatorEqRetRefThis(); // Warning upon no "return *this;" @@ -170,7 +166,6 @@ private: void memsetErrorFloat(const Token *tok, const std::string &type); void mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname); void mallocOnClassWarning(const Token* tok, const std::string &memfunc, const Token* classTok); - void operatorEqReturnError(const Token *tok, const std::string &className); void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived, bool inconclusive); void thisSubtractionError(const Token *tok); void operatorEqRetRefThisError(const Token *tok); @@ -208,7 +203,6 @@ private: c.memsetErrorFloat(nullptr, "class"); c.mallocOnClassWarning(nullptr, "malloc", nullptr); c.mallocOnClassError(nullptr, "malloc", nullptr, "std::string"); - c.operatorEqReturnError(nullptr, "class"); c.virtualDestructorError(nullptr, "Base", "Derived", false); c.thisSubtractionError(nullptr); c.operatorEqRetRefThisError(nullptr); @@ -244,7 +238,6 @@ private: "- Warn if memory for classes is allocated with malloc()\n" "- If it's a base class, check that the destructor is virtual\n" "- Are there unused private functions?\n" - "- 'operator=' should return reference to self\n" "- 'operator=' should check for assignment to self\n" "- Constness for member functions\n" "- Order of initializations\n" diff --git a/test/testclass.cpp b/test/testclass.cpp index 844c59b92..82d559000 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -74,11 +74,6 @@ private: TEST_CASE(noOperatorEq); // class with memory management should have operator eq TEST_CASE(noDestructor); // class with memory management should have destructor - TEST_CASE(operatorEq1); - TEST_CASE(operatorEq2); - TEST_CASE(operatorEq3); // ticket #3051 - TEST_CASE(operatorEq4); // ticket #3114 - TEST_CASE(operatorEq5); // ticket #3296 TEST_CASE(operatorEqRetRefThis1); TEST_CASE(operatorEqRetRefThis2); // ticket #1323 TEST_CASE(operatorEqRetRefThis3); // ticket #1405 @@ -966,172 +961,6 @@ private: ASSERT_EQUALS("", errout.str()); } - // Check the operator Equal - void checkOpertorEq(const char code[]) { - // Clear the error log - errout.str(""); - - settings0.inconclusive = true; - - // Tokenize.. - Tokenizer tokenizer(&settings0, this); - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.cpp"); - - // Check.. - CheckClass checkClass(&tokenizer, &settings0, this); - checkClass.operatorEq(); - } - - void operatorEq1() { - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " void goo() {}" - " void operator=(const A&);\n" - "};"); - ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " void goo() {}" - " void operator=(const A&)=delete;\n" - "};"); - ASSERT_EQUALS("", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " void goo() {}" - " void operator=(A&);\n" - "};"); - ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "private:\n" - " void operator=(const A&);\n" - "};"); - ASSERT_EQUALS("", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "protected:\n" - " void operator=(const A&);\n" - "};"); - ASSERT_EQUALS("", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "private:\n" - " void operator=(const A&)=delete;\n" - "};"); - ASSERT_EQUALS("", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - " void operator=(const A&);\n" - "};"); - ASSERT_EQUALS("", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " void goo() {}\n" - "private:\n" - " void operator=(const A&);\n" - "};"); - ASSERT_EQUALS("", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " void operator=(const A&);\n" - "};\n" - "class B\n" - "{\n" - "public:\n" - " void operator=(const B&);\n" - "};"); - ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n" - "[test.cpp:9]: (style) 'B::operator=' should return 'B &'.\n", errout.str()); - - checkOpertorEq("struct A\n" - "{\n" - " void operator=(const A&);\n" - "};"); - ASSERT_EQUALS("[test.cpp:3]: (style) 'A::operator=' should return 'A &'.\n", errout.str()); - - checkOpertorEq("struct A\n" - "{\n" - " void operator=(const A&)=delete;\n" - "};"); - ASSERT_EQUALS("", errout.str()); - - // Ticket #7017 - checkOpertorEq("template struct X {\n" - " inline X(const X& Rhs);\n" - " inline X& operator =(const X& Rhs);\n" - "};"); - ASSERT_EQUALS("", errout.str()); - } - - void operatorEq2() { - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " void * operator=(const A&);\n" - "};"); - ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " A * operator=(const A&);\n" - "};"); - ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " const A & operator=(const A&);\n" - "};"); - ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str()); - - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " B & operator=(const A&);\n" - "};"); - ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str()); - } - - void operatorEq3() { // ticket #3051 - checkOpertorEq("class A\n" - "{\n" - "public:\n" - " A * operator=(const A*);\n" - "};"); - ASSERT_EQUALS("", errout.str()); - } - - void operatorEq4() { // ticket #3114 (infinite loop) - checkOpertorEq("struct A {\n" - " A& operator=(A const& a) { return operator=(&a); }\n" - " A& operator=(const A*) { return *this; }\n" - "};"); - 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 void checkOpertorEqRetRefThis(const char code[]) { // Clear the error log