do not throw simplecpp::Output from Preprocessor::preprocess()

Author: firewave

lib/cppcheck.cpp

@@ -1142,22 +1142,30 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
             try {
                 TokenList tokenlist{mSettings, file.lang()};
 
-                try {
+                {
+                    bool skipCfg = false;
                     // Create tokens, skip rest of iteration if failed
                     Timer::run("Tokenizer::createTokens", mSettings.showtime, &s_timerResults, [&]() {
-                        simplecpp::TokenList tokensP = preprocessor.preprocess(currentConfig, files, true);
-                        tokenlist.createTokens(std::move(tokensP));
+                        simplecpp::OutputList outputList_cfg;
+                        simplecpp::TokenList tokensP = preprocessor.preprocess(currentConfig, files, outputList_cfg);
+                        const simplecpp::Output* o = preprocessor.handleErrors(outputList_cfg);
+                        if (!o) {
+                            tokenlist.createTokens(std::move(tokensP));
+                        }
+                        else {
+                            // #error etc during preprocessing
+                            configurationError.push_back((currentConfig.empty() ? "\'\'" : currentConfig) + " : [" + o->location.file() + ':' + std::to_string(o->location.line) + "] " + o->msg);
+                            --checkCount; // don't count invalid configurations
+
+                            if (!hasValidConfig && currCfg == *configurations.rbegin()) {
+                                // If there is no valid configuration then report error..
+                                preprocessor.error(o->location.file(), o->location.line, o->location.col, o->msg, o->type);
+                            }
+                            skipCfg = true;
+                        }
                     });
-                } catch (const simplecpp::Output &o) {
-                    // #error etc during preprocessing
-                    configurationError.push_back((currentConfig.empty() ? "\'\'" : currentConfig) + " : [" + o.location.file() + ':' + std::to_string(o.location.line) + "] " + o.msg);
-                    --checkCount; // don't count invalid configurations
-
-                    if (!hasValidConfig && currCfg == *configurations.rbegin()) {
-                        // If there is no valid configuration then report error..
-                        preprocessor.error(o.location.file(), o.location.line, o.location.col, o.msg, o.type);
-                    }
-                    continue;
+                    if (skipCfg)
+                        continue;
                 }
                 hasValidConfig = true;
 

lib/preprocessor.cpp

@@ -761,19 +761,10 @@ bool Preprocessor::hasErrors(const simplecpp::Output &output)
     return false;
 }
 
-bool Preprocessor::handleErrors(const simplecpp::OutputList& outputList, bool throwError)
+const simplecpp::Output* Preprocessor::handleErrors(const simplecpp::OutputList& outputList)
 {
     const bool showerror = (!mSettings.userDefines.empty() && !mSettings.force);
-    const bool hasError = reportOutput(outputList, showerror);
-    if (throwError) {
-        const auto it = std::find_if(outputList.cbegin(), outputList.cend(), [](const simplecpp::Output &output){
-            return hasErrors(output);
-        });
-        if (it != outputList.cend()) {
-            throw *it;
-        }
-    }
-    return hasError;
+    return reportOutput(outputList, showerror);
 }
 
 bool Preprocessor::loadFiles(std::vector<std::string> &files)
@@ -782,7 +773,7 @@ bool Preprocessor::loadFiles(std::vector<std::string> &files)
 
     simplecpp::OutputList outputList;
     mFileCache = simplecpp::load(mTokens, files, dui, &outputList);
-    return !handleErrors(outputList, false);
+    return !handleErrors(outputList);
 }
 
 void Preprocessor::removeComments()
@@ -813,28 +804,27 @@ void Preprocessor::setPlatformInfo()
     mTokens.sizeOfType["long double *"] = mSettings.platform.sizeof_pointer;
 }
 
