From c79bfdce2ce142dc55e5fa31fbb7f2d7c314cda5 Mon Sep 17 00:00:00 2001 From: Dmitry-Me Date: Sat, 24 Jan 2015 11:18:33 +0100 Subject: [PATCH] CheckClass: Better checking of what operator= returns --- lib/checkclass.cpp | 38 ++++++++++++++++++++++++++++++++++++-- lib/checkclass.h | 4 ++++ test/testclass.cpp | 40 +++++++++++++++++++++++++++++++++++++--- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 9784925d0..5fc7834a2 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -1180,6 +1180,8 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co { bool foundReturn = false; + const Token* const startTok = tok; + for (; tok && tok != last; tok = tok->next()) { // check for return of reference to this if (tok->str() == "return") { @@ -1228,8 +1230,26 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co operatorEqRetRefThisError(func->token); } } - if (!foundReturn) - operatorEqRetRefThisError(func->token); + if (foundReturn) { + return; + } + if (startTok->next() == last) { + if (Token::Match(func->argDef, std::string("( const " + scope->className + " &").c_str())) { + // Typical wrong way to suppress default assignment operator by declaring it and leaving empty + operatorEqMissingReturnStatementError(func->token, func->access == Public); + } else { + operatorEqMissingReturnStatementError(func->token, true); + } + return; + } + if (_settings->library.isScopeNoReturn(last, 0)) { + // Typical wrong way to prohibit default assignment operator + // by always throwing an exception or calling a noreturn function + operatorEqShouldBeLeftUnimplementedError(func->token); + return; + } + + operatorEqMissingReturnStatementError(func->token, func->access == Public); } void CheckClass::operatorEqRetRefThisError(const Token *tok) @@ -1237,6 +1257,20 @@ void CheckClass::operatorEqRetRefThisError(const Token *tok) reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to 'this' instance."); } +void CheckClass::operatorEqShouldBeLeftUnimplementedError(const Token *tok) +{ + reportError(tok, Severity::style, "operatorEqShouldBeLeftUnimplemented", "'operator=' should either return reference to 'this' instance or be declared private and left unimplemented."); +} + +void CheckClass::operatorEqMissingReturnStatementError(const Token *tok, bool error) +{ + if (error) { + reportError(tok, Severity::error, "operatorEqMissingReturnStatement", "No 'return' statement in non-void function causes undefined behavior."); + } else { + operatorEqRetRefThisError(tok); + } +} + //--------------------------------------------------------------------------- // ClassCheck: "C& operator=(const C& rhs) { if (this == &rhs) ... }" // operator= should check for assignment to self diff --git a/lib/checkclass.h b/lib/checkclass.h index aff66274b..7b0ad9d67 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -151,6 +151,8 @@ private: 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); + void operatorEqShouldBeLeftUnimplementedError(const Token *tok); + void operatorEqMissingReturnStatementError(const Token *tok, bool error); void operatorEqToSelfError(const Token *tok); void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic); void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic); @@ -178,6 +180,8 @@ private: c.virtualDestructorError(0, "Base", "Derived", false); c.thisSubtractionError(0); c.operatorEqRetRefThisError(0); + c.operatorEqMissingReturnStatementError(0, true); + c.operatorEqShouldBeLeftUnimplementedError(0); c.operatorEqToSelfError(0); c.checkConstError(0, "class", "function", false); c.checkConstError(0, "class", "function", true); diff --git a/test/testclass.cpp b/test/testclass.cpp index 488e285b8..3b40ba7a6 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -812,7 +812,7 @@ private: "{\n" " szp &operator =(int *other) {};\n" "};"); - ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str()); checkOpertorEqRetRefThis( "class szp\n" @@ -820,7 +820,7 @@ private: " szp &operator =(int *other);\n" "};\n" "szp &szp::operator =(int *other) {}"); - ASSERT_EQUALS("[test.cpp:5]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str()); } void operatorEqRetRefThis3() { @@ -889,15 +889,49 @@ private: "public:\n" " A & operator=(const A &a) { }\n" "};"); + ASSERT_EQUALS("[test.cpp:3]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str()); + + checkOpertorEqRetRefThis( + "class A {\n" + "protected:\n" + " A & operator=(const A &a) {}\n" + "};"); ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str()); + checkOpertorEqRetRefThis( + "class A {\n" + "private:\n" + " A & operator=(const A &a) {}\n" + "};"); + ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str()); + + checkOpertorEqRetRefThis( + "class A {\n" + "public:\n" + " A & operator=(const A &a) {\n" + " rand();\n" + " throw std::exception();\n" + " }\n" + "};"); + ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.\n", errout.str()); + + checkOpertorEqRetRefThis( + "class A {\n" + "public:\n" + " A & operator=(const A &a) {\n" + " rand();\n" + " abort();\n" + " }\n" + "};"); + ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.\n", errout.str()); + checkOpertorEqRetRefThis( "class A {\n" "public:\n" " A & operator=(const A &a);\n" "};\n" "A & A :: operator=(const A &a) { }"); - ASSERT_EQUALS("[test.cpp:5]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str()); } void operatorEqRetRefThis6() { // ticket #2478 (segmentation fault)