make sure executors are only used in the intended context / TestSuppressions cleanups (#4963)

* added asserts to make sure executors are only used in the intended context

* TestSuppressions: specify proper job counts in `checkSuppression*()`

* TestSuppressions: enabled all asserts in `runChecks()`

* TestSuppressions: removed unnecessary setting from `checkSuppression()`

* TestSuppressions: small cleanup in the way tests are called

* TestSuppressions: use `SingleExecutor`

Author: firewave

Makefile

@@ -823,7 +823,7 @@ test/teststring.o: test/teststring.cpp lib/check.h lib/checkstring.h lib/color.h
 test/testsummaries.o: test/testsummaries.cpp lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/summaries.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsummaries.cpp
 
-test/testsuppressions.o: test/testsuppressions.cpp cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
+test/testsuppressions.o: test/testsuppressions.cpp cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h cli/singleexecutor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsuppressions.cpp
 
 test/testsymboldatabase.o: test/testsymboldatabase.cpp lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h

cli/processexecutor.cpp

@@ -32,6 +32,7 @@
 
 #include <algorithm>
 #include <numeric>
+#include <cassert>
 #include <cerrno>
 #include <csignal>
 #include <cstdlib>
@@ -61,7 +62,9 @@ using std::memset;
 
 ProcessExecutor::ProcessExecutor(const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
     : Executor(files, settings, errorLogger)
-{}
+{
+    assert(mSettings.jobs > 1);
+}
 
 ProcessExecutor::~ProcessExecutor()
 {}

cli/singleexecutor.cpp

@@ -23,6 +23,7 @@
 #include "library.h"
 #include "settings.h"
 
+#include <cassert>
 #include <list>
 #include <numeric>
 #include <utility>
@@ -32,7 +33,9 @@ class ErrorLogger;
 SingleExecutor::SingleExecutor(CppCheck &cppcheck, const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
     : Executor(files, settings, errorLogger)
     , mCppcheck(cppcheck)
-{}
+{
+    assert(mSettings.jobs == 1);
+}
 
 SingleExecutor::~SingleExecutor()
 {}

cli/threadexecutor.cpp

@@ -26,6 +26,7 @@
 #include "settings.h"
 
 #include <algorithm>
+#include <cassert>
 #include <cstdlib>
 #include <functional>
 #include <future>
@@ -41,7 +42,9 @@ enum class Color;
 
 ThreadExecutor::ThreadExecutor(const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
     : Executor(files, settings, errorLogger)
-{}
+{
+    assert(mSettings.jobs > 1);
+}
 
 ThreadExecutor::~ThreadExecutor()
 {}

test/testsuppressions.cpp

@@ -26,6 +26,7 @@
 #include "fixture.h"
 #include "helpers.h"
 #include "threadexecutor.h"
+#include "singleexecutor.h"
 
 #include <algorithm>
 #include <cstddef>
@@ -178,35 +179,35 @@ class TestSuppressions : public TestFixture {
     }
 
     // Check the suppression for multiple files
-    unsigned int checkSuppression(std::map<std::string, std::string> &files, const std::string &suppression = emptyString) {
+    unsigned int checkSuppression(std::map<std::string, std::string> &f, const std::string &suppression = emptyString) {
         // Clear the error log
         errout.str("");
+        output.str("");
+
+        std::map<std::string, std::size_t> files;
+        for (std::map<std::string, std::string>::const_iterator i = f.cbegin(); i != f.cend(); ++i) {
+            files[i->first] = i->second.size();
+        }
 
         CppCheck cppCheck(*this, true, nullptr);
         Settings& settings = cppCheck.settings();
-        settings.exitCode = 1;
+        settings.jobs = 1;
         settings.inlineSuppressions = true;
+        settings.severity.enable(Severity::information);
         if (suppression == "unusedFunction")
             settings.checks.setEnabled(Checks::unusedFunction, true);
-        settings.severity.enable(Severity::information);
-        settings.jobs = 1;
         if (!suppression.empty()) {
-            std::string r = settings.nomsg.addSuppressionLine(suppression);
-            EXPECT_EQ("", r);
+            EXPECT_EQ("", settings.nomsg.addSuppressionLine(suppression));
         }
+        SingleExecutor executor(cppCheck, files, settings, *this);
+        std::vector<std::unique_ptr<ScopedFile>> scopedfiles;
+        scopedfiles.reserve(files.size());
+        for (std::map<std::string, std::string>::const_iterator i = f.cbegin(); i != f.cend(); ++i)
+            scopedfiles.emplace_back(new ScopedFile(i->first, i->second));
 
-        unsigned int exitCode = std::accumulate(files.cbegin(), files.cend(), 0U, [&](unsigned int v, const std::pair<std::string, std::string>& f) {
-            return v | cppCheck.check(f.first, f.second);
-        });
-
-        if (cppCheck.analyseWholeProgram())
-            exitCode |= settings.exitCode;
-
-        std::map<std::string, std::size_t> files_for_report;
-        for (std::map<std::string, std::string>::const_iterator file = files.cbegin(); file != files.cend(); ++file)
-            files_for_report[file->first] = file->second.size();
+        const unsigned int exitCode = executor.check();
 
-        CppCheckExecutor::reportSuppressions(settings, false, files_for_report, *this);
+        CppCheckExecutor::reportSuppressions(settings, false, files, *this);
 
         return exitCode;
     }
@@ -219,7 +220,7 @@ class TestSuppressions : public TestFixture {
         files["test.cpp"] = strlen(code);
 
         Settings settings;
-        settings.jobs = 1;
+        settings.jobs = 2;
         settings.inlineSuppressions = true;
         settings.severity.enable(Severity::information);
         if (!suppression.empty()) {
@@ -247,7 +248,7 @@ class TestSuppressions : public TestFixture {
         files["test.cpp"] = strlen(code);
 
         Settings settings;
-        settings.jobs = 1;
+        settings.jobs = 2;
         settings.inlineSuppressions = true;
         settings.severity.enable(Severity::information);
         if (!suppression.empty()) {
@@ -298,9 +299,8 @@ class TestSuppressions : public TestFixture {
                                         "    a++;\n"
                                         "}\n",
                                         "uninitvar:test.cpp"));
-        //TODO_ASSERT_EQUALS("", "[test.cpp]: (information) Unmatched suppression: uninitvar\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
 
-        // TODO: add assert - gives different result with threads/processes
         // suppress uninitvar for this file only, without error present
         (this->*check)("void f() {\n"
                        "    int a;\n"
@@ -315,9 +315,8 @@ class TestSuppressions : public TestFixture {
                                         "    a++;\n"
                                         "}\n",
                                         "*:test.cpp"));
-        //TODO_ASSERT_EQUALS("", "[test.cpp]: (information) Unmatched suppression: *\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
 
-        // TODO: add assert - gives different result with threads/processes
         // suppress all for this file only, without error present
         (this->*check)("void f() {\n"
                        "    int a;\n"
@@ -334,14 +333,13 @@ class TestSuppressions : public TestFixture {
                                         "uninitvar:test.cpp:3"));
         ASSERT_EQUALS("", errout.str());
 
-        // TODO: add assert - gives different result with threads/processes
         // suppress uninitvar for this file and line, without error present
         (this->*check)("void f() {\n"
                        "    int a;\n"
                        "    b++;\n"
                        "}\n",
                        "uninitvar:test.cpp:3");
-        //TODO_ASSERT_EQUALS("[test.cpp:3]: (information) Unmatched suppression: uninitvar\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (information) Unmatched suppression: uninitvar\n", errout.str());
 
         // suppress uninitvar inline
         ASSERT_EQUALS(0, (this->*check)("void f() {\n"
@@ -464,15 +462,14 @@ class TestSuppressions : public TestFixture {
                                         ""));
         ASSERT_EQUALS("", errout.str());
 
-        // TODO: add assert - gives different result with threads/processes
         // suppress uninitvar inline, without error present
         (this->*check)("void f() {\n"
                        "    int a;\n"
                        "    // cppcheck-suppress uninitvar\n"
                        "    b++;\n"
                        "}\n",
                        "");
-        //TODO_ASSERT_EQUALS("[test.cpp:4]: (information) Unmatched suppression: uninitvar\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (information) Unmatched suppression: uninitvar\n", errout.str());
 
         // #5746 - exitcode
         ASSERT_EQUALS(1U,
@@ -745,16 +742,14 @@ class TestSuppressions : public TestFixture {
     }
 
     void suppressingSyntaxErrors() { // syntaxErrors should be suppressible (#7076)
-        std::map<std::string, std::string> files;
-        files["test.cpp"] = "if if\n";
+        const char code[] = "if if\n";
 
-        ASSERT_EQUALS(0, checkSuppression(files, "syntaxError:test.cpp:1"));
+        ASSERT_EQUALS(0, checkSuppression(code, "syntaxError:test.cpp:1"));
         ASSERT_EQUALS("", errout.str());
     }
 
     void suppressingSyntaxErrorsInline() { // syntaxErrors should be suppressible (#5917)
-        std::map<std::string, std::string> files;
-        files["test.cpp"] = "double result(0.0);\n"
+        const char code[] = "double result(0.0);\n"
                             "_asm\n"
                             "{\n"
                             "   // cppcheck-suppress syntaxError\n"
@@ -764,13 +759,12 @@ class TestSuppressions : public TestFixture {
                             "   fstp  QWORD PTR result  ; store a double (8 bytes)\n"
                             "   pop   EAX               ; restore EAX\n"
                             "}";
-        ASSERT_EQUALS(0, checkSuppression(files, ""));
+        ASSERT_EQUALS(0, checkSuppression(code, ""));
         ASSERT_EQUALS("", errout.str());
     }
 
     void suppressingSyntaxErrorsWhileFileRead() { // syntaxError while file read should be suppressible (PR #1333)
-        std::map<std::string, std::string> files;
-        files["test.cpp"] = "CONST (genType, KS_CONST) genService[KS_CFG_NR_OF_NVM_BLOCKS] =\n"
+        const char code[] = "CONST (genType, KS_CONST) genService[KS_CFG_NR_OF_NVM_BLOCKS] =\n"
                             "{\n"
                             "[!VAR \"BC\" = \"$BC + 1\"!][!//\n"
                             "[!IF \"(as:modconf('Ks')[1]/KsGeneral/KsType = 'KS_CFG_TYPE_KS_MASTER') and\n"
@@ -783,7 +777,7 @@ class TestSuppressions : public TestFixture {
                             "[!VAR \"BC\" = \"$BC + 1\"!][!//\n"
                             "[!ENDIF!][!//\n"
                             "};";
-        ASSERT_EQUALS(0, checkSuppression(files, "syntaxError:test.cpp:4"));
+        ASSERT_EQUALS(0, checkSuppression(code, "syntaxError:test.cpp:4"));
         ASSERT_EQUALS("", errout.str());
     }
 
@@ -813,34 +807,31 @@ class TestSuppressions : public TestFixture {
     }
 
     void suppressingSyntaxErrorAndExitCode() {
-        std::map<std::string, std::string> files;
-        files["test.cpp"] = "fi if;";
+        const char code[] = "fi if;";
 
-        ASSERT_EQUALS(0, checkSuppression(files, "*:test.cpp"));
+        ASSERT_EQUALS(0, checkSuppression(code, "*:test.cpp"));
         ASSERT_EQUALS("", errout.str());
 
         // multi files, but only suppression one
         std::map<std::string, std::string> mfiles;
         mfiles["test.cpp"] = "fi if;";
         mfiles["test2.cpp"] = "fi if";
-        ASSERT_EQUALS(1, checkSuppression(mfiles, "*:test.cpp"));
+        ASSERT_EQUALS(2, checkSuppression(mfiles, "*:test.cpp"));
         ASSERT_EQUALS("[test2.cpp:1]: (error) syntax error\n", errout.str());
 
         // multi error in file, but only suppression one error
-        std::map<std::string, std::string> file2;
-        file2["test.cpp"] = "fi fi\n"
-                            "if if;";
-        ASSERT_EQUALS(1, checkSuppression(file2, "*:test.cpp:1"));  // suppress all error at line 1 of test.cpp
+        const char code2[] = "fi fi\n"
+                             "if if;";
+        ASSERT_EQUALS(2, checkSuppression(code2, "*:test.cpp:1"));  // suppress all error at line 1 of test.cpp
         ASSERT_EQUALS("[test.cpp:2]: (error) syntax error\n", errout.str());
 
         // multi error in file, but only suppression one error (2)
-        std::map<std::string, std::string> file3;
-        file3["test.cpp"] = "void f(int x, int y){\n"
-                            "    int a = x/0;\n"
-                            "    int b = y/0;\n"
-                            "}\n"
-                            "f(0, 1);\n";
-        ASSERT_EQUALS(1, checkSuppression(file3, "zerodiv:test.cpp:3"));  // suppress 'errordiv' at line 3 of test.cpp
+        const char code3[] = "void f(int x, int y){\n"
+                             "    int a = x/0;\n"
+                             "    int b = y/0;\n"
+                             "}\n"
+                             "f(0, 1);\n";
+        ASSERT_EQUALS(2, checkSuppression(code3, "zerodiv:test.cpp:3"));  // suppress 'errordiv' at line 3 of test.cpp
     }
 
 };