-simplecpp::TokenList Preprocessor::preprocess(const std::string &cfg, std::vector<std::string> &files, bool throwError)
+simplecpp::TokenList Preprocessor::preprocess(const std::string &cfg, std::vector<std::string> &files, simplecpp::OutputList& outputList)
 {
     const simplecpp::DUI dui = createDUI(mSettings, cfg, mLang);
 
-    simplecpp::OutputList outputList;
     std::list<simplecpp::MacroUsage> macroUsage;
     std::list<simplecpp::IfCond> ifCond;
     simplecpp::TokenList tokens2(files);
     simplecpp::preprocess(tokens2, mTokens, files, mFileCache, dui, &outputList, &macroUsage, &ifCond);
     mMacroUsage = std::move(macroUsage);
     mIfCond = std::move(ifCond);
 
-    (void)handleErrors(outputList, throwError);
-
     tokens2.removeComments();
 
     return tokens2;
 }
 
 std::string Preprocessor::getcode(const std::string &cfg, std::vector<std::string> &files, const bool writeLocations)
 {
-    simplecpp::TokenList tokens2 = preprocess(cfg, files, false);
+    simplecpp::OutputList outputList;
+    simplecpp::TokenList tokens2 = preprocess(cfg, files, outputList);
+    handleErrors(outputList);
     unsigned int prevfile = 0;
     unsigned int line = 1;
     std::ostringstream ret;
@@ -859,14 +849,14 @@ std::string Preprocessor::getcode(const std::string &cfg, std::vector<std::strin
     return ret.str();
 }
 
-bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool showerror)
+const simplecpp::Output* Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool showerror)
 {
-    bool hasError = false;
+    const simplecpp::Output* out_ret = nullptr;
 
     for (const simplecpp::Output &out : outputList) {
         switch (out.type) {
         case simplecpp::Output::ERROR:
-            hasError = true;
+            out_ret = &out;
             if (!startsWith(out.msg,"#error") || showerror)
                 error(out.location.file(), out.location.line, out.location.col, out.msg, out.type);
             break;
@@ -884,19 +874,19 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
         case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
         case simplecpp::Output::SYNTAX_ERROR:
         case simplecpp::Output::UNHANDLED_CHAR_ERROR:
-            hasError = true;
+            out_ret = &out;
             error(out.location.file(), out.location.line, out.location.col, out.msg, out.type);
             break;
         case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
         case simplecpp::Output::FILE_NOT_FOUND:
         case simplecpp::Output::DUI_ERROR:
-            hasError = true;
+            out_ret = &out;
             error("", 0, 0, out.msg, out.type);
             break;
         }
     }
 
-    return hasError;
+    return out_ret;
 }
 
 static std::string simplecppErrToId(simplecpp::Output::Type type)

lib/preprocessor.h

@@ -118,10 +118,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
 
     void setPlatformInfo();
 
-    /**
-     * @throws simplecpp::Output thrown in case of preprocessing error if throwError is true
-     */
-    simplecpp::TokenList preprocess(const std::string &cfg, std::vector<std::string> &files, bool throwError = false);
+    simplecpp::TokenList preprocess(const std::string &cfg, std::vector<std::string> &files, simplecpp::OutputList& outputList);
 
     std::string getcode(const std::string &cfg, std::vector<std::string> &files, bool writeLocations);
 
@@ -142,18 +139,15 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
      */
     void dump(std::ostream &out) const;
 
-    bool reportOutput(const simplecpp::OutputList &outputList, bool showerror);
+    const simplecpp::Output* reportOutput(const simplecpp::OutputList &outputList, bool showerror);
 
     void error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type);
 
+    const simplecpp::Output* handleErrors(const simplecpp::OutputList &outputList);
+
 private:
     static bool hasErrors(const simplecpp::Output &output);
 
-    /**
-     * @throws simplecpp::Output thrown in case of preprocessing error if throwError is true
-     */
-    bool handleErrors(const simplecpp::OutputList &outputList, bool throwError);
-
     static void simplifyPragmaAsmPrivate(simplecpp::TokenList &tokenList);
 
     /**

test/cli/other_test.py

@@ -3986,3 +3986,51 @@ def test_max_configs(tmp_path, max_configs, number_of_configs, check_config, exp
             '{}:0:0: information: Too many #ifdef configurations - cppcheck only checks {} of {} configurations. Use --force to check all configurations. [toomanyconfigs]'
             .format(test_file, max_configs, number_of_configs)
         ]
+
+
+def test_no_valid_configuration(tmp_path):
+    test_file = tmp_path / 'test.c'
+    with open(test_file, "w") as f:
+        f.write(
+"""#include ""
+#ifdef DEF_1
+#include ""
+#endif
+""")
+
+    args = [
+        '--template=simple',
+        '--emit-duplicates',
+        '--enable=information',
+        '--suppress=checkersReport',
+        str(test_file)
+    ]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0, stdout
+    assert stdout.splitlines() == [
+        'Checking {} ...'.format(test_file)
+    ]
+    # TODO: this lacks context about the configuration which encounters these errors
+    # TODO: add message when a configuration is dropped?
+    assert stderr.splitlines() == [
+        # TODO: should only report the error once
+        '{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
+        '{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
+        '{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
+        '{}:0:0: information: This file is not analyzed. Cppcheck failed to extract a valid configuration. Use -v for more details. [noValidConfiguration]'.format(test_file)
+    ]
+
+    args += ['--check-config']
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0, stdout
+    assert stdout.splitlines() == [
+        'Checking {} ...'.format(test_file)
+    ]
+    # TODO: this lacks context about the configuration which encounters these errors
+    # TODO: add message when a configuration is dropped
+    assert stderr.splitlines() == [
+        '{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
+        '{}:1:2: error: No header in #include [syntaxError]'.format(test_file)
+    ]

test/helpers.cpp

@@ -116,7 +116,9 @@ void SimpleTokenizer2::preprocess(const char* code, std::size_t size, std::vecto
     simplecpp::TokenList tokens1(code, size, files, file0);
 
     Preprocessor preprocessor(tokens1, tokenizer.getSettings(), errorlogger, Path::identify(tokens1.getFiles()[0], false));
-    simplecpp::TokenList tokens2 = preprocessor.preprocess("", files, true);
+    simplecpp::OutputList outputList;
+    simplecpp::TokenList tokens2 = preprocessor.preprocess("", files, outputList);
+    (void)preprocessor.reportOutput(outputList, true);
 
     // Tokenizer..
     tokenizer.list.createTokens(std::move(tokens2));

test/testpreprocessor.cpp

@@ -55,8 +55,10 @@ class TestPreprocessor : public TestFixture {
         std::vector<std::string> files;
         simplecpp::TokenList tokens1 = simplecpp::TokenList(code, files, "file.cpp", &outputList);
         Preprocessor p(tokens1, settingsDefault, errorLogger, Path::identify(tokens1.getFiles()[0], false));
-        simplecpp::TokenList tokens2 = p.preprocess("", files, true);
         (void)p.reportOutput(outputList, true);
+        simplecpp::OutputList outputList_pp;
+        simplecpp::TokenList tokens2 = p.preprocess("", files, outputList_pp);
+        (void)p.reportOutput(outputList_pp, true);
         return tokens2.stringify();
     }
 

test/testtokenlist.cpp

@@ -158,7 +158,9 @@ class TestTokenList : public TestFixture {
         std::vector<std::string> files;
         simplecpp::TokenList tokens1(code, files, "poll.h", nullptr);
         Preprocessor preprocessor(tokens1, settingsDefault, *this, Path::identify(tokens1.getFiles()[0], false));
-        simplecpp::TokenList tokensP = preprocessor.preprocess("", files, true);
+        simplecpp::OutputList outputList_pp;
+        simplecpp::TokenList tokensP = preprocessor.preprocess("", files, outputList_pp);
+        ASSERT(!preprocessor.reportOutput(outputList_pp, true));
         TokenList tokenlist(settingsDefault, Standards::Language::C); // headers are treated as C files
         tokenlist.createTokens(std::move(tokensP)); // do not assert
     }