Implemented support for move constructors:

Adapt code to Function::eMoveConstructor
introduced in commit eb2962792f
This commit is contained in:
Frank Zingsheim 2013-04-10 21:57:22 +02:00
parent 7fdaba43ed
commit 54e7c8f6a2
8 changed files with 169 additions and 33 deletions

View File

@ -120,8 +120,7 @@ void CheckClass::constructors()
std::vector<Usage> usage(scope->varlist.size()); std::vector<Usage> usage(scope->varlist.size());
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (!func->hasBody || !(func->type == Function::eConstructor || if (!func->hasBody || !(func->isConstructor() ||
func->type == Function::eCopyConstructor ||
func->type == Function::eOperatorEqual)) func->type == Function::eOperatorEqual))
continue; continue;
@ -160,8 +159,15 @@ void CheckClass::constructors()
} }
// Check if type can't be copied // Check if type can't be copied
if (!var->isPointer() && var->typeScope() && canNotCopy(var->typeScope())) if (!var->isPointer() && var->typeScope()) {
if (func->type == Function::eMoveConstructor) {
if (canNotMove(var->typeScope()))
continue; continue;
} else {
if (canNotCopy(var->typeScope()))
continue;
}
}
// Don't warn about unknown types in copy constructors since we // Don't warn about unknown types in copy constructors since we
// don't know if they can be copied or not.. // don't know if they can be copied or not..
@ -197,7 +203,7 @@ void CheckClass::constructors()
const Scope *varType = var->typeScope(); const Scope *varType = var->typeScope();
if (!varType || varType->type != Scope::eUnion) { if (!varType || varType->type != Scope::eUnion) {
if (func->type == Function::eConstructor && 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->argCount() == 0 && func->functionScope &&
func->arg && func->arg->link()->next() == func->functionScope->classStart && func->arg && func->arg->link()->next() == func->functionScope->classStart &&
func->functionScope->classStart->link() == func->functionScope->classStart->next()) { func->functionScope->classStart->link() == func->functionScope->classStart->next()) {
@ -324,9 +330,10 @@ bool CheckClass::canNotCopy(const Scope *scope)
bool publicCopy = false; bool publicCopy = false;
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (func->type == Function::eConstructor || func->type == Function::eCopyConstructor) if (func->isConstructor())
constructor = true; constructor = true;
if (func->type == Function::eCopyConstructor && func->access == Public) if ((func->type == Function::eCopyConstructor) &&
func->access == Public)
publicCopy = true; publicCopy = true;
else if (func->type == Function::eOperatorEqual && func->access == Public) else if (func->type == Function::eOperatorEqual && func->access == Public)
publicAssign = true; publicAssign = true;
@ -335,6 +342,30 @@ bool CheckClass::canNotCopy(const Scope *scope)
return constructor && !(publicAssign || publicCopy); 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) void CheckClass::assignVar(const std::string &varname, const Scope *scope, std::vector<Usage> &usage)
{ {
std::list<Variable>::const_iterator var; 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) 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(); const Token *ftok = func.arg->link()->next();
int level = 0; int level = 0;
for (; ftok != func.functionScope->classEnd; ftok = ftok->next()) { 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 // check if member function
if (ftok->function() && ftok->function()->nestedIn == scope && if (ftok->function() && ftok->function()->nestedIn == scope &&
ftok->function()->type != Function::eConstructor) { !ftok->function()->isConstructor()) {
const Function *member = ftok->function(); const Function *member = ftok->function();
// recursive call // recursive call
@ -621,7 +652,7 @@ void CheckClass::initializeVarList(const Function &func, std::list<const Functio
// not member function // not member function
else { else {
// could be a base class virtual function, so we assume it initializes everything // 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 /** @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 * 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]; const Scope * scope = symbolDatabase->functionScopes[i];
// Check every constructor // Check every constructor
if (!scope->function || (scope->function->type != Function::eConstructor && scope->function->type != Function::eCopyConstructor)) if (!scope->function || (!scope->function->isConstructor()))
continue; continue;
const Scope* owner = scope->functionOf; const Scope* owner = scope->functionOf;
@ -728,7 +759,8 @@ void CheckClass::initializationListUsage()
for (const Token* tok2 = tok->tokAt(2); tok2->str() != ";"; tok2 = tok2->next()) { for (const Token* tok2 = tok->tokAt(2); tok2->str() != ";"; tok2 = tok2->next()) {
if (tok2->varId()) { if (tok2->varId()) {
const Variable* var2 = tok2->variable(); 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; allowed = false;
break; break;
} }
@ -1816,7 +1848,7 @@ void CheckClass::initializerListOrder()
// iterate through all member functions looking for constructors // iterate through all member functions looking for constructors
for (func = info->functionList.begin(); func != info->functionList.end(); ++func) { 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 // check for initializer list
const Token *tok = func->arg->link()->next(); const Token *tok = func->arg->link()->next();
@ -1878,10 +1910,8 @@ void CheckClass::checkPureVirtualFunctionCall()
for (std::size_t i = 0; i < functions; ++i) { for (std::size_t i = 0; i < functions; ++i) {
const Scope * scope = symbolDatabase->functionScopes[i]; const Scope * scope = symbolDatabase->functionScopes[i];
if (scope->function == 0 || !scope->function->hasBody || if (scope->function == 0 || !scope->function->hasBody ||
!(scope->function->type==Function::eConstructor || !(scope->function->isConstructor() ||
scope->function->type==Function::eCopyConstructor || scope->function->isDestructor()))
scope->function->type==Function::eMoveConstructor ||
scope->function->type==Function::eDestructor))
continue; continue;
const std::list<const Token *> & pureVirtualFunctionCalls=callsPureVirtualFunction(*scope->function,callsPureVirtualFunctionMap); const std::list<const Token *> & pureVirtualFunctionCalls=callsPureVirtualFunction(*scope->function,callsPureVirtualFunctionMap);

View File

@ -279,6 +279,8 @@ private:
std::list<const Token *> & pureFuncStack); std::list<const Token *> & pureFuncStack);
static bool canNotCopy(const Scope *scope); static bool canNotCopy(const Scope *scope);
static bool canNotMove(const Scope *scope);
}; };
/// @} /// @}
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------

View File

@ -2368,8 +2368,8 @@ void CheckMemoryLeakInClass::variable(const Scope *scope, const Token *tokVarnam
// Inspect member functions // Inspect member functions
std::list<Function>::const_iterator func; std::list<Function>::const_iterator func;
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
const bool constructor = func->type == Function::eConstructor; const bool constructor = func->isConstructor();
const bool destructor = func->type == Function::eDestructor; const bool destructor = func->isDestructor();
if (!func->hasBody) { if (!func->hasBody) {
if (destructor) { // implementation for destructor is not seen => assume it deallocates all variables properly if (destructor) { // implementation for destructor is not seen => assume it deallocates all variables properly
deallocInDestructor = true; deallocInDestructor = true;

View File

@ -1131,7 +1131,7 @@ void CheckNullPointer::nullConstantDereference()
const Token *tok = scope->classStart; 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 tok = scope->function->token; // Check initialization list
for (; tok != scope->classEnd; tok = tok->next()) { for (; tok != scope->classEnd; tok = tok->next()) {

View File

@ -433,12 +433,11 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
function.isConst = true; function.isConst = true;
// count the number of constructors // count the number of constructors
if (function.type == Function::eConstructor) if (function.isConstructor())
scope->numConstructors++; scope->numConstructors++;
else if (function.type == Function::eCopyConstructor) { if (function.type == Function::eCopyConstructor ||
scope->numConstructors++; function.type == Function::eMoveConstructor)
scope->numCopyConstructors++; scope->numCopyOrMoveConstructors++;
}
// assume implementation is inline (definition and implementation same) // assume implementation is inline (definition and implementation same)
function.token = function.tokenDef; function.token = function.tokenDef;
@ -1987,7 +1986,7 @@ Scope::Scope(const SymbolDatabase *check_, const Token *classDef_, const Scope *
classEnd(start_->link()), classEnd(start_->link()),
nestedIn(nestedIn_), nestedIn(nestedIn_),
numConstructors(0), numConstructors(0),
numCopyConstructors(0), numCopyOrMoveConstructors(0),
type(type_), type(type_),
definedType(NULL), definedType(NULL),
functionOf(NULL), functionOf(NULL),
@ -2002,7 +2001,7 @@ Scope::Scope(const SymbolDatabase *check_, const Token *classDef_, const Scope *
classEnd(NULL), classEnd(NULL),
nestedIn(nestedIn_), nestedIn(nestedIn_),
numConstructors(0), numConstructors(0),
numCopyConstructors(0), numCopyOrMoveConstructors(0),
definedType(NULL), definedType(NULL),
functionOf(NULL), functionOf(NULL),
function(NULL) function(NULL)

View File

@ -476,6 +476,16 @@ public:
/** @brief check if this function is virtual in the base classes */ /** @brief check if this function is virtual in the base classes */
bool isImplicitlyVirtual(bool defaultVal = false) const; 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 *tokenDef; // function name token in class definition
const Token *argDef; // function argument start '(' in class definition const Token *argDef; // function argument start '(' in class definition
const Token *token; // function name token in implementation const Token *token; // function name token in implementation
@ -530,7 +540,7 @@ public:
const Scope *nestedIn; const Scope *nestedIn;
std::list<Scope *> nestedList; std::list<Scope *> nestedList;
unsigned int numConstructors; unsigned int numConstructors;
unsigned int numCopyConstructors; unsigned int numCopyOrMoveConstructors;
std::list<UsingInfo> usingList; std::list<UsingInfo> usingList;
ScopeType type; ScopeType type;
Type* definedType; Type* definedType;

View File

@ -5544,6 +5544,20 @@ private:
"};"); "};");
ASSERT_EQUALS("[test.cpp:4]: (performance) Variable 'c' is assigned in constructor body. Consider performing initialization in initialization list.\n", errout.str()); 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 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" checkInitializationListUsage("class C;\n"
"class Fred {\n" "class Fred {\n"
" C a;\n" " C a;\n"

View File

@ -190,6 +190,8 @@ private:
"{\n" "{\n"
"public:\n" "public:\n"
" Fred() : i(0) { }\n" " Fred() : i(0) { }\n"
" Fred(Fred const & other) : i(other.i) {}\n"
" Fred(Fred && other) : i(other.i) {}\n"
" int i;\n" " int i;\n"
"};"); "};");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
@ -198,6 +200,8 @@ private:
"{\n" "{\n"
"public:\n" "public:\n"
" Fred() { i = 0; }\n" " Fred() { i = 0; }\n"
" Fred(Fred const & other) {i=other.i}\n"
" Fred(Fred && other) {i=other.i}\n"
" int i;\n" " int i;\n"
"};"); "};");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
@ -206,16 +210,24 @@ private:
"{\n" "{\n"
"public:\n" "public:\n"
" Fred() { }\n" " Fred() { }\n"
" Fred(Fred const & other) {}\n"
" Fred(Fred && other) {}\n"
" int i;\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" check("struct Fred\n"
"{\n" "{\n"
" Fred() { }\n" " Fred() { }\n"
" Fred(Fred const & other) {}\n"
" Fred(Fred && other) {}\n"
" int i;\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 @@ private:
"{\n" "{\n"
" Fred();\n" " Fred();\n"
" explicit Fred(int _i);\n" " explicit Fred(int _i);\n"
" Fred(Fred const & other);\n"
" int i;\n" " int i;\n"
"};\n" "};\n"
"Fred::Fred()\n" "Fred::Fred()\n"
@ -262,7 +275,7 @@ private:
"{\n" "{\n"
" i = _i;\n" " i = _i;\n"
"}\n", true); "}\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 void simple5() { // ticket #2560
@ -1004,7 +1017,6 @@ private:
check("class B\n" check("class B\n"
"{\n" "{\n"
" B (const B & Var);\n" " B (const B & Var);\n"
" B & operator= (const B & Var);\n"
"};\n" "};\n"
"class A\n" "class A\n"
"{\n" "{\n"
@ -1012,6 +1024,39 @@ private:
"public:\n" "public:\n"
" A(){}\n" " A(){}\n"
" A(const 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" " const A& operator=(const A&){return *this;}\n"
"};"); "};");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
@ -1020,6 +1065,40 @@ private:
"{\n" "{\n"
"public:\n" "public:\n"
" B (const B & Var);\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" " B & operator= (const B & Var);\n"
"};\n" "};\n"
"class A\n" "class A\n"
@ -1028,10 +1107,12 @@ private:
"public:\n" "public:\n"
" A(){}\n" " A(){}\n"
" A(const A&){}\n" " A(const A&){}\n"
" A(A &&){}\n"
" const A& operator=(const A&){return *this;}\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" 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" check("class A\n"
"{\n" "{\n"