small Check::instances() usage cleanup

- consistently use range loop
- fixed `shadowFunction` selfcheck warnings
- return a const reference

Author: firewave

cli/cmdlineparser.cpp

@@ -359,9 +359,9 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
         if (std::strcmp(argv[i], "--doc") == 0) {
             std::ostringstream doc;
             // Get documentation..
-            for (const Check * it : Check::instances()) {
-                const std::string& name(it->name());
-                const std::string info(it->classInfo());
+            for (const Check * const c : Check::instances()) {
+                const std::string& name(c->name());
+                const std::string info(c->classInfo());
                 if (!name.empty() && !info.empty())
                     doc << "## " << name << " ##\n"
                         << info << "\n";

lib/check.cpp

@@ -50,9 +50,9 @@ Check::Check(const std::string &aname)
         return i->name() > aname;
     });
     if (it == instances().end())
-        instances().push_back(this);
+        instances_internal().push_back(this);
     else
-        instances().insert(it, this);
+        instances_internal().insert(it, this);
 }
 
 void Check::writeToErrorList(const ErrorMessage &errmsg)
@@ -88,7 +88,7 @@ bool Check::wrongData(const Token *tok, const char *str)
     return true;
 }
 
-std::list<Check *> &Check::instances()
+std::list<Check *> &Check::instances_internal()
 {
 #ifdef __SVR4
     // Under Solaris, destructors are called in wrong order which causes a segmentation fault.
@@ -101,6 +101,11 @@ std::list<Check *> &Check::instances()
 #endif
 }
 
+const std::list<Check *> &Check::instances()
+{
+    return instances_internal();
+}
+
 std::string Check::getMessageId(const ValueFlow::Value &value, const char id[])
 {
     if (value.condition != nullptr)

lib/check.h

@@ -66,17 +66,20 @@ class CPPCHECKLIB Check {
     Check(std::string aname, const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
         : mTokenizer(tokenizer), mSettings(settings), mErrorLogger(errorLogger), mName(std::move(aname)) {}
 
+private:
+    static std::list<Check *> &instances_internal();
+
 public:
     virtual ~Check() {
         if (!mTokenizer)
-            instances().remove(this);
+            instances_internal().remove(this);
     }
 
     Check(const Check &) = delete;
     Check& operator=(const Check &) = delete;
 
     /** List of registered check classes. This is used by Cppcheck to run checks and generate documentation */
-    static std::list<Check *> &instances();
+    static const std::list<Check *> &instances();
 
     /** run checks, the token list is not simplified */
     virtual void runChecks(const Tokenizer &, ErrorLogger *) = 0;

lib/cppcheck.cpp

@@ -1338,8 +1338,7 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation
         const std::time_t maxTime = mSettings.checksMaxTime > 0 ? std::time(nullptr) + mSettings.checksMaxTime : 0;
 
         // call all "runChecks" in all registered Check classes
-        // cppcheck-suppress shadowFunction - TODO: fix this
-        for (Check *check : Check::instances()) {
+        for (Check * const c : Check::instances()) {
             if (Settings::terminated())
                 return;
 
@@ -1357,8 +1356,8 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation
                 return;
             }
 
-            Timer::run(check->name() + "::runChecks", mTimerResults, [&]() {
-                check->runChecks(tokenizer, &mErrorLogger);
+            Timer::run(c->name() + "::runChecks", mTimerResults, [&]() {
+                c->runChecks(tokenizer, &mErrorLogger);
             });
         }
     }
@@ -1388,11 +1387,10 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation
         }
 
         if (!doUnusedFunctionOnly) {
-            // cppcheck-suppress shadowFunction - TODO: fix this
-            for (const Check *check : Check::instances()) {
-                if (Check::FileInfo * const fi = check->getFileInfo(tokenizer, mSettings, currentConfig)) {
+            for (const Check * const c : Check::instances()) {
+                if (Check::FileInfo * const fi = c->getFileInfo(tokenizer, mSettings, currentConfig)) {
                     if (analyzerInformation)
-                        analyzerInformation->setFileInfo(check->name(), fi->toString());
+                        analyzerInformation->setFileInfo(c->name(), fi->toString());
                     if (mSettings.useSingleJob())
                         mFileInfo.push_back(fi);
                     else
@@ -1714,8 +1712,8 @@ void CppCheck::getErrorMessages(ErrorLogger &errorlogger)
     s.addEnabled("all");
 
     // call all "getErrorMessages" in all registered Check classes
-    for (auto it = Check::instances().cbegin(); it != Check::instances().cend(); ++it)
-        (*it)->getErrorMessages(&errorlogger, &s);
+    for (const Check * const c : Check::instances())
+        c->getErrorMessages(&errorlogger, &s);
 
     CheckUnusedFunctions::getErrorMessages(errorlogger);
     Preprocessor::getErrorMessages(errorlogger, s);
@@ -1832,9 +1830,8 @@ bool CppCheck::analyseWholeProgram()
             }
         }
 
-        // cppcheck-suppress shadowFunction - TODO: fix this
-        for (Check *check : Check::instances())
-            errors |= check->analyseWholeProgram(ctu, mFileInfo, mSettings, mErrorLogger);  // TODO: ctu
+        for (Check * const c : Check::instances())
+            errors |= c->analyseWholeProgram(ctu, mFileInfo, mSettings, mErrorLogger);  // TODO: ctu
     }
 
     if (mUnusedFunctionsCheck)
@@ -1881,9 +1878,8 @@ unsigned int CppCheck::analyseWholeProgram(const std::string &buildDir, const st
     }
     else {
         // Analyse the tokens
-        // cppcheck-suppress shadowFunction - TODO: fix this
-        for (Check *check : Check::instances())
-            check->analyseWholeProgram(ctuFileInfo, fileInfoList, mSettings, mErrorLogger);
+        for (Check * const c : Check::instances())
+            c->analyseWholeProgram(ctuFileInfo, fileInfoList, mSettings, mErrorLogger);
     }
 
     for (Check::FileInfo *fi : fileInfoList)

test/testcheck.cpp

@@ -33,18 +33,19 @@ class TestCheck : public TestFixture {
     }
 
     void instancesSorted() const {
-        for (auto i = Check::instances().cbegin(); i != Check::instances().cend(); ++i) {
+        const auto& checks = Check::instances();
+        for (auto i = checks.cbegin(); i != checks.cend(); ++i) {
             auto j = i;
             ++j;
-            if (j != Check::instances().cend()) {
+            if (j != checks.cend()) {
                 ASSERT_EQUALS(true, (*i)->name() < (*j)->name());
             }
         }
     }
 
     void classInfoFormat() const {
-        for (auto i = Check::instances().cbegin(); i != Check::instances().cend(); ++i) {
-            const std::string info = (*i)->classInfo();
+        for (Check * const c : Check::instances()) {
+            const std::string info = c->classInfo();
             if (!info.empty()) {
                 ASSERT('\n' != info[0]);         // No \n in the beginning
                 ASSERT('\n' == info.back());     // \n at end

test/testgarbage.cpp

@@ -285,8 +285,8 @@ class TestGarbage : public TestFixture {
         ASSERT_LOC(tokenizer.tokenize(code), file, line);
 
         // call all "runChecks" in all registered Check classes
-        for (auto it = Check::instances().cbegin(); it != Check::instances().cend(); ++it) {
-            (*it)->runChecks(tokenizer, this);
+        for (Check * const c : Check::instances()) {
+            c->runChecks(tokenizer, this);
         }
 
         return tokenizer.tokens()->stringifyList(false, false, false, true, false, nullptr, nullptr);