Skip to content

Commit a002654

Browse files
danmardanmar
committed
Reverted refactoring 828417c for now. It caused a major slowdown in the unused functions checking.
1 parent 298021a commit a002654

11 files changed

+71
-84
lines changed

‎cli/cppcheckexecutor.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -791,6 +791,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck, int /*argc*/, const cha
791791
}
792792
}
793793

794+
cppcheck.checkFunctionUsage();
794795
cppcheck.analyseWholeProgram();
795796
} else if (!ThreadExecutor::isEnabled()) {
796797
std::cout << "No thread support yet implemented for this platform." << std::endl;

‎lib/check.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,8 @@ class CPPCHECKLIB Check {
9494
virtual ~FileInfo() {}
9595
};
9696

97-
virtual FileInfo * getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const {
97+
virtual FileInfo * getFileInfo(const Tokenizer *tokenizer) const {
9898
(void)tokenizer;
99-
(void)settings;
10099
return nullptr;
101100
}
102101

‎lib/checkbufferoverrun.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,10 +1805,8 @@ void CheckBufferOverrun::writeOutsideBufferSizeError(const Token *tok, const std
18051805
" Please check the second and the third parameter of the function '"+strFunctionName+"'.");
18061806
}
18071807

1808-
Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const
1808+
Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer) const
18091809
{
1810-
(void)settings;
1811-
18121810
MyFileInfo *fileInfo = new MyFileInfo;
18131811

18141812
// Array usage..

‎lib/checkbufferoverrun.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
220220
};
221221

222222
/** @brief Parse current TU and extract file info */
223-
Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const;
223+
Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) const;
224224

225225
/** @brief Analyse all file infos for all TU */
226226
virtual void analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger);

‎lib/checkuninitvar.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,9 +1013,8 @@ class UninitVar : public ExecutionPath {
10131013
/// @}
10141014

10151015

1016-
Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const
1016+
Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer) const
10171017
{
1018-
(void)settings;
10191018
MyFileInfo * mfi = new MyFileInfo;
10201019
analyseFunctions(tokenizer, mfi->uvarFunctions);
10211020
// TODO: add suspicious function calls

‎lib/checkuninitvar.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class CPPCHECKLIB CheckUninitVar : public Check {
8080
};
8181

8282
/** @brief Parse current TU and extract file info */
83-
Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const;
83+
Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) const;
8484

8585
/** @brief Analyse all file infos for all TU */
8686
virtual void analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger);

‎lib/checkunusedfunctions.cpp

Lines changed: 24 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,19 @@
2525
#include <cctype>
2626
//---------------------------------------------------------------------------
2727

28+
29+
2830
// Register this check class
29-
namespace {
30-
CheckUnusedFunctions instance;
31-
}
31+
CheckUnusedFunctions CheckUnusedFunctions::instance;
32+
3233

3334
//---------------------------------------------------------------------------
3435
// FUNCTION USAGE - Check for unused functions etc
3536
//---------------------------------------------------------------------------
3637

