Skip to content

Commit c79bfdc

Browse files
Dmitry-Medanmar
Dmitry-Me
authored and
danmar
committed
CheckClass: Better checking of what operator= returns
1 parent e5c7766 commit c79bfdc

3 files changed

Lines changed: 77 additions & 5 deletions

File tree

‎lib/checkclass.cpp‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,6 +1180,8 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co
11801180
{
11811181
bool foundReturn = false;
11821182

1183+
const Token* const startTok = tok;
1184+
11831185
for (; tok && tok != last; tok = tok->next()) {
11841186
// check for return of reference to this
11851187
if (tok->str() == "return") {
@@ -1228,15 +1230,47 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co
12281230
operatorEqRetRefThisError(func->token);
12291231
}
12301232
}
1231-
if (!foundReturn)
1232-
operatorEqRetRefThisError(func->token);
1233+
if (foundReturn) {
1234+
return;
1235+
}
1236+
if (startTok->next() == last) {
1237+
if (Token::Match(func->argDef, std::string("( const " + scope->className + " &").c_str())) {
1238+
// Typical wrong way to suppress default assignment operator by declaring it and leaving empty
1239+
operatorEqMissingReturnStatementError(func->token, func->access == Public);
1240+
} else {
1241+
operatorEqMissingReturnStatementError(func->token, true);
1242+
}
1243+
return;
1244+
}
1245+
if (_settings->library.isScopeNoReturn(last, 0)) {
1246+
// Typical wrong way to prohibit default assignment operator
1247+
// by always throwing an exception or calling a noreturn function
1248+
operatorEqShouldBeLeftUnimplementedError(func->token);
1249+
return;
1250+
}
1251+
1252+
operatorEqMissingReturnStatementError(func->token, func->access == Public);
12331253
}
12341254

12351255
void CheckClass::operatorEqRetRefThisError(const Token *tok)
12361256
{
12371257
reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to 'this' instance.");
12381258
}
12391259

1260+
void CheckClass::operatorEqShouldBeLeftUnimplementedError(const Token *tok)
1261+
{
1262+
reportError(tok, Severity::style, "operatorEqShouldBeLeftUnimplemented", "'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.");
1263+
}
1264+
1265+
void CheckClass::operatorEqMissingReturnStatementError(const Token *tok, bool error)
1266+
{
1267+
if (error) {
1268+
reportError(tok, Severity::error, "operatorEqMissingReturnStatement", "No 'return' statement in non-void function causes undefined behavior.");
1269+
} else {
1270+
operatorEqRetRefThisError(tok);
1271+
}
1272+
}
1273+
12401274
//---------------------------------------------------------------------------
12411275
// ClassCheck: "C& operator=(const C& rhs) { if (this == &rhs) ... }"
12421276
// operator= should check for assignment to self

‎lib/checkclass.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ class CPPCHECKLIB CheckClass : public Check {
151151
void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived, bool inconclusive);
152152
void thisSubtractionError(const Token *tok);
153153
void operatorEqRetRefThisError(const Token *tok);
154+
void operatorEqShouldBeLeftUnimplementedError(const Token *tok);
155+
void operatorEqMissingReturnStatementError(const Token *tok, bool error);
154156
void operatorEqToSelfError(const Token *tok);
155157
void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic);
156158
void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic);
@@ -178,6 +180,8 @@ class CPPCHECKLIB CheckClass : public Check {
178180
c.virtualDestructorError(0, "Base", "Derived", false);
179181
c.thisSubtractionError(0);
180182
c.operatorEqRetRefThisError(0);
183+
c.operatorEqMissingReturnStatementError(0, true);
184+
c.operatorEqShouldBeLeftUnimplementedError(0);
181185
c.operatorEqToSelfError(0);
182186
c.checkConstError(0, "class", "function", false);
183187
c.checkConstError(0, "class", "function", true);

‎test/testclass.cpp‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -812,15 +812,15 @@ class TestClass : public TestFixture {
812812
"{\n"
813813
" szp &operator =(int *other) {};\n"
814814
"};");
815-
ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
815+
ASSERT_EQUALS("[test.cpp:3]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str());
816816

817817
checkOpertorEqRetRefThis(
818818
"class szp\n"
819819
"{\n"
820820
" szp &operator =(int *other);\n"
821821
"};\n"
822822
"szp &szp::operator =(int *other) {}");
823-
ASSERT_EQUALS("[test.cpp:5]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
823+
ASSERT_EQUALS("[test.cpp:5]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str());
824824
}
825825

826826
void operatorEqRetRefThis3() {
@@ -889,15 +889,49 @@ class TestClass : public TestFixture {
889889
"public:\n"
890890
" A & operator=(const A &a) { }\n"
891891
"};");
892+
ASSERT_EQUALS("[test.cpp:3]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str());
893+
894+
checkOpertorEqRetRefThis(
895+
"class A {\n"
896+
"protected:\n"
897+
" A & operator=(const A &a) {}\n"
898+
"};");
899+
ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
900+
901+
checkOpertorEqRetRefThis(
902+
"class A {\n"
903+
"private:\n"
904+
" A & operator=(const A &a) {}\n"
905+
"};");
892906
ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
893907

908+
checkOpertorEqRetRefThis(
909+
"class A {\n"
910+
"public:\n"
911+
" A & operator=(const A &a) {\n"
912+
" rand();\n"
913+
" throw std::exception();\n"
914+
" }\n"
915+
"};");
916+
ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.\n", errout.str());
917+
918+
checkOpertorEqRetRefThis(
919+
"class A {\n"
920+
"public:\n"
921+
" A & operator=(const A &a) {\n"
922+
" rand();\n"
923+
" abort();\n"
924+
" }\n"
925+
"};");
926+
ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.\n", errout.str());
927+
894928
checkOpertorEqRetRefThis(
895929
"class A {\n"
896930
"public:\n"
897931
" A & operator=(const A &a);\n"
898932
"};\n"
899933
"A & A :: operator=(const A &a) { }");
900-
ASSERT_EQUALS("[test.cpp:5]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
934+
ASSERT_EQUALS("[test.cpp:5]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str());
901935
}
902936

903937
void operatorEqRetRefThis6() { // ticket #2478 (segmentation fault)

0 commit comments

Comments
 (0)