walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Ticket #695: new style check : explicit declaration of ctor

This commit is contained in:
Jakub Melka 2015-03-07 20:07:54 +01:00
parent 0131bda065
commit 2af9212b16
3 changed files with 136 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
lib/checkclass.cpp
View File

@ -217,6 +217,52 @@ void CheckClass::constructors()
}
}
void CheckClass::checkExplicitConstructors()
{
if (!_settings->isEnabled("performance"))
return;
const std::size_t classes = symbolDatabase->classAndStructScopes.size();
for (std::size_t i = 0; i < classes; ++i) {
const Scope * scope = symbolDatabase->classAndStructScopes[i];
// Do not perform check, if the class/struct has not any constructors
if (scope->numConstructors == 0)
continue;
// Is class abstract? Maybe this test is over-simplification, but it will suffice for simple cases,
// and it will avoid false positives.
bool isAbstractClass = false;
for (std::list<Function>::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (func->isPure()) {
isAbstractClass = true;
break;
}
}
for (std::list<Function>::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
// We are looking for constructors, which are meeting following criteria:
// 1) Constructor is declared with a single parameter
// 2) Constructor is not declared as explicit
// 3) It is not a copy/move constructor of non-abstract class
// 4) Constructor is not marked as delete (programmer can mark the default constructor as deleted, which is ok)
if (!func->isConstructor() || func->isDelete())
continue;
if (!func->isExplicit() && func->argCount() == 1) {
// We must decide, if it is not a copy/move constructor, or it is a copy/move constructor of abstract class.
if (func->type != Function::eCopyConstructor && func->type != Function::eMoveConstructor) {
noExplicitConstructorError(func->tokenDef, scope->className, scope->type == Scope::eStruct);
}
else if (isAbstractClass) {
noExplicitCopyMoveConstructorError(func->tokenDef, scope->className, scope->type == Scope::eStruct);
}
}
}
}
}
void CheckClass::copyconstructors()
{
if (!_settings->isEnabled("style"))
@ -723,6 +769,16 @@ void CheckClass::noConstructorError(const Token *tok, const std::string &classna
"instantiated. That may cause bugs or undefined behavior.");
}
void CheckClass::noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct)
{
reportError(tok, Severity::performance, "noExplicitConstructor", "The constructor of " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' should be marked as explicit. ");
}
void CheckClass::noExplicitCopyMoveConstructorError(const Token *tok, const std::string &classname, bool isStruct)
{
reportError(tok, Severity::performance, "noExplicitCopyMoveConstructor", "The copy/move constructor of abstract " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' should be marked as explicit. ");
}
void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive)
{
reportError(tok, Severity::warning, "uninitMemberVar", "Member variable '" + classname + "::" + varname + "' is not initialized in the constructor.", inconclusive);

10
lib/checkclass.h
View File

@ -76,12 +76,17 @@ public:
checkClass.checkPureVirtualFunctionCall();
checkClass.checkDuplInheritedMembers();
checkClass.checkExplicitConstructors();
}
/** @brief %Check that all class constructors are ok */
void constructors();
/** @brief %Check that constructors with single parameter are explicit,
* if they has to be.*/
void checkExplicitConstructors();
/** @brief %Check that all private functions are called */
void privateFunctions();
@ -136,6 +141,8 @@ private:
// Reporting errors..
void noConstructorError(const Token *tok, const std::string &classname, bool isStruct);
void noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct);
void noExplicitCopyMoveConstructorError(const Token *tok, const std::string &classname, bool isStruct);
//void copyConstructorMallocError(const Token *cctor, const Token *alloc, const std::string& var_name);
void copyConstructorShallowCopyError(const Token *tok, const std::string& varname);
void noCopyConstructorError(const Token *tok, const std::string &classname, bool isStruct);
@ -165,6 +172,8 @@ private:
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckClass c(0, settings, errorLogger);
c.noConstructorError(0, "classname", false);
c.noExplicitConstructorError(0, "classname", false);
c.noExplicitCopyMoveConstructorError(0, "classname", false);
//c.copyConstructorMallocError(0, 0, "var");
c.copyConstructorShallowCopyError(0, "var");
c.noCopyConstructorError(0, "class", false);
@ -199,6 +208,7 @@ private:
return "Check the code for each class.\n"
"- Missing constructors and copy constructors\n"
//"- Missing allocation of memory in copy constructor\n"
"- Constructors which should be explicit are explicit.\n"
"- Are all variables initialized by the constructors?\n"
"- Are all variables assigned by 'operator='?\n"
"- Warn if memset, memcpy etc are used on a class\n"

70
test/testclass.cpp
View File

@ -180,6 +180,76 @@ private:
TEST_CASE(pureVirtualFunctionCallPrevented);
TEST_CASE(duplInheritedMembers);
TEST_CASE(explicitConstructors);
}
void checkExplicitConstructors(const char code[]) {
// Clear the error log
errout.str("");
Settings settings;
settings.addEnabled("performance");
// Tokenize..
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
tokenizer.simplifyTokenList2();
// Check..
CheckClass checkClass(&tokenizer, &settings, this);
checkClass.checkExplicitConstructors();
}
void explicitConstructors() {
checkExplicitConstructors("class Class \n"
"{ \n"
" Class() = delete; \n"
" Class(const Class& other) { } \n"
" Class(Class&& other) { } \n"
" explicit Class(int i) { } \n"
" explicit Class(const std::string&) { } \n"
" Class(int a, int b) { } \n"
"};");
ASSERT_EQUALS("", errout.str());
checkExplicitConstructors("class Class \n"
"{ \n"
" Class() = delete; \n"
" explicit Class(const Class& other) { } \n"
" explicit Class(Class&& other) { } \n"
" virtual int i() = 0; \n"
"};");
ASSERT_EQUALS("", errout.str());
checkExplicitConstructors("class Class \n"
"{ \n"
" Class() = delete; \n"
" Class(const Class& other) = delete; \n"
" Class(Class&& other) = delete; \n"
" virtual int i() = 0; \n"
"};");
ASSERT_EQUALS("", errout.str());
checkExplicitConstructors("class Class \n"
"{ \n"
" Class(int i) { } \n"
"};");
ASSERT_EQUALS("[test.cpp:3]: (performance) The constructor of class 'Class' should be marked as explicit. \n", errout.str());
checkExplicitConstructors("class Class \n"
"{ \n"
" Class(const Class& other) { } \n"
" virtual int i() = 0; \n"
"};");
ASSERT_EQUALS("[test.cpp:3]: (performance) The copy/move constructor of abstract class 'Class' should be marked as explicit. \n", errout.str());
checkExplicitConstructors("class Class \n"
"{ \n"
" Class(Class&& other) { } \n"
" virtual int i() = 0; \n"
"};");
ASSERT_EQUALS("[test.cpp:3]: (performance) The copy/move constructor of abstract class 'Class' should be marked as explicit. \n", errout.str());
}
void checkDuplInheritedMembers(const char code[]) {