various optimizations (#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

lib/astutils.cpp

@@ -1269,7 +1269,7 @@ const Token* followReferences(const Token* tok, ErrorPath* errors)
     auto refs = followAllReferences(tok, true, false);
     if (refs.size() == 1) {
         if (errors)
-            *errors = refs.front().errors;
+            *errors = std::move(refs.front().errors);
         return refs.front().token;
     }
     return nullptr;
@@ -2328,16 +2328,17 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
 
     if (!tok->function() && !tok->variable() && tok->isName()) {
         if (settings) {
-            const bool requireInit = settings->library.isuninitargbad(tok, 1 + argnr);
-            const bool requireNonNull = settings->library.isnullargbad(tok, 1 + argnr);
             // Check if direction (in, out, inout) is specified in the library configuration and use that
             const Library::ArgumentChecks::Direction argDirection = settings->library.getArgDirection(tok, 1 + argnr);
             if (argDirection == Library::ArgumentChecks::Direction::DIR_IN)
                 return false;
-            else if (argDirection == Library::ArgumentChecks::Direction::DIR_OUT ||
-                     argDirection == Library::ArgumentChecks::Direction::DIR_INOUT) {
+
+            const bool requireNonNull = settings->library.isnullargbad(tok, 1 + argnr);
+            if (argDirection == Library::ArgumentChecks::Direction::DIR_OUT ||
+                argDirection == Library::ArgumentChecks::Direction::DIR_INOUT) {
                 if (indirect == 0 && isArray(tok1))
                     return true;
+                const bool requireInit = settings->library.isuninitargbad(tok, 1 + argnr);
                 // Assume that if the variable must be initialized then the indirection is 1
                 if (indirect > 0 && requireInit && requireNonNull)
                     return true;

lib/checkunusedfunctions.cpp

@@ -199,7 +199,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi
                 if (index == argIndex) {
                     value = value.substr(1, value.length() - 2);
                     mFunctions[value].usedOtherFile = true;
-                    mFunctionCalls.insert(value);
+                    mFunctionCalls.insert(std::move(value));
                 }
             }
         }
@@ -306,7 +306,7 @@ static bool isOperatorFunction(const std::string & funcName)
 bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings& settings) const
 {
     bool errors = false;
-    for (std::map<std::string, FunctionUsage>::const_iterator it = mFunctions.begin(); it != mFunctions.end(); ++it) {
+    for (std::unordered_map<std::string, FunctionUsage>::const_iterator it = mFunctions.begin(); it != mFunctions.end(); ++it) {
         const FunctionUsage &func = it->second;
         if (func.usedOtherFile || func.filename.empty())
             continue;

lib/checkunusedfunctions.h

@@ -26,9 +26,9 @@
 #include "config.h"
 
 #include <list>
-#include <map>
 #include <set>
 #include <string>
+#include <unordered_map>
 
 class ErrorLogger;
 class Function;
@@ -111,7 +111,7 @@ private:
         bool usedOtherFile;
     };
 
-    std::map<std::string, FunctionUsage> mFunctions;
+    std::unordered_map<std::string, FunctionUsage> mFunctions;
 
     class CPPCHECKLIB FunctionDecl {
     public:

lib/library.cpp

@@ -1508,14 +1508,14 @@ bool Library::reportErrors(const std::string &path) const
 
 bool Library::isexecutableblock(const std::string &file, const std::string &token) const
 {
-    const std::map<std::string, CodeBlock>::const_iterator it = mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
+    const std::unordered_map<std::string, CodeBlock>::const_iterator it = mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
     return (it != mExecutableBlocks.end() && it->second.isBlock(token));
 }
 
 int Library::blockstartoffset(const std::string &file) const
 {
     int offset = -1;
-    const std::map<std::string, CodeBlock>::const_iterator map_it
+    const std::unordered_map<std::string, CodeBlock>::const_iterator map_it
         = mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
 
     if (map_it != mExecutableBlocks.end()) {
@@ -1526,7 +1526,7 @@ int Library::blockstartoffset(const std::string &file) const
 
 const std::string& Library::blockstart(const std::string &file) const
 {
-    const std::map<std::string, CodeBlock>::const_iterator map_it
+    const std::unordered_map<std::string, CodeBlock>::const_iterator map_it
         = mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
 
     if (map_it != mExecutableBlocks.end()) {
@@ -1537,7 +1537,7 @@ const std::string& Library::blockstart(const std::string &file) const
 
 const std::string& Library::blockend(const std::string &file) const
 {
-    const std::map<std::string, CodeBlock>::const_iterator map_it
+    const std::unordered_map<std::string, CodeBlock>::const_iterator map_it
         = mExecutableBlocks.find(Path::getFilenameExtensionInLowerCase(file));
 
     if (map_it != mExecutableBlocks.end()) {

lib/library.h

@@ -483,7 +483,7 @@ public:
         bool unique = false;
     };
 
-    std::map<std::string, SmartPointer> smartPointers;
+    std::unordered_map<std::string, SmartPointer> smartPointers;
     bool isSmartPointer(const Token *tok) const;
     const SmartPointer* detectSmartPointer(const Token* tok) const;
 
@@ -634,7 +634,7 @@ private:
     std::map<std::string, bool> mProcessAfterCode;
     std::set<std::string> mMarkupExtensions; // file extensions of markup files
     std::map<std::string, std::set<std::string>> mKeywords;  // keywords for code in the library
-    std::map<std::string, CodeBlock> mExecutableBlocks; // keywords for blocks of executable code
+    std::unordered_map<std::string, CodeBlock> mExecutableBlocks; // keywords for blocks of executable code
     std::map<std::string, ExportedFunctions> mExporters; // keywords that export variables/functions to libraries (meta-code/macros)
     std::map<std::string, std::set<std::string>> mImporters;  // keywords that import variables/functions
     std::map<std::string, int> mReflection; // invocation of reflection

lib/path.cpp

@@ -97,14 +97,14 @@ std::string Path::removeQuotationMarks(std::string path)
     return path;
 }
 
-std::string Path::getFilenameExtension(const std::string &path)
+std::string Path::getFilenameExtension(const std::string &path, bool lowercase)
 {
     const std::string::size_type dotLocation = path.find_last_of('.');
     if (dotLocation == std::string::npos)
         return "";
 
     std::string extension = path.substr(dotLocation);
-    if (caseInsensitiveFilesystem()) {
+    if (lowercase || caseInsensitiveFilesystem()) {
         // on a case insensitive filesystem the case doesn't matter so
         // let's return the extension in lowercase
         strTolower(extension);
@@ -114,9 +114,7 @@ std::string Path::getFilenameExtension(const std::string &path)
 
 std::string Path::getFilenameExtensionInLowerCase(const std::string &path)
 {
-    std::string extension = getFilenameExtension(path);
-    strTolower(extension);
-    return extension;
+    return getFilenameExtension(path, true);
 }
 
 std::string Path::getCurrentPath()

lib/path.h

@@ -86,9 +86,10 @@ public:
     /**
      * @brief Get an extension of the filename.
      * @param path Path containing filename.
+     * @param lowercase Return the extension in lower case
      * @return Filename extension (containing the dot, e.g. ".h" or ".CPP").
      */
-    static std::string getFilenameExtension(const std::string &path);
+    static std::string getFilenameExtension(const std::string &path, bool lowercase = false);
 
     /**
      * @brief Get an extension of the filename in lower case.

lib/symboldatabase.cpp

@@ -1144,7 +1144,7 @@ void SymbolDatabase::createSymbolDatabaseSetFunctionPointers(bool firstPass)
 
 void SymbolDatabase::createSymbolDatabaseSetTypePointers()
 {
-    std::set<std::string> typenames;
+    std::unordered_set<std::string> typenames;
     for (const Type &t : typeList) {
         typenames.insert(t.name());
     }
@@ -2879,9 +2879,9 @@ static bool checkReturns(const Function* function, bool unknown, bool emptyEnabl
     if (function->type != Function::eFunction && function->type != Function::eOperatorEqual)
         return false;
     const Token* defStart = function->retDef;
-    const Token* defEnd = function->returnDefEnd();
     if (!defStart)
         return unknown;
+    const Token* defEnd = function->returnDefEnd();
     if (!defEnd)
         return unknown;
     if (defEnd == defStart)

lib/tokenize.cpp

@@ -9812,9 +9812,9 @@ bool Tokenizer::hasIfdef(const Token *start, const Token *end) const
     if (!mPreprocessor)
         return false;
     for (const Directive &d: mPreprocessor->getDirectives()) {
-        if (d.str.compare(0,3,"#if") == 0 &&
-            d.linenr >= start->linenr() &&
+        if (d.linenr >= start->linenr() &&
             d.linenr <= end->linenr() &&
+            d.str.compare(0,3,"#if") == 0 &&
             start->fileIndex() < list.getFiles().size() &&
             d.file == list.getFiles()[start->fileIndex()])
             return true;

test/testunusedfunctions.cpp

@@ -472,11 +472,10 @@ private:
     }
 
     void lineNumber() {
-        check("void foo() {}\n"
+        check("void foo();\n"
               "void bar() {}\n"
-              "int main()");
-        ASSERT_EQUALS("[test.cpp:2]: (style) The function 'bar' is never used.\n"
-                      "[test.cpp:1]: (style) The function 'foo' is never used.\n", errout.str());
+              "int main() {}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) The function 'bar' is never used.\n", errout.str());
     }
 
     void ignore_declaration() {