From a002654c47b889942ac94d6663e42da3a4e4307d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 24 Nov 2014 06:37:08 +0100 Subject: [PATCH] Reverted refactoring 828417c for now. It caused a major slowdown in the unused functions checking. --- 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, 71 insertions(+), 84 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 7042e0442..a1d48b33b 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -791,6 +791,7 @@ 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 6328b7dfc..af8f46a16 100644 --- a/lib/check.h +++ b/lib/check.h @@ -94,9 +94,8 @@ public: virtual ~FileInfo() {} }; - virtual FileInfo * getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { + virtual FileInfo * getFileInfo(const Tokenizer *tokenizer) const { (void)tokenizer; - (void)settings; return nullptr; } diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 6fd91c39a..72243ae41 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -1805,10 +1805,8 @@ 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 Settings *settings) const +Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer) const { - (void)settings; - MyFileInfo *fileInfo = new MyFileInfo; // Array usage.. diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index e64bdd65a..f9ba2c343 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 Settings *settings) const; + Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) 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 7b8e29ceb..9a290da2e 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1013,9 +1013,8 @@ public: /// @} -Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const +Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer) 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 56434b80c..eb8eba297 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 Settings *settings) const; + Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) 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 56e3e2be8..c1dcddb17 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -25,25 +25,19 @@ #include //--------------------------------------------------------------------------- + + // Register this check class -namespace { - CheckUnusedFunctions instance; -} +CheckUnusedFunctions CheckUnusedFunctions::instance; + //--------------------------------------------------------------------------- // FUNCTION USAGE - Check for unused functions etc //--------------------------------------------------------------------------- -Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const +void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings *settings) { - if (!settings->isEnabled("unusedFunction")) - return nullptr; - - const SymbolDatabase* symbolDatabase = tokenizer->getSymbolDatabase(); - - MyFileInfo *fileInfo = new MyFileInfo; - - const std::string FileName = tokenizer->list.getFiles().front(); + const SymbolDatabase* symbolDatabase = tokenizer.getSymbolDatabase(); // Function declarations.. for (std::size_t i = 0; i < symbolDatabase->functionScopes.size(); i++) { @@ -60,24 +54,24 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c if (func->retDef->str() == "template") continue; - FunctionUsage &usage = fileInfo->_functions[func->name()]; + FunctionUsage &usage = _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())) { @@ -94,11 +88,11 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c } else if (markupVarToken->str() == settings->library.blockend(FileName)) scope--; else if (!settings->library.iskeyword(FileName, markupVarToken->str())) { - if (fileInfo->_functions.find(markupVarToken->str()) != fileInfo->_functions.end()) - fileInfo->_functions[markupVarToken->str()].usedOtherFile = true; + if (_functions.find(markupVarToken->str()) != _functions.end()) + _functions[markupVarToken->str()].usedOtherFile = true; else if (markupVarToken->next()->str() == "(") { - FunctionUsage &func = fileInfo->_functions[markupVarToken->str()]; - func.filename = tokenizer->list.getSourceFilePath(); + FunctionUsage &func = _functions[markupVarToken->str()]; + func.filename = tokenizer.list.getSourceFilePath(); if (func.filename.empty() || func.filename == "+") func.usedOtherFile = true; else @@ -116,15 +110,15 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c if (settings->library.isexportedprefix(tok->str(), propToken->str())) { const Token* nextPropToken = propToken->next(); const std::string& value = nextPropToken->str(); - if (fileInfo->_functions.find(value) != fileInfo->_functions.end()) { - fileInfo->_functions[value].usedOtherFile = true; + if (_functions.find(value) != _functions.end()) { + _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 != ")" && fileInfo->_functions.find(value) != fileInfo->_functions.end()) { - fileInfo->_functions[value].usedOtherFile = true; + if (value != ")" && _functions.find(value) != _functions.end()) { + _functions[value].usedOtherFile = true; } } propToken = propToken->next(); @@ -139,7 +133,7 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c while (propToken && propToken->str() != ")") { const std::string& value = propToken->str(); if (!value.empty()) { - fileInfo->_functions[value].usedOtherFile = true; + _functions[value].usedOtherFile = true; break; } propToken = propToken->next(); @@ -164,7 +158,7 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c } if (index == argIndex) { value = value.substr(1, value.length() - 2); - fileInfo->_functions[value].usedOtherFile = true; + _functions[value].usedOtherFile = true; } } } @@ -208,7 +202,7 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c } if (funcname) { - FunctionUsage &func = fileInfo->_functions[ funcname->str()]; + FunctionUsage &func = _functions[ funcname->str()]; if (func.filename.empty() || func.filename == "+") func.usedOtherFile = true; @@ -216,33 +210,13 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c func.usedSameFile = true; } } - - return fileInfo; } -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; - } - } - } - } +void CheckUnusedFunctions::check(ErrorLogger * const errorLogger) +{ for (std::map::const_iterator it = _functions.begin(); it != _functions.end(); ++it) { const FunctionUsage &func = it->second; if (func.usedOtherFile || func.filename.empty()) @@ -259,7 +233,7 @@ void CheckUnusedFunctions::analyseWholeProgram(const std::list filename = ""; else filename = func.filename; - CheckUnusedFunctions::unusedFunctionError(&errorLogger, filename, func.lineNumber, it->first); + 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 c4e09597f..9f1c10e80 100644 --- a/lib/checkunusedfunctions.h +++ b/lib/checkunusedfunctions.h @@ -43,12 +43,11 @@ 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); - /** @brief Parse current TU and extract file info */ - Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const; + void check(ErrorLogger * const errorLogger); - /** @brief Analyse all file infos for all TU */ - virtual void analyseWholeProgram(const std::list &fileInfo, ErrorLogger &errorLogger); + static CheckUnusedFunctions instance; private: @@ -67,7 +66,9 @@ 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"; @@ -77,7 +78,7 @@ private: return "Check for functions that are never called\n"; } - class FunctionUsage { + class CPPCHECKLIB FunctionUsage { public: FunctionUsage() : lineNumber(0), usedSameFile(false), usedOtherFile(false) { } @@ -88,10 +89,7 @@ private: bool usedOtherFile; }; - class MyFileInfo : public Check::FileInfo { - public: - std::map _functions; - }; + std::map _functions; }; /// @} //--------------------------------------------------------------------------- diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 94236e890..ef36338a4 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -19,6 +19,7 @@ #include "preprocessor.h" // Preprocessor #include "tokenize.h" // Tokenizer +#include "checkunusedfunctions.h" #include "check.h" #include "path.h" @@ -277,6 +278,23 @@ 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.. @@ -361,6 +379,9 @@ 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) @@ -383,7 +404,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, &_settings); + Check::FileInfo *fi = (*it)->getFileInfo(&_tokenizer); if (fi != nullptr) fileInfo.push_back(fi); } diff --git a/lib/cppcheck.h b/lib/cppcheck.h index bfecb2c12..ee3f95c42 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -82,6 +82,12 @@ 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 895a037a8..6f8555333 100644 --- a/test/testunusedfunctions.cpp +++ b/test/testunusedfunctions.cpp @@ -65,7 +65,7 @@ private: errout.str(""); Settings settings; - settings.addEnabled("unusedFunction"); + settings.addEnabled("style"); // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -74,10 +74,8 @@ private: // Check for unused functions.. CheckUnusedFunctions checkUnusedFunctions(&tokenizer, &settings, this); - std::list fileInfo; - fileInfo.push_back(checkUnusedFunctions.getFileInfo(&tokenizer, &settings)); - checkUnusedFunctions.analyseWholeProgram(fileInfo,*this); - delete fileInfo.back(); + checkUnusedFunctions.parseTokens(tokenizer, "someFile.c", &settings); + checkUnusedFunctions.check(this); } void incondition() { @@ -328,8 +326,6 @@ private: const char code[] = "static void f() { }"; - std::list fileInfo; - for (int i = 1; i <= 2; ++i) { std::ostringstream fname; fname << "test" << i << ".cpp"; @@ -338,21 +334,16 @@ private: errout.str(""); Settings settings; - settings.addEnabled("unusedFunction"); Tokenizer tokenizer(&settings, this); std::istringstream istr(code); tokenizer.tokenize(istr, fname.str().c_str()); - fileInfo.push_back(c.getFileInfo(&tokenizer, &settings)); + c.parseTokens(tokenizer, "someFile.c", &settings); } // Check for unused functions.. - c.analyseWholeProgram(fileInfo,*this); - while (!fileInfo.empty()) { - delete fileInfo.back(); - fileInfo.pop_back(); - } + c.check(this); ASSERT_EQUALS("[test1.cpp:1]: (style) The function 'f' is never used.\n", errout.str()); }