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

Refactorized CheckClass::privateFunctions: 

- Resolved todo about nested classes, fixed false negative, fixed wrong unit test
- Removed slow and unnecessary Token::findmatch
- Removed false positive when function implementation in friend class is not seen (#4384)
This commit is contained in:
PKEuS 2013-02-16 01:51:08 -08:00
parent 89cf24f23f
commit afe45ff39f
2 changed files with 31 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
lib/checkclass.cpp
View File

@ -718,15 +718,24 @@ void CheckClass::suggestInitializationList(const Token* tok, const std::string&
static bool checkFunctionUsage(const std::string& name, const Scope* scope)
{
if (!scope)
return true; // Assume its used, if scope is not seen
return true; // Assume it is used, if scope is not seen
for (std::list<Function>::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (func->functionScope) {
for (const Token *ftok = func->functionScope->classStart; ftok != func->functionScope->classEnd; ftok = ftok->next()) {
if (ftok->str() == name && ftok->next()->str() == "(") // Function called. TODO: Handle overloads
for (const Token *ftok = func->functionScope->classDef->linkAt(1); ftok != func->functionScope->classEnd; ftok = ftok->next()) {
if (ftok->str() == name) // Function used. TODO: Handle overloads
return true;
}
}
} else if ((func->type != Function::eCopyConstructor &&
func->type != Function::eOperatorEqual) ||
func->access != Private) // Assume it is used, if a function implementation isn't seen, but empty private copy constructors and assignment operators are OK
return true;
}
for (std::list<Scope*>::const_iterator i = scope->nestedList.cbegin(); i != scope->nestedList.end(); ++i) {
if ((*i)->isClassOrStruct())
if (checkFunctionUsage(name, *i)) // Check nested classes, which can access private functions of their base
return true;
}
return false; // Unused in this scope
@ -745,31 +754,11 @@ void CheckClass::privateFunctions()
if (Token::findsimplematch(scope->classStart, "; __property ;", scope->classEnd))
continue;
// check that the whole class implementation is seen
bool whole = true;
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 ||
func->type == Function::eOperatorEqual) &&
func->access == Private)
continue;
whole = false;
break;
}
}
if (!whole)
continue;
std::list<const Function*> FuncList;
/** @todo embedded class have access to private functions */
if (!scope->getNestedNonFunctions()) {
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);
}
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);
}
// Bailout for overridden virtual functions of base classes
@ -791,23 +780,8 @@ void CheckClass::privateFunctions()
for (std::list<Scope::FriendInfo>::const_iterator it = scope->friendList.begin(); !used && it != scope->friendList.end(); ++it)
used = checkFunctionUsage(funcName, it->scope);
if (!used) {
// Final check; check if the function pointer is used somewhere..
const std::string _pattern("return|throw|(|)|,|= &|" + funcName);
// or if the function address is used somewhere...
// eg. sigc::mem_fun(this, &className::classFunction)
const std::string _pattern2("& " + scope->className + " :: " + funcName);
const std::string methodAsArgument("(|, " + scope->className + " :: " + funcName + " ,|)");
const std::string methodAssigned("%var% = &| " + scope->className + " :: " + funcName);
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()->tokenDef, scope->className, funcName);
}
}
if (!used)
unusedPrivateFunctionError(FuncList.front()->tokenDef, scope->className, funcName);
FuncList.pop_front();
}

14
test/testunusedprivfunc.cpp
View File

@ -339,11 +339,11 @@ private:
" {\n"
" private:\n"
" };\n"
"\n"
"private:\n"
" static void f()\n"
" { }\n"
"};\n");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:10]: (style) Unused private function: 'A::f'\n", errout.str());
check("class A\n"
"{\n"
@ -452,6 +452,16 @@ private:
"};");
ASSERT_EQUALS("", errout.str());
check("struct Bar {\n"
" void g() { f(); }\n" // Friend class seen, but f not seen
"};\n"
"class Foo {\n"
"private:\n"
" friend Bar;\n"
" void f();\n"
"};");
ASSERT_EQUALS("", errout.str());
check("struct Bar {\n"
" void g() { f(); }\n" // Friend class seen, but f() used in it
"};\n"