From 828417c934f150ef79053bfa50f6bd7e30620a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 15 Nov 2014 18:44:23 +0100 Subject: [PATCH] CheckUnusedFunction: Refactorings to use same infrastructure for whole program analysis as CheckUninitVar and CheckBufferOverrun --- cli/cppcheckexecutor.cpp | 1 - lib/check.h | 3 +- lib/checkbufferoverrun.cpp | 4 +- lib/checkbufferoverrun.h | 2 +- lib/checkuninitvar.cpp | 3 +- lib/checkuninitvar.h | 2 +- lib/checkunusedfunctions.cpp | 74 ++++++++++++++++++++++++------------ lib/checkunusedfunctions.h | 18 +++++---- lib/cppcheck.cpp | 23 +---------- lib/cppcheck.h | 6 --- test/testunusedfunctions.cpp | 19 ++++++--- 11 files changed, 84 insertions(+), 71 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index eef85ea3b..74bda2879 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -785,7 +785,6 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck, int /*argc*/, const cha } } - cppcheck.checkFunctionUsage(); cppcheck.analyseWholeProgram(); } else if (!ThreadExecutor::isEnabled()) { std::cout << "No thread support yet implemented for this platform." << std::endl; diff --git a/lib/check.h b/lib/check.h index af8f46a16..6328b7dfc 100644 --- a/lib/check.h +++ b/lib/check.h @@ -94,8 +94,9 @@ public: virtual ~FileInfo() {} }; - virtual FileInfo * getFileInfo(const Tokenizer *tokenizer) const { + virtual FileInfo * getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { (void)tokenizer; + (void)settings; return nullptr; } diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 72243ae41..6fd91c39a 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -1805,8 +1805,10 @@ void CheckBufferOverrun::writeOutsideBufferSizeError(const Token *tok, const std " Please check the second and the third parameter of the function '"+strFunctionName+"'."); } -Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer) const +Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { + (void)settings; + MyFileInfo *fileInfo = new MyFileInfo; // Array usage.. diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index f9ba2c343..e64bdd65a 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -220,7 +220,7 @@ public: }; /** @brief Parse current TU and extract file info */ - Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) const; + Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const; /** @brief Analyse all file infos for all TU */ virtual void analyseWholeProgram(const std::list &fileInfo, ErrorLogger &errorLogger); diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 9a290da2e..7b8e29ceb 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1013,8 +1013,9 @@ public: /// @} -Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer) const +Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { + (void)settings; MyFileInfo * mfi = new MyFileInfo; analyseFunctions(tokenizer, mfi->uvarFunctions); // TODO: add suspicious function calls diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index eb8eba297..56434b80c 100644 --- a/lib/checkuninitvar.h +++ b/lib/checkuninitvar.h @@ -80,7 +80,7 @@ public: }; /** @brief Parse current TU and extract file info */ - Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) const; + Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const; /** @brief Analyse all file infos for all TU */ virtual void analyseWholeProgram(const std::list &fileInfo, ErrorLogger &errorLogger); diff --git a/lib/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index c1dcddb17..56e3e2be8 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -25,19 +25,25 @@ #include //--------------------------------------------------------------------------- - - // Register this check class -CheckUnusedFunctions CheckUnusedFunctions::instance; - +namespace { + CheckUnusedFunctions instance; +} //--------------------------------------------------------------------------- // FUNCTION USAGE - Check for unused functions etc //--------------------------------------------------------------------------- -void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings *settings) +Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { - const SymbolDatabase* symbolDatabase = tokenizer.getSymbolDatabase(); + if (!settings->isEnabled("unusedFunction")) + return nullptr; + + const SymbolDatabase* symbolDatabase = tokenizer->getSymbolDatabase(); + + MyFileInfo *fileInfo = new MyFileInfo; + + const std::string FileName = tokenizer->list.getFiles().front(); // Function declarations.. for (std::size_t i = 0; i < symbolDatabase->functionScopes.size(); i++) { @@ -54,24 +60,24 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi if (func->retDef->str() == "template") continue; - FunctionUsage &usage = _functions[func->name()]; + FunctionUsage &usage = fileInfo->_functions[func->name()]; if (!usage.lineNumber) usage.lineNumber = func->token->linenr(); // No filename set yet.. if (usage.filename.empty()) { - usage.filename = tokenizer.list.getSourceFilePath(); + usage.filename = tokenizer->list.getSourceFilePath(); } // Multiple files => filename = "+" - else if (usage.filename != tokenizer.list.getSourceFilePath()) { + else if (usage.filename != tokenizer->list.getSourceFilePath()) { //func.filename = "+"; usage.usedOtherFile |= usage.usedSameFile; } } // Function usage.. - for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { + for (const Token *tok = tokenizer->tokens(); tok; tok = tok->next()) { // parsing of library code to find called functions if (settings->library.isexecutableblock(FileName, tok->str())) { @@ -88,11 +94,11 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi } else if (markupVarToken->str() == settings->library.blockend(FileName)) scope--; else if (!settings->library.iskeyword(FileName, markupVarToken->str())) { - if (_functions.find(markupVarToken->str()) != _functions.end()) - _functions[markupVarToken->str()].usedOtherFile = true; + if (fileInfo->_functions.find(markupVarToken->str()) != fileInfo->_functions.end()) + fileInfo->_functions[markupVarToken->str()].usedOtherFile = true; else if (markupVarToken->next()->str() == "(") { - FunctionUsage &func = _functions[markupVarToken->str()]; - func.filename = tokenizer.list.getSourceFilePath(); + FunctionUsage &func = fileInfo->_functions[markupVarToken->str()]; + func.filename = tokenizer->list.getSourceFilePath(); if (func.filename.empty() || func.filename == "+") func.usedOtherFile = true; else @@ -110,15 +116,15 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi if (settings->library.isexportedprefix(tok->str(), propToken->str())) { const Token* nextPropToken = propToken->next(); const std::string& value = nextPropToken->str(); - if (_functions.find(value) != _functions.end()) { - _functions[value].usedOtherFile = true; + if (fileInfo->_functions.find(value) != fileInfo->_functions.end()) { + fileInfo->_functions[value].usedOtherFile = true; } } if (settings->library.isexportedsuffix(tok->str(), propToken->str())) { const Token* prevPropToken = propToken->previous(); const std::string& value = prevPropToken->str(); - if (value != ")" && _functions.find(value) != _functions.end()) { - _functions[value].usedOtherFile = true; + if (value != ")" && fileInfo->_functions.find(value) != fileInfo->_functions.end()) { + fileInfo->_functions[value].usedOtherFile = true; } } propToken = propToken->next(); @@ -133,7 +139,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi while (propToken && propToken->str() != ")") { const std::string& value = propToken->str(); if (!value.empty()) { - _functions[value].usedOtherFile = true; + fileInfo->_functions[value].usedOtherFile = true; break; } propToken = propToken->next(); @@ -158,7 +164,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi } if (index == argIndex) { value = value.substr(1, value.length() - 2); - _functions[value].usedOtherFile = true; + fileInfo->_functions[value].usedOtherFile = true; } } } @@ -202,7 +208,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi } if (funcname) { - FunctionUsage &func = _functions[ funcname->str()]; + FunctionUsage &func = fileInfo->_functions[ funcname->str()]; if (func.filename.empty() || func.filename == "+") func.usedOtherFile = true; @@ -210,13 +216,33 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi func.usedSameFile = true; } } + + return fileInfo; } - -void CheckUnusedFunctions::check(ErrorLogger * const errorLogger) +void CheckUnusedFunctions::analyseWholeProgram(const std::list &fileInfo, ErrorLogger &errorLogger) { + std::map _functions; + for (std::list::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) { + const MyFileInfo *f = dynamic_cast(*it); + if (f) { + for (std::map::const_iterator it2 = f->_functions.begin(); it2 != f->_functions.end(); ++it2) { + std::map::iterator it3 = _functions.find(it2->first); + if (it3 == _functions.end()) + _functions[it2->first] = it2->second; + else { + if (it3->second.filename.empty()) { + it3->second.filename = it2->second.filename; + it3->second.lineNumber = it2->second.lineNumber; + } + it3->second.usedOtherFile |= it2->second.usedOtherFile; + } + } + } + } + for (std::map::const_iterator it = _functions.begin(); it != _functions.end(); ++it) { const FunctionUsage &func = it->second; if (func.usedOtherFile || func.filename.empty()) @@ -233,7 +259,7 @@ void CheckUnusedFunctions::check(ErrorLogger * const errorLogger) filename = ""; else filename = func.filename; - unusedFunctionError(errorLogger, filename, func.lineNumber, it->first); + CheckUnusedFunctions::unusedFunctionError(&errorLogger, filename, func.lineNumber, it->first); } else if (! func.usedOtherFile) { /** @todo add error message "function is only used in it can be static" */ /* diff --git a/lib/checkunusedfunctions.h b/lib/checkunusedfunctions.h index 9f1c10e80..c4e09597f 100644 --- a/lib/checkunusedfunctions.h +++ b/lib/checkunusedfunctions.h @@ -43,11 +43,12 @@ public: // Parse current tokens and determine.. // * Check what functions are used // * What functions are declared - void parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings *settings); - void check(ErrorLogger * const errorLogger); + /** @brief Parse current TU and extract file info */ + Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const; - static CheckUnusedFunctions instance; + /** @brief Analyse all file infos for all TU */ + virtual void analyseWholeProgram(const std::list &fileInfo, ErrorLogger &errorLogger); private: @@ -66,9 +67,7 @@ private: /** * Dummy implementation, just to provide error for --errorlist */ - void runSimplifiedChecks(const Tokenizer *, const Settings *, ErrorLogger *) { - - } + void runSimplifiedChecks(const Tokenizer *, const Settings *, ErrorLogger *) {} static std::string myName() { return "Unused functions"; @@ -78,7 +77,7 @@ private: return "Check for functions that are never called\n"; } - class CPPCHECKLIB FunctionUsage { + class FunctionUsage { public: FunctionUsage() : lineNumber(0), usedSameFile(false), usedOtherFile(false) { } @@ -89,7 +88,10 @@ private: bool usedOtherFile; }; - std::map _functions; + class MyFileInfo : public Check::FileInfo { + public: + std::map _functions; + }; }; /// @} //--------------------------------------------------------------------------- diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index ef36338a4..94236e890 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -19,7 +19,6 @@ #include "preprocessor.h" // Preprocessor #include "tokenize.h" // Tokenizer -#include "checkunusedfunctions.h" #include "check.h" #include "path.h" @@ -278,23 +277,6 @@ void CppCheck::internalError(const std::string &filename, const std::string &msg } } - -void CppCheck::checkFunctionUsage() -{ - // This generates false positives - especially for libraries - if (_settings.isEnabled("unusedFunction") && _settings._jobs == 1) { - const bool verbose_orig = _settings._verbose; - _settings._verbose = false; - - if (_settings._errorsOnly == false) - _errorLogger.reportOut("Checking usage of global functions.."); - - CheckUnusedFunctions::instance.check(this); - - _settings._verbose = verbose_orig; - } -} - void CppCheck::analyseFile(std::istream &fin, const std::string &filename) { // Preprocess file.. @@ -379,9 +361,6 @@ bool CppCheck::checkFile(const std::string &code, const char FileName[], std::se (*it)->runChecks(&_tokenizer, &_settings, this); } - if (_settings.isEnabled("unusedFunction") && _settings._jobs == 1) - CheckUnusedFunctions::instance.parseTokens(_tokenizer, FileName, &_settings); - executeRules("normal", _tokenizer); if (!_simplify) @@ -404,7 +383,7 @@ bool CppCheck::checkFile(const std::string &code, const char FileName[], std::se // Analyse the tokens.. for (std::list::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) { - Check::FileInfo *fi = (*it)->getFileInfo(&_tokenizer); + Check::FileInfo *fi = (*it)->getFileInfo(&_tokenizer, &_settings); if (fi != nullptr) fileInfo.push_back(fi); } diff --git a/lib/cppcheck.h b/lib/cppcheck.h index ee3f95c42..bfecb2c12 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -82,12 +82,6 @@ public: */ unsigned int check(const std::string &path, const std::string &content); - /** - * @brief Check function usage. - * @note Call this after all files has been checked - */ - void checkFunctionUsage(); - /** * @brief Get reference to current settings. * @return a reference to current settings diff --git a/test/testunusedfunctions.cpp b/test/testunusedfunctions.cpp index 6f8555333..895a037a8 100644 --- a/test/testunusedfunctions.cpp +++ b/test/testunusedfunctions.cpp @@ -65,7 +65,7 @@ private: errout.str(""); Settings settings; - settings.addEnabled("style"); + settings.addEnabled("unusedFunction"); // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -74,8 +74,10 @@ private: // Check for unused functions.. CheckUnusedFunctions checkUnusedFunctions(&tokenizer, &settings, this); - checkUnusedFunctions.parseTokens(tokenizer, "someFile.c", &settings); - checkUnusedFunctions.check(this); + std::list fileInfo; + fileInfo.push_back(checkUnusedFunctions.getFileInfo(&tokenizer, &settings)); + checkUnusedFunctions.analyseWholeProgram(fileInfo,*this); + delete fileInfo.back(); } void incondition() { @@ -326,6 +328,8 @@ private: const char code[] = "static void f() { }"; + std::list fileInfo; + for (int i = 1; i <= 2; ++i) { std::ostringstream fname; fname << "test" << i << ".cpp"; @@ -334,16 +338,21 @@ private: errout.str(""); Settings settings; + settings.addEnabled("unusedFunction"); Tokenizer tokenizer(&settings, this); std::istringstream istr(code); tokenizer.tokenize(istr, fname.str().c_str()); - c.parseTokens(tokenizer, "someFile.c", &settings); + fileInfo.push_back(c.getFileInfo(&tokenizer, &settings)); } // Check for unused functions.. - c.check(this); + c.analyseWholeProgram(fileInfo,*this); + while (!fileInfo.empty()) { + delete fileInfo.back(); + fileInfo.pop_back(); + } ASSERT_EQUALS("[test1.cpp:1]: (style) The function 'f' is never used.\n", errout.str()); }