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

refactorizations for CheckClass and for less false negatives related to derived classes

This commit is contained in:
PKEuS 2011-12-14 21:11:40 +01:00 committed by Daniel Marjamäki
parent 295f486cde
commit 00d6a0e877
4 changed files with 67 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
lib/checkclass.cpp
View File

@ -39,20 +39,11 @@ namespace {
CheckClass::CheckClass(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
: Check(myName(), tokenizer, settings, errorLogger),
symbolDatabase(NULL)
symbolDatabase(tokenizer?tokenizer->getSymbolDatabase():NULL)
{
}
void CheckClass::createSymbolDatabase()
{
// Multiple calls => bail out
if (symbolDatabase)
return;
symbolDatabase = _tokenizer->getSymbolDatabase();
}
//---------------------------------------------------------------------------
// ClassCheck: Check that all class constructors are ok.
//---------------------------------------------------------------------------
@ -62,8 +53,6 @@ void CheckClass::constructors()
if (!_settings->isEnabled("style"))
return;
createSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -535,12 +524,6 @@ void CheckClass::privateFunctions()
if (_tokenizer->codeWithTemplates())
return;
// dont check borland classes with properties..
if (Token::findsimplematch(_tokenizer->tokens(), "; __property ;"))
return;
createSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -548,21 +531,17 @@ void CheckClass::privateFunctions()
if (!scope->isClassOrStruct())
continue;
// don’t check derived classes
if (!scope->derivedFrom.empty())
continue;
// skip checking if there are friends
if (!scope->friendList.empty())
continue;
// Locate some class
const Token *tok1 = scope->classDef;
// dont check borland classes with properties..
if (Token::findsimplematch(scope->classStart, "; __property ;", scope->classEnd))
continue;
// check that the whole class implementation is seen
bool whole = true;
std::list<Function>::const_iterator func;
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
for (std::list<Function>::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (!func->hasBody) {
// empty private copy constructors and assignment operators are OK
if ((func->type == Function::eCopyConstructor ||
@ -574,31 +553,49 @@ void CheckClass::privateFunctions()
break;
}
}
if (!whole)
continue;
const std::string &classname = tok1->next()->str();
std::list<const Token *> FuncList;
/** @todo embedded class have access to private functions */
if (!scope->getNestedNonFunctions()) {
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
for (std::list<Function>::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
// Get private functions..
if (func->type == Function::eFunction && func->access == Private)
FuncList.push_back(func->tokenDef);
}
}
// Check that all private functions are used..
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
const Token *ftok = func->start;
if (ftok) {
const Token *etok = ftok->link();
// Bailout for overriden virtual functions of base classes
if (!scope->derivedFrom.empty()) {
for (std::vector<Scope::BaseInfo>::const_iterator i = scope->derivedFrom.begin(); i != scope->derivedFrom.end(); ++i) {
if (!i->scope || !i->scope->classStart) {
whole = false; // We need to see the complete definition of the class
break;
}
for (std::list<Function>::const_iterator func = i->scope->functionList.begin(); func != i->scope->functionList.end(); ++func) {
if (func->isVirtual) { // Only virtual functions can be overriden
// Remove function from FuncList. TODO: Handle overloads
std::list<const Token *>::iterator it = FuncList.begin();
while (it != FuncList.end()) {
if (func->token->str() == (*it)->str())
FuncList.erase(it++);
else
++it;
}
}
}
}
}
if (!whole)
continue;
for (; ftok != etok; ftok = ftok->next()) {
// Check that all private functions are used..
for (std::list<Function>::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (func->start) {
for (const Token *ftok = func->start; ftok != func->start->link(); ftok = ftok->next()) {
if (Token::Match(ftok, "%var% (")) {
// Remove function from FuncList
// Remove function from FuncList. TODO: Handle overloads
std::list<const Token *>::iterator it = FuncList.begin();
while (it != FuncList.end()) {
if (ftok->str() == (*it)->str())
@ -613,19 +610,19 @@ void CheckClass::privateFunctions()
while (!FuncList.empty()) {
// Final check; check if the function pointer is used somewhere..
const std::string _pattern("return|(|)|,|= &|" + FuncList.front()->str());
const std::string _pattern("return|throw|(|)|,|= &|" + FuncList.front()->str());
// or if the function address is used somewhere...
// eg. sigc::mem_fun(this, &className::classFunction)
const std::string _pattern2("& " + classname + " :: " + FuncList.front()->str());
const std::string methodAsArgument("(|, " + classname + " :: " + FuncList.front()->str() + " ,|)");
const std::string methodAssigned("%var% = &| " + classname + " :: " + FuncList.front()->str());
const std::string _pattern2("& " + scope->className + " :: " + FuncList.front()->str());
const std::string methodAsArgument("(|, " + scope->className + " :: " + FuncList.front()->str() + " ,|)");
const std::string methodAssigned("%var% = &| " + scope->className + " :: " + FuncList.front()->str());
if (!Token::findmatch(_tokenizer->tokens(), _pattern.c_str()) &&
!Token::findmatch(_tokenizer->tokens(), _pattern2.c_str()) &&
!Token::findmatch(_tokenizer->tokens(), methodAsArgument.c_str()) &&
!Token::findmatch(_tokenizer->tokens(), methodAssigned.c_str())) {
unusedPrivateFunctionError(FuncList.front(), classname, FuncList.front()->str());
unusedPrivateFunctionError(FuncList.front(), scope->className, FuncList.front()->str());
}
FuncList.pop_front();
}
@ -643,8 +640,6 @@ void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string
void CheckClass::noMemset()
{
createSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -744,8 +739,6 @@ void CheckClass::operatorEq()
if (!_settings->isEnabled("style"))
return;
createSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -788,8 +781,6 @@ void CheckClass::operatorEqRetRefThis()
if (!_settings->isEnabled("style"))
return;
createSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -889,8 +880,6 @@ void CheckClass::operatorEqToSelf()
if (!_settings->isEnabled("style"))
return;
createSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -1025,8 +1014,6 @@ void CheckClass::virtualDestructor()
// * derived class has non-empty destructor
// * base class is deleted
createSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -1183,8 +1170,6 @@ void CheckClass::checkConst()
return;
}
createSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -1592,8 +1577,6 @@ void CheckClass::initializerList()
if (!_settings->inconclusive)
return;
createSymbolDatabase();
std::list<Scope>::const_iterator info;
// iterate through all scopes looking for classes and structures

6
lib/checkclass.h
View File

@ -106,12 +106,6 @@ public:
void initializerList();
private:
/**
* @brief Create symbol database. For performance reasons, only call
* it if it's needed.
*/
void createSymbolDatabase();
const SymbolDatabase *symbolDatabase;
// Reporting errors..

33
test/testclass.cpp
View File

@ -206,10 +206,9 @@ private:
TEST_CASE(constVirtualFunc);
TEST_CASE(constIfCfg); // ticket #1881 - fp when there are #if
TEST_CASE(constFriend); // ticket #1921 - fp for friend function
TEST_CASE(constUnion); // ticket #2111 - fp when there are union
TEST_CASE(constUnion); // ticket #2111 - fp when there is a union
TEST_CASE(initializerList);
TEST_CASE(unusedPrivate); // ticket #2233
}
// Check the operator Equal
@ -6405,36 +6404,6 @@ private:
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (style) Member variable 'Fred::b' is in the wrong order in the initializer list.\n"
"[test.cpp:4] -> [test.cpp:2]: (style) Member variable 'Fred::a' is in the wrong order in the initializer list.\n", errout.str());
}
void check(const char code[]) {
// Clear the error buffer..
errout.str("");
Settings settings;
settings.addEnabled("style");
settings.inconclusive = true;
// Tokenize..
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
// Check..
CheckClass checkClass(&tokenizer, &settings, this);
checkClass.runSimplifiedChecks(&tokenizer, &settings, this);
}
void unusedPrivate() {
check("class A {\n"
"public:\n"
" A() { f = A::func; }\n"
" void (*f)();\n"
"private:\n"
" static void func() { }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
}
};
REGISTER_TEST(TestClass)

28
test/testunusedprivfunc.cpp
View File

@ -44,6 +44,7 @@ private:
TEST_CASE(func_pointer2);
TEST_CASE(func_pointer3);
TEST_CASE(func_pointer4); // ticket #2807
TEST_CASE(func_pointer5); // ticket #2233
TEST_CASE(ctor);
@ -301,6 +302,19 @@ private:
}
void func_pointer5() {
check("class A {\n"
"public:\n"
" A() { f = A::func; }\n"
" void (*f)();\n"
"private:\n"
" static void func() { }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
}
void ctor() {
check("class PrivateCtor\n"
"{\n"
@ -388,7 +402,7 @@ private:
}
void derivedClass() {
// skip warning in derived classes in case the function is virtual
// skip warning in derived classes in case the base class is invisible
check("class derived : public base\n"
"{\n"
"public:\n"
@ -397,6 +411,18 @@ private:
" void f();\n"
"};\n");
ASSERT_EQUALS("", errout.str());
check("class base {\n"
"public:\n"
" virtual void foo();\n"
" void bar();\n"
"};\n"
"class derived : public base {\n"
"private:\n"
" void foo() {}\n" // Skip for overrides of virtual functions of base
" void bar() {}\n" // Don't skip if no function is overriden
"};");
ASSERT_EQUALS("[test.cpp:9]: (style) Unused private function 'derived::bar'\n", errout.str());
}
void friendClass() {