37-
Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const
38+
void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings *settings)
3839
{
39-
if (!settings->isEnabled("unusedFunction"))
40-
return nullptr;
41-
42-
const SymbolDatabase* symbolDatabase = tokenizer->getSymbolDatabase();
43-
44-
MyFileInfo *fileInfo = new MyFileInfo;
45-
46-
const std::string FileName = tokenizer->list.getFiles().front();
40+
const SymbolDatabase* symbolDatabase = tokenizer.getSymbolDatabase();
4741

4842
// Function declarations..
4943
for (std::size_t i = 0; i < symbolDatabase->functionScopes.size(); i++) {
@@ -60,24 +54,24 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c
6054
if (func->retDef->str() == "template")
6155
continue;
6256

63-
FunctionUsage &usage = fileInfo->_functions[func->name()];
57+
FunctionUsage &usage = _functions[func->name()];
6458

6559
if (!usage.lineNumber)
6660
usage.lineNumber = func->token->linenr();
6761

6862
// No filename set yet..
6963
if (usage.filename.empty()) {
70-
usage.filename = tokenizer->list.getSourceFilePath();
64+
usage.filename = tokenizer.list.getSourceFilePath();
7165
}
7266
// Multiple files => filename = "+"
73-
else if (usage.filename != tokenizer->list.getSourceFilePath()) {
67+
else if (usage.filename != tokenizer.list.getSourceFilePath()) {
7468
//func.filename = "+";
7569
usage.usedOtherFile |= usage.usedSameFile;
7670
}
7771
}
7872

7973
// Function usage..
80-
for (const Token *tok = tokenizer->tokens(); tok; tok = tok->next()) {
74+
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
8175

8276
// parsing of library code to find called functions
8377
if (settings->library.isexecutableblock(FileName, tok->str())) {
@@ -94,11 +88,11 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c
9488
} else if (markupVarToken->str() == settings->library.blockend(FileName))
9589
scope--;
9690
else if (!settings->library.iskeyword(FileName, markupVarToken->str())) {
97-
if (fileInfo->_functions.find(markupVarToken->str()) != fileInfo->_functions.end())
98-
fileInfo->_functions[markupVarToken->str()].usedOtherFile = true;
91+
if (_functions.find(markupVarToken->str()) != _functions.end())
92+
_functions[markupVarToken->str()].usedOtherFile = true;
9993
else if (markupVarToken->next()->str() == "(") {
100-
FunctionUsage &func = fileInfo->_functions[markupVarToken->str()];
101-
func.filename = tokenizer->list.getSourceFilePath();
94+
FunctionUsage &func = _functions[markupVarToken->str()];
95+
func.filename = tokenizer.list.getSourceFilePath();
10296
if (func.filename.empty() || func.filename == "+")
10397
func.usedOtherFile = true;
10498
else
@@ -116,15 +110,15 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c
116110
if (settings->library.isexportedprefix(tok->str(), propToken->str())) {
117111
const Token* nextPropToken = propToken->next();
118112
const std::string& value = nextPropToken->str();
119-
if (fileInfo->_functions.find(value) != fileInfo->_functions.end()) {
120-
fileInfo->_functions[value].usedOtherFile = true;
113+
if (_functions.find(value) != _functions.end()) {
114+
_functions[value].usedOtherFile = true;
121115
}
122116
}
123117
if (settings->library.isexportedsuffix(tok->str(), propToken->str())) {
124118
const Token* prevPropToken = propToken->previous();
125119
const std::string& value = prevPropToken->str();
126-
if (value != ")" && fileInfo->_functions.find(value) != fileInfo->_functions.end()) {
127-
fileInfo->_functions[value].usedOtherFile = true;
120+
if (value != ")" && _functions.find(value) != _functions.end()) {
121+
_functions[value].usedOtherFile = true;
128122
}
129123
}
130124
propToken = propToken->next();
@@ -139,7 +133,7 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c
139133
while (propToken && propToken->str() != ")") {
140134
const std::string& value = propToken->str();
141135
if (!value.empty()) {
142-
fileInfo->_functions[value].usedOtherFile = true;
136+
_functions[value].usedOtherFile = true;
143137
break;
144138
}
145139
propToken = propToken->next();
@@ -164,7 +158,7 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c
164158
}
165159
if (index == argIndex) {
166160
value = value.substr(1, value.length() - 2);
167-
fileInfo->_functions[value].usedOtherFile = true;
161+
_functions[value].usedOtherFile = true;
168162
}
169163
}
170164
}
@@ -208,41 +202,21 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c
208202
}
209203

210204
if (funcname) {
211-
FunctionUsage &func = fileInfo->_functions[ funcname->str()];
205+
FunctionUsage &func = _functions[ funcname->str()];
212206

213207
if (func.filename.empty() || func.filename == "+")
214208
func.usedOtherFile = true;
215209
else
216210
func.usedSameFile = true;
217211
}
218212
}
219-
220-
return fileInfo;
221213
}
222214

223215

224216

225-
void CheckUnusedFunctions::analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger)
226-
{
227-
std::map<std::string, FunctionUsage> _functions;
228-
for (std::list<Check::FileInfo*>::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) {
229-
const MyFileInfo *f = dynamic_cast<MyFileInfo*>(*it);
230-
if (f) {
231-
for (std::map<std::string, FunctionUsage>::const_iterator it2 = f->_functions.begin(); it2 != f->_functions.end(); ++it2) {
232-
std::map<std::string, FunctionUsage>::iterator it3 = _functions.find(it2->first);
233-
if (it3 == _functions.end())
234-
_functions[it2->first] = it2->second;
235-
else {
236-
if (it3->second.filename.empty()) {
237-
it3->second.filename = it2->second.filename;
238-
it3->second.lineNumber = it2->second.lineNumber;
239-
}
240-
it3->second.usedOtherFile |= it2->second.usedOtherFile;
241-
}
242-
}
243-
}
244-
}
245217

218+
void CheckUnusedFunctions::check(ErrorLogger * const errorLogger)
219+
{
246220
for (std::map<std::string, FunctionUsage>::const_iterator it = _functions.begin(); it != _functions.end(); ++it) {
247221
const FunctionUsage &func = it->second;
248222
if (func.usedOtherFile || func.filename.empty())
@@ -259,7 +233,7 @@ void CheckUnusedFunctions::analyseWholeProgram(const std::list<Check::FileInfo*>
259233
filename = "";
260234
else
261235
filename = func.filename;
262-
CheckUnusedFunctions::unusedFunctionError(&errorLogger, filename, func.lineNumber, it->first);
236+
unusedFunctionError(errorLogger, filename, func.lineNumber, it->first);
263237
} else if (! func.usedOtherFile) {
264238
/** @todo add error message "function is only used in <file> it can be static" */
265239
/*

‎lib/checkunusedfunctions.h

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,11 @@ class CPPCHECKLIB CheckUnusedFunctions : public Check {
4343
// Parse current tokens and determine..
4444
// * Check what functions are used
4545
// * What functions are declared
46+
void parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings *settings);
4647

47-
/** @brief Parse current TU and extract file info */
48-
Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const;
48+
void check(ErrorLogger * const errorLogger);
4949

50-
/** @brief Analyse all file infos for all TU */
51-
virtual void analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger);
50+
static CheckUnusedFunctions instance;
5251

5352
private:
5453

@@ -67,7 +66,9 @@ class CPPCHECKLIB CheckUnusedFunctions : public Check {
6766
/**
6867
* Dummy implementation, just to provide error for --errorlist
6968
*/
70-
void runSimplifiedChecks(const Tokenizer *, const Settings *, ErrorLogger *) {}
69+
void runSimplifiedChecks(const Tokenizer *, const Settings *, ErrorLogger *) {
70+
71+
}
7172

7273
static std::string myName() {
7374
return "Unused functions";
@@ -77,7 +78,7 @@ class CPPCHECKLIB CheckUnusedFunctions : public Check {
7778
return "Check for functions that are never called\n";
7879
}
7980

80-
class FunctionUsage {
81+
class CPPCHECKLIB FunctionUsage {
8182
public:
8283
FunctionUsage() : lineNumber(0), usedSameFile(false), usedOtherFile(false) {
8384
}
@@ -88,10 +89,7 @@ class CPPCHECKLIB CheckUnusedFunctions : public Check {
8889
bool usedOtherFile;
8990
};
9091

91-
class MyFileInfo : public Check::FileInfo {
92-
public:
93-
std::map<std::string, FunctionUsage> _functions;
94-
};
92+
std::map<std::string, FunctionUsage> _functions;
9593
};
9694
/// @}
9795
//---------------------------------------------------------------------------

‎lib/cppcheck.cpp

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "preprocessor.h" // Preprocessor
2121
#include "tokenize.h" // Tokenizer
22+
#include "checkunusedfunctions.h"
2223

2324
#include "check.h"
2425
#include "path.h"
@@ -277,6 +278,23 @@ void CppCheck::internalError(const std::string &filename, const std::string &msg
277278
}
278279
}
279280

281+
282+
void CppCheck::checkFunctionUsage()
283+
{
284+
// This generates false positives - especially for libraries
285+
if (_settings.isEnabled("unusedFunction") && _settings._jobs == 1) {
286+
const bool verbose_orig = _settings._verbose;
287+
_settings._verbose = false;
288+
289+
if (_settings._errorsOnly == false)
290+
_errorLogger.reportOut("Checking usage of global functions..");
291+
292+
CheckUnusedFunctions::instance.check(this);
293+
294+
_settings._verbose = verbose_orig;
295+
}
296+
}
297+
280298
void CppCheck::analyseFile(std::istream &fin, const std::string &filename)
281299
{
282300
// Preprocess file..
@@ -361,6 +379,9 @@ bool CppCheck::checkFile(const std::string &code, const char FileName[], std::se
361379
(*it)->runChecks(&_tokenizer, &_settings, this);
362380
}
363381

382+
if (_settings.isEnabled("unusedFunction") && _settings._jobs == 1)
383+
CheckUnusedFunctions::instance.parseTokens(_tokenizer, FileName, &_settings);
384+
364385
executeRules("normal", _tokenizer);
365386

366387
if (!_simplify)
@@ -383,7 +404,7 @@ bool CppCheck::checkFile(const std::string &code, const char FileName[], std::se
383404

384405
// Analyse the tokens..
385406
for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) {
386-
Check::FileInfo *fi = (*it)->getFileInfo(&_tokenizer, &_settings);
407+
Check::FileInfo *fi = (*it)->getFileInfo(&_tokenizer);
387408
if (fi != nullptr)
388409
fileInfo.push_back(fi);
389410
}

‎lib/cppcheck.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
8282
*/
8383
unsigned int check(const std::string &path, const std::string &content);
8484

85+
/**
86+
* @brief Check function usage.
87+
* @note Call this after all files has been checked
88+
*/
89+
void checkFunctionUsage();
90+
8591
/**
8692
* @brief Get reference to current settings.
8793
* @return a reference to current settings

‎test/testunusedfunctions.cpp

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class TestUnusedFunctions : public TestFixture {
6565
errout.str("");
6666

6767
Settings settings;
68-
settings.addEnabled("unusedFunction");
68+
settings.addEnabled("style");
6969

7070
// Tokenize..
7171
Tokenizer tokenizer(&settings, this);
@@ -74,10 +74,8 @@ class TestUnusedFunctions : public TestFixture {
7474

7575
// Check for unused functions..
7676
CheckUnusedFunctions checkUnusedFunctions(&tokenizer, &settings, this);
77-
std::list<Check::FileInfo*> fileInfo;
78-
fileInfo.push_back(checkUnusedFunctions.getFileInfo(&tokenizer, &settings));
79-
checkUnusedFunctions.analyseWholeProgram(fileInfo,*this);
80-
delete fileInfo.back();
77+
checkUnusedFunctions.parseTokens(tokenizer, "someFile.c", &settings);
78+
checkUnusedFunctions.check(this);
8179
}
8280

8381
void incondition() {
@@ -328,8 +326,6 @@ class TestUnusedFunctions : public TestFixture {
328326

329327
const char code[] = "static void f() { }";
330328

331-
std::list<Check::FileInfo*> fileInfo;
332-
333329
for (int i = 1; i <= 2; ++i) {
334330
std::ostringstream fname;
335331
fname << "test" << i << ".cpp";
@@ -338,21 +334,16 @@ class TestUnusedFunctions : public TestFixture {
338334
errout.str("");
339335

340336
Settings settings;
341-
settings.addEnabled("unusedFunction");
342337

343338
Tokenizer tokenizer(&settings, this);
344339
std::istringstream istr(code);
345340
tokenizer.tokenize(istr, fname.str().c_str());
346341

347-
fileInfo.push_back(c.getFileInfo(&tokenizer, &settings));
342+
c.parseTokens(tokenizer, "someFile.c", &settings);
348343
}
349344

350345
// Check for unused functions..
351-
c.analyseWholeProgram(fileInfo,*this);
352-
while (!fileInfo.empty()) {
353-
delete fileInfo.back();
354-
fileInfo.pop_back();
355-
}
346+
c.check(this);
356347

357348
ASSERT_EQUALS("[test1.cpp:1]: (style) The function 'f' is never used.\n", errout.str());
358349
}

0 commit comments

Comments
 (0)