Skip to content

Commit 7ead32f

Browse files
firewavefirewave
authored
various optimizations (cppcheck-opensource#4535)
* avoid potentially duplicated `strTolower()` call in `Path::getFilenameExtensionInLowerCase()` * avoid unnecessary copies * use `unordered_*` containers for faster lookups * symboldatabase.cpp: do not perform call in `checkReturns()` until needed * astutils.cpp: do not perform calls in `isVariableChangedByFunctionCall()` until necessary * tokenize.cpp: small `hasIfDef()` optimization * use `unordered_map` for `CheckUnusedFunctions::FunctionUsage::mFunctions` / adjusted test case
1 parent 6634cb9 commit 7ead32f

10 files changed

Lines changed: 28 additions & 29 deletions

‎lib/astutils.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,7 +1269,7 @@ const Token* followReferences(const Token* tok, ErrorPath* errors)
12691269
auto refs = followAllReferences(tok, true, false);
12701270
if (refs.size() == 1) {
12711271
if (errors)
1272-
*errors = refs.front().errors;
1272+
*errors = std::move(refs.front().errors);
12731273
return refs.front().token;
12741274
}
12751275
return nullptr;
@@ -2328,16 +2328,17 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
23282328

23292329
if (!tok->function() && !tok->variable() && tok->isName()) {
23302330
if (settings) {
2331-
const bool requireInit = settings->library.isuninitargbad(tok, 1 + argnr);
2332-
const bool requireNonNull = settings->library.isnullargbad(tok, 1 + argnr);
23332331
// Check if direction (in, out, inout) is specified in the library configuration and use that
23342332
const Library::ArgumentChecks::Direction argDirection = settings->library.getArgDirection(tok, 1 + argnr);
23352333
if (argDirection == Library::ArgumentChecks::Direction::DIR_IN)
23362334
return false;
2337-
else if (argDirection == Library::ArgumentChecks::Direction::DIR_OUT ||
2338-
argDirection == Library::ArgumentChecks::Direction::DIR_INOUT) {
2335+
2336+
const bool requireNonNull = settings->library.isnullargbad(tok, 1 + argnr);
2337+
if (argDirection == Library::ArgumentChecks::Direction::DIR_OUT ||
2338+
argDirection == Library::ArgumentChecks::Direction::DIR_INOUT) {
23392339
if (indirect == 0 && isArray(tok1))
23402340
return true;
2341+
const bool requireInit = settings->library.isuninitargbad(tok, 1 + argnr);
23412342
// Assume that if the variable must be initialized then the indirection is 1
23422343
if (indirect > 0 && requireInit && requireNonNull)
23432344
return true;

‎lib/checkunusedfunctions.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi
199199
if (index == argIndex) {
200200
value = value.substr(1, value.length() - 2);
201201
mFunctions[value].usedOtherFile = true;
202-
mFunctionCalls.insert(value);
202+
mFunctionCalls.insert(std::move(value));
203203
}
204204
}
205205
}
@@ -306,7 +306,7 @@ static bool isOperatorFunction(const std::string & funcName)
306306
bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings& settings) const
307307
{
308308
bool errors = false;
309-
for (std::map<std::string, FunctionUsage>::const_iterator it = mFunctions.begin(); it != mFunctions.end(); ++it) {
309+
for (std::unordered_map<std::string, FunctionUsage>::const_iterator it = mFunctions.begin(); it != mFunctions.end(); ++it) {
310310
const FunctionUsage &func = it->second;
311311
if (func.usedOtherFile || func.filename.empty())
312312
continue;

‎lib/checkunusedfunctions.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@
2626
#include "config.h"
2727

2828
#include <list>
29-
#include <map>
3029
#include <set>
3130
#include <string>
31+
#include <unordered_map>
3232

3333
class ErrorLogger;
3434
class Function;
@@ -111,7 +111,7 @@ class CPPCHECKLIB CheckUnusedFunctions : public Check {
111111
bool usedOtherFile;
112112
};
113113

114-
std::map<std::string, FunctionUsage> mFunctions;
114+
std::unordered_map<std::string, FunctionUsage> mFunctions;
115115

116116
class CPPCHECKLIB FunctionDecl {
117117
public:

‎lib/library.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,14 +1508,14 @@ bool Library::reportErrors(const std::string &path) const
15081508

15091509
bool Library::isexecutableblock(const std::string &file, const std::string &token) const
15101510
{
1511-
const std::map<std::string, CodeBlock>::const_iterator it = mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
1511+
const std::unordered_map<std::string, CodeBlock>::const_iterator it = mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
15121512
return (it != mExecutableBlocks.end() && it->second.isBlock(token));
15131513
}
15141514

15151515
int Library::blockstartoffset(const std::string &file) const
15161516
{
15171517
int offset = -1;
1518-
const std::map<std::string, CodeBlock>::const_iterator map_it
1518+
const std::unordered_map<std::string, CodeBlock>::const_iterator map_it
15191519
= mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
15201520

15211521
if (map_it != mExecutableBlocks.end()) {
@@ -1526,7 +1526,7 @@ int Library::blockstartoffset(const std::string &file) const
15261526

15271527
const std::string& Library::blockstart(const std::string &file) const
15281528
{
1529-
const std::map<std::string, CodeBlock>::const_iterator map_it
1529+
const std::unordered_map<std::string, CodeBlock>::const_iterator map_it
15301530
= mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
15311531

15321532
if (map_it != mExecutableBlocks.end()) {
@@ -1537,7 +1537,7 @@ const std::string& Library::blockstart(const std::string &file) const
15371537

15381538
const std::string& Library::blockend(const std::string &file) const
15391539
{
1540-
const std::map<std::string, CodeBlock>::const_iterator map_it
1540+
const std::unordered_map<std::string, CodeBlock>::const_iterator map_it
15411541
= mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
15421542

15431543
if (map_it != mExecutableBlocks.end()) {

‎lib/library.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ class CPPCHECKLIB Library {
483483
bool unique = false;
484484
};
485485

486-
std::map<std::string, SmartPointer> smartPointers;
486+
std::unordered_map<std::string, SmartPointer> smartPointers;
487487
bool isSmartPointer(const Token *tok) const;
488488
const SmartPointer* detectSmartPointer(const Token* tok) const;
489489

@@ -634,7 +634,7 @@ class CPPCHECKLIB Library {
634634
std::map<std::string, bool> mProcessAfterCode;
635635
std::set<std::string> mMarkupExtensions; // file extensions of markup files
636636
std::map<std::string, std::set<std::string>> mKeywords; // keywords for code in the library
637-
std::map<std::string, CodeBlock> mExecutableBlocks; // keywords for blocks of executable code
637+
std::unordered_map<std::string, CodeBlock> mExecutableBlocks; // keywords for blocks of executable code
638638
std::map<std::string, ExportedFunctions> mExporters; // keywords that export variables/functions to libraries (meta-code/macros)
639639
std::map<std::string, std::set<std::string>> mImporters; // keywords that import variables/functions
640640
std::map<std::string, int> mReflection; // invocation of reflection

‎lib/path.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,14 @@ std::string Path::removeQuotationMarks(std::string path)
9797
return path;
9898
}
9999

100-
std::string Path::getFilenameExtension(const std::string &path)
100+
std::string Path::getFilenameExtension(const std::string &path, bool lowercase)
101101
{
102102
const std::string::size_type dotLocation = path.find_last_of('.');
103103
if (dotLocation == std::string::npos)
104104
return "";
105105

106106
std::string extension = path.substr(dotLocation);
107-
if (caseInsensitiveFilesystem()) {
107+
if (lowercase || caseInsensitiveFilesystem()) {
108108
// on a case insensitive filesystem the case doesn't matter so
109109
// let's return the extension in lowercase
110110
strTolower(extension);
@@ -114,9 +114,7 @@ std::string Path::getFilenameExtension(const std::string &path)
114114

115115
std::string Path::getFilenameExtensionInLowerCase(const std::string &path)
116116
{
117-
std::string extension = getFilenameExtension(path);
118-
strTolower(extension);
119-
return extension;
117+
return getFilenameExtension(path, true);
120118
}
121119

122120
std::string Path::getCurrentPath()

‎lib/path.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,10 @@ class CPPCHECKLIB Path {
8686
/**
8787
* @brief Get an extension of the filename.
8888
* @param path Path containing filename.
89+
* @param lowercase Return the extension in lower case
8990
* @return Filename extension (containing the dot, e.g. ".h" or ".CPP").
9091
*/
91-
static std::string getFilenameExtension(const std::string &path);
92+
static std::string getFilenameExtension(const std::string &path, bool lowercase = false);
9293

9394
/**
9495
* @brief Get an extension of the filename in lower case.

‎lib/symboldatabase.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,7 +1144,7 @@ void SymbolDatabase::createSymbolDatabaseSetFunctionPointers(bool firstPass)
11441144

11451145
void SymbolDatabase::createSymbolDatabaseSetTypePointers()
11461146
{
1147-
std::set<std::string> typenames;
1147+
std::unordered_set<std::string> typenames;
11481148
for (const Type &t : typeList) {
11491149
typenames.insert(t.name());
11501150
}
@@ -2879,9 +2879,9 @@ static bool checkReturns(const Function* function, bool unknown, bool emptyEnabl
28792879
if (function->type != Function::eFunction && function->type != Function::eOperatorEqual)
28802880
return false;
28812881
const Token* defStart = function->retDef;
2882-
const Token* defEnd = function->returnDefEnd();
28832882
if (!defStart)
28842883
return unknown;
2884+
const Token* defEnd = function->returnDefEnd();
28852885
if (!defEnd)
28862886
return unknown;
28872887
if (defEnd == defStart)

‎lib/tokenize.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9812,9 +9812,9 @@ bool Tokenizer::hasIfdef(const Token *start, const Token *end) const
98129812
if (!mPreprocessor)
98139813
return false;
98149814
for (const Directive &d: mPreprocessor->getDirectives()) {
9815-
if (d.str.compare(0,3,"#if") == 0 &&
9816-
d.linenr >= start->linenr() &&
9815+
if (d.linenr >= start->linenr() &&
98179816
d.linenr <= end->linenr() &&
9817+
d.str.compare(0,3,"#if") == 0 &&
98189818
start->fileIndex() < list.getFiles().size() &&
98199819
d.file == list.getFiles()[start->fileIndex()])
98209820
return true;

‎test/testunusedfunctions.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -472,11 +472,10 @@ class TestUnusedFunctions : public TestFixture {
472472
}
473473

474474
void lineNumber() {
475-
check("void foo() {}\n"
475+
check("void foo();\n"
476476
"void bar() {}\n"
477-
"int main()");
478-
ASSERT_EQUALS("[test.cpp:2]: (style) The function 'bar' is never used.\n"
479-
"[test.cpp:1]: (style) The function 'foo' is never used.\n", errout.str());
477+
"int main() {}");
478+
ASSERT_EQUALS("[test.cpp:2]: (style) The function 'bar' is never used.\n", errout.str());
480479
}
481480

482481
void ignore_declaration() {

0 commit comments

Comments
 (0)