Permalink
Browse files

Implemented support for move constructors:

Adapt code to Function::eMoveConstructor
introduced in commit eb29627
  • Loading branch information...
1 parent 7fdaba4 commit 54e7c8f6a25dc16cd641a801446f16915fa5d6ea @zingsheim zingsheim committed Apr 10, 2013
Showing with 168 additions and 32 deletions.
  1. +47 −17 lib/checkclass.cpp
  2. +2 −0 lib/checkclass.h
  3. +2 −2 lib/checkmemoryleak.cpp
  4. +1 −1 lib/checknullpointer.cpp
  5. +6 −7 lib/symboldatabase.cpp
  6. +11 −1 lib/symboldatabase.h
  7. +14 −0 test/testclass.cpp
  8. +85 −4 test/testconstructors.cpp
View
64 lib/checkclass.cpp
@@ -120,8 +120,7 @@ void CheckClass::constructors()
std::vector<Usage> usage(scope->varlist.size());
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
- if (!func->hasBody || !(func->type == Function::eConstructor ||
- func->type == Function::eCopyConstructor ||
+ if (!func->hasBody || !(func->isConstructor() ||
func->type == Function::eOperatorEqual))
continue;
@@ -160,8 +159,15 @@ void CheckClass::constructors()
}
// Check if type can't be copied
- if (!var->isPointer() && var->typeScope() && canNotCopy(var->typeScope()))
- continue;
+ if (!var->isPointer() && var->typeScope()) {
+ if (func->type == Function::eMoveConstructor) {
+ if (canNotMove(var->typeScope()))
+ continue;
+ } else {
+ if (canNotCopy(var->typeScope()))
+ continue;
+ }
+ }
// Don't warn about unknown types in copy constructors since we
// don't know if they can be copied or not..
@@ -197,7 +203,7 @@ void CheckClass::constructors()
const Scope *varType = var->typeScope();
if (!varType || varType->type != Scope::eUnion) {
if (func->type == Function::eConstructor &&
- func->nestedIn && (func->nestedIn->numConstructors - func->nestedIn->numCopyConstructors) > 1 &&
+ func->nestedIn && (func->nestedIn->numConstructors - func->nestedIn->numCopyOrMoveConstructors) > 1 &&
func->argCount() == 0 && func->functionScope &&
func->arg && func->arg->link()->next() == func->functionScope->classStart &&
func->functionScope->classStart->link() == func->functionScope->classStart->next()) {
@@ -324,9 +330,10 @@ bool CheckClass::canNotCopy(const Scope *scope)
bool publicCopy = false;
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
- if (func->type == Function::eConstructor || func->type == Function::eCopyConstructor)
+ if (func->isConstructor())
constructor = true;
- if (func->type == Function::eCopyConstructor && func->access == Public)
+ if ((func->type == Function::eCopyConstructor) &&
+ func->access == Public)
publicCopy = true;
else if (func->type == Function::eOperatorEqual && func->access == Public)
publicAssign = true;
@@ -335,6 +342,30 @@ bool CheckClass::canNotCopy(const Scope *scope)
return constructor && !(publicAssign || publicCopy);
}
+bool CheckClass::canNotMove(const Scope *scope)
+{
+ std::list<Function>::const_iterator func;
+ bool constructor = false;
+ bool publicAssign = false;
+ bool publicCopy = false;
+ bool publicMove = false;
+
+ for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
+ if (func->isConstructor())
+ constructor = true;
+ if ((func->type == Function::eCopyConstructor) &&
+ func->access == Public)
+ publicCopy = true;
+ else if ((func->type == Function::eMoveConstructor) &&
+ func->access == Public)
+ publicMove = true;
+ else if (func->type == Function::eOperatorEqual && func->access == Public)
+ publicAssign = true;
+ }
+
+ return constructor && !(publicAssign || publicCopy || publicMove);
+}
+
void CheckClass::assignVar(const std::string &varname, const Scope *scope, std::vector<Usage> &usage)
{
std::list<Variable>::const_iterator var;
@@ -401,7 +432,7 @@ bool CheckClass::isBaseClassFunc(const Token *tok, const Scope *scope)
void CheckClass::initializeVarList(const Function &func, std::list<const Function *> &callstack, const Scope *scope, std::vector<Usage> &usage)
{
- bool initList = func.type == Function::eConstructor || func.type == Function::eCopyConstructor;
+ bool initList = func.isConstructor();
const Token *ftok = func.arg->link()->next();
int level = 0;
for (; ftok != func.functionScope->classEnd; ftok = ftok->next()) {
@@ -582,7 +613,7 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
// check if member function
if (ftok->function() && ftok->function()->nestedIn == scope &&
- ftok->function()->type != Function::eConstructor) {
+ !ftok->function()->isConstructor()) {
const Function *member = ftok->function();
// recursive call
@@ -621,7 +652,7 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
// not member function
else {
// could be a base class virtual function, so we assume it initializes everything
- if (func.type != Function::eConstructor && isBaseClassFunc(ftok, scope)) {
+ if (!func.isConstructor() && isBaseClassFunc(ftok, scope)) {
/** @todo False Negative: we should look at the base class functions to see if they
* call any derived class virtual functions that change the derived class state
*/
@@ -712,7 +743,7 @@ void CheckClass::initializationListUsage()
const Scope * scope = symbolDatabase->functionScopes[i];
// Check every constructor
- if (!scope->function || (scope->function->type != Function::eConstructor && scope->function->type != Function::eCopyConstructor))
+ if (!scope->function || (!scope->function->isConstructor()))
continue;
const Scope* owner = scope->functionOf;
@@ -728,7 +759,8 @@ void CheckClass::initializationListUsage()
for (const Token* tok2 = tok->tokAt(2); tok2->str() != ";"; tok2 = tok2->next()) {
if (tok2->varId()) {
const Variable* var2 = tok2->variable();
- if (var2 && var2->scope() == owner) { // Is there a dependency between two member variables?
+ if (var2 && var2->scope() == owner &&
+ tok2->strAt(-1)!=".") { // Is there a dependency between two member variables?
allowed = false;
break;
}
@@ -1816,7 +1848,7 @@ void CheckClass::initializerListOrder()
// iterate through all member functions looking for constructors
for (func = info->functionList.begin(); func != info->functionList.end(); ++func) {
- if ((func->type == Function::eConstructor || func->type == Function::eCopyConstructor) && func->hasBody) {
+ if ((func->isConstructor()) && func->hasBody) {
// check for initializer list
const Token *tok = func->arg->link()->next();
@@ -1878,10 +1910,8 @@ void CheckClass::checkPureVirtualFunctionCall()
for (std::size_t i = 0; i < functions; ++i) {
const Scope * scope = symbolDatabase->functionScopes[i];
if (scope->function == 0 || !scope->function->hasBody ||
- !(scope->function->type==Function::eConstructor ||
- scope->function->type==Function::eCopyConstructor ||
- scope->function->type==Function::eMoveConstructor ||
- scope->function->type==Function::eDestructor))
+ !(scope->function->isConstructor() ||
+ scope->function->isDestructor()))
continue;
const std::list<const Token *> & pureVirtualFunctionCalls=callsPureVirtualFunction(*scope->function,callsPureVirtualFunctionMap);
View
2 lib/checkclass.h
@@ -279,6 +279,8 @@ class CPPCHECKLIB CheckClass : public Check {
std::list<const Token *> & pureFuncStack);
static bool canNotCopy(const Scope *scope);
+
+ static bool canNotMove(const Scope *scope);
};
/// @}
//---------------------------------------------------------------------------
View
4 lib/checkmemoryleak.cpp
@@ -2368,8 +2368,8 @@ void CheckMemoryLeakInClass::variable(const Scope *scope, const Token *tokVarnam
// Inspect member functions
std::list<Function>::const_iterator func;
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
- const bool constructor = func->type == Function::eConstructor;
- const bool destructor = func->type == Function::eDestructor;
+ const bool constructor = func->isConstructor();
+ const bool destructor = func->isDestructor();
if (!func->hasBody) {
if (destructor) { // implementation for destructor is not seen => assume it deallocates all variables properly
deallocInDestructor = true;
View
2 lib/checknullpointer.cpp
@@ -1131,7 +1131,7 @@ void CheckNullPointer::nullConstantDereference()
const Token *tok = scope->classStart;
- if (scope->function && (scope->function->type == Function::eConstructor || scope->function->type == Function::eCopyConstructor))
+ if (scope->function && scope->function->isConstructor())
tok = scope->function->token; // Check initialization list
for (; tok != scope->classEnd; tok = tok->next()) {
View
13 lib/symboldatabase.cpp
@@ -433,12 +433,11 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
function.isConst = true;
// count the number of constructors
- if (function.type == Function::eConstructor)
+ if (function.isConstructor())
scope->numConstructors++;
- else if (function.type == Function::eCopyConstructor) {
- scope->numConstructors++;
- scope->numCopyConstructors++;
- }
+ if (function.type == Function::eCopyConstructor ||
+ function.type == Function::eMoveConstructor)
+ scope->numCopyOrMoveConstructors++;
// assume implementation is inline (definition and implementation same)
function.token = function.tokenDef;
@@ -1987,7 +1986,7 @@ Scope::Scope(const SymbolDatabase *check_, const Token *classDef_, const Scope *
classEnd(start_->link()),
nestedIn(nestedIn_),
numConstructors(0),
- numCopyConstructors(0),
+ numCopyOrMoveConstructors(0),
type(type_),
definedType(NULL),
functionOf(NULL),
@@ -2002,7 +2001,7 @@ Scope::Scope(const SymbolDatabase *check_, const Token *classDef_, const Scope *
classEnd(NULL),
nestedIn(nestedIn_),
numConstructors(0),
- numCopyConstructors(0),
+ numCopyOrMoveConstructors(0),
definedType(NULL),
functionOf(NULL),
function(NULL)
View
12 lib/symboldatabase.h
@@ -476,6 +476,16 @@ class CPPCHECKLIB Function {
/** @brief check if this function is virtual in the base classes */
bool isImplicitlyVirtual(bool defaultVal = false) const;
+ bool isConstructor() const {
+ return type==eConstructor ||
+ type==eCopyConstructor ||
+ type==eMoveConstructor;
+ }
+
+ bool isDestructor() const {
+ return type==eDestructor;
+ }
+
const Token *tokenDef; // function name token in class definition
const Token *argDef; // function argument start '(' in class definition
const Token *token; // function name token in implementation
@@ -530,7 +540,7 @@ class CPPCHECKLIB Scope {
const Scope *nestedIn;
std::list<Scope *> nestedList;
unsigned int numConstructors;
- unsigned int numCopyConstructors;
+ unsigned int numCopyOrMoveConstructors;
std::list<UsingInfo> usingList;
ScopeType type;
Type* definedType;
View
14 test/testclass.cpp
@@ -5546,6 +5546,20 @@ class TestClass : public TestFixture {
checkInitializationListUsage("class C;\n"
"class Fred {\n"
+ " C c;\n"
+ " Fred(Fred const & other) { c = other.c; }\n"
+ "};");
+ ASSERT_EQUALS("[test.cpp:4]: (performance) Variable 'c' is assigned in constructor body. Consider performing initialization in initialization list.\n", errout.str());
+
+ checkInitializationListUsage("class C;\n"
+ "class Fred {\n"
+ " C c;\n"
+ " Fred(Fred && other) { c = other.c; }\n"
+ "};");
+ ASSERT_EQUALS("[test.cpp:4]: (performance) Variable 'c' is assigned in constructor body. Consider performing initialization in initialization list.\n", errout.str());
+
+ checkInitializationListUsage("class C;\n"
+ "class Fred {\n"
" C a;\n"
" Fred() { initB(); a = b; }\n"
"};");
View
89 test/testconstructors.cpp
@@ -190,6 +190,8 @@ class TestConstructors : public TestFixture {
"{\n"
"public:\n"
" Fred() : i(0) { }\n"
+ " Fred(Fred const & other) : i(other.i) {}\n"
+ " Fred(Fred && other) : i(other.i) {}\n"
" int i;\n"
"};");
ASSERT_EQUALS("", errout.str());
@@ -198,6 +200,8 @@ class TestConstructors : public TestFixture {
"{\n"
"public:\n"
" Fred() { i = 0; }\n"
+ " Fred(Fred const & other) {i=other.i}\n"
+ " Fred(Fred && other) {i=other.i}\n"
" int i;\n"
"};");
ASSERT_EQUALS("", errout.str());
@@ -206,16 +210,24 @@ class TestConstructors : public TestFixture {
"{\n"
"public:\n"
" Fred() { }\n"
+ " Fred(Fred const & other) {}\n"
+ " Fred(Fred && other) {}\n"
" int i;\n"
"};");
- ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str());
+ ASSERT_EQUALS("[test.cpp:4]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n"
+ "[test.cpp:5]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n"
+ "[test.cpp:6]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str());
check("struct Fred\n"
"{\n"
" Fred() { }\n"
+ " Fred(Fred const & other) {}\n"
+ " Fred(Fred && other) {}\n"
" int i;\n"
"};");
- ASSERT_EQUALS("[test.cpp:3]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str());
+ ASSERT_EQUALS("[test.cpp:3]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n"
+ "[test.cpp:4]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n"
+ "[test.cpp:5]: (warning) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str());
}
@@ -254,6 +266,7 @@ class TestConstructors : public TestFixture {
"{\n"
" Fred();\n"
" explicit Fred(int _i);\n"
+ " Fred(Fred const & other);\n"
" int i;\n"
"};\n"
"Fred::Fred()\n"
@@ -262,7 +275,7 @@ class TestConstructors : public TestFixture {
"{\n"
" i = _i;\n"
"}\n", true);
- ASSERT_EQUALS("[test.cpp:7]: (warning, inconclusive) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str());
+ ASSERT_EQUALS("[test.cpp:8]: (warning, inconclusive) Member variable 'Fred::i' is not initialized in the constructor.\n", errout.str());
}
void simple5() { // ticket #2560
@@ -1004,14 +1017,46 @@ class TestConstructors : public TestFixture {
check("class B\n"
"{\n"
" B (const B & Var);\n"
+ "};\n"
+ "class A\n"
+ "{\n"
+ " B m_SemVar;\n"
+ "public:\n"
+ " A(){}\n"
+ " A(const A&){}\n"
+ " A(A &&){}\n"
+ " const A& operator=(const A&){return *this;}\n"
+ "};");
+ ASSERT_EQUALS("", errout.str());
+
+ check("class B\n"
+ "{\n"
+ " B (B && Var);\n"
+ "};\n"
+ "class A\n"
+ "{\n"
+ " B m_SemVar;\n"
+ "public:\n"
+ " A(){}\n"
+ " A(const A&){}\n"
+ " A(A &&){}\n"
+ " const A& operator=(const A&){return *this;}\n"
+ "};");
+ ASSERT_EQUALS("", errout.str());
+
+ check("class B\n"
+ "{\n"
" B & operator= (const B & Var);\n"
+ "public:\n"
+ " B ();\n"
"};\n"
"class A\n"
"{\n"
" B m_SemVar;\n"
"public:\n"
" A(){}\n"
" A(const A&){}\n"
+ " A(A &&){}\n"
" const A& operator=(const A&){return *this;}\n"
"};");
ASSERT_EQUALS("", errout.str());
@@ -1020,6 +1065,40 @@ class TestConstructors : public TestFixture {
"{\n"
"public:\n"
" B (const B & Var);\n"
+ "};\n"
+ "class A\n"
+ "{\n"
+ " B m_SemVar;\n"
+ "public:\n"
+ " A(){}\n"
+ " A(const A&){}\n"
+ " A(A &&){}\n"
+ " const A& operator=(const A&){return *this;}\n"
+ "};");
+ ASSERT_EQUALS("[test.cpp:11]: (warning) Member variable 'A::m_SemVar' is not initialized in the constructor.\n"
+ "[test.cpp:12]: (warning) Member variable 'A::m_SemVar' is not initialized in the constructor.\n"
+ "[test.cpp:13]: (warning) Member variable 'A::m_SemVar' is not assigned a value in 'A::operator='.\n", errout.str());
+
+ check("class B\n"
+ "{\n"
+ "public:\n"
+ " B (B && Var);\n"
+ "};\n"
+ "class A\n"
+ "{\n"
+ " B m_SemVar;\n"
+ "public:\n"
+ " A(){}\n"
+ " A(const A&){}\n"
+ " A(A &&){}\n"
+ " const A& operator=(const A&){return *this;}\n"
+ "};");
+ ASSERT_EQUALS("[test.cpp:12]: (warning) Member variable 'A::m_SemVar' is not initialized in the constructor.\n", errout.str());
+
+ check("class B\n"
+ "{\n"
+ "public:\n"
+ " B ();\n"
" B & operator= (const B & Var);\n"
"};\n"
"class A\n"
@@ -1028,10 +1107,12 @@ class TestConstructors : public TestFixture {
"public:\n"
" A(){}\n"
" A(const A&){}\n"
+ " A(A &&){}\n"
" const A& operator=(const A&){return *this;}\n"
"};");
ASSERT_EQUALS("[test.cpp:12]: (warning) Member variable 'A::m_SemVar' is not initialized in the constructor.\n"
- "[test.cpp:13]: (warning) Member variable 'A::m_SemVar' is not assigned a value in 'A::operator='.\n", errout.str());
+ "[test.cpp:13]: (warning) Member variable 'A::m_SemVar' is not initialized in the constructor.\n"
+ "[test.cpp:14]: (warning) Member variable 'A::m_SemVar' is not assigned a value in 'A::operator='.\n", errout.str());
check("class A\n"
"{\n"

0 comments on commit 54e7c8f

Please sign in to comment.