From 44cb133d0de49555f7648007d5143897ee33d2f5 Mon Sep 17 00:00:00 2001 From: firewave Date: Wed, 10 Apr 2024 14:34:32 +0200 Subject: [PATCH] moved directives from `Preprocessor` to `Tokenizer` --- lib/cppcheck.cpp | 5 +- lib/preprocessor.cpp | 19 ++----- lib/preprocessor.h | 12 +---- lib/tokenize.cpp | 31 ++++++++++-- lib/tokenize.h | 5 ++ test/helpers.cpp | 4 +- test/testpreprocessor.cpp | 88 +-------------------------------- test/testtokenize.cpp | 101 ++++++++++++++++++++++++++++++++++++++ test/testunusedvar.cpp | 4 +- 9 files changed, 148 insertions(+), 121 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 188d54977d25..313b6d5d1ac1 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -796,7 +796,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string } // Get directives - preprocessor.setDirectives(tokens1); + std::list directives = preprocessor.createDirectives(tokens1); preprocessor.simplifyPragmaAsm(&tokens1); preprocessor.setPlatformInfo(&tokens1); @@ -821,7 +821,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string // Run define rules on raw code if (hasRule("define")) { std::string code; - for (const Directive &dir : preprocessor.getDirectives()) { + for (const Directive &dir : directives) { if (startsWith(dir.str,"#define ") || startsWith(dir.str,"#include ")) code += "#line " + std::to_string(dir.linenr) + " \"" + dir.file + "\"\n" + dir.str + '\n'; } @@ -890,6 +890,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string Tokenizer tokenizer(mSettings, this, &preprocessor); if (mSettings.showtime != SHOWTIME_MODES::SHOWTIME_NONE) tokenizer.setTimerResults(&s_timerResults); + tokenizer.setDirectives(directives); // TODO: how to avoid repeated copies? try { // Create tokens, skip rest of iteration if failed diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index dd449ce3e927..ba5213470aae 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -308,10 +308,10 @@ void Preprocessor::inlineSuppressions(const simplecpp::TokenList &tokens, Suppre } } -void Preprocessor::setDirectives(const simplecpp::TokenList &tokens) +std::list Preprocessor::createDirectives(const simplecpp::TokenList &tokens) const { // directive list.. - mDirectives.clear(); + std::list directives; std::vector list; list.reserve(1U + mTokenLists.size()); @@ -337,9 +337,11 @@ void Preprocessor::setDirectives(const simplecpp::TokenList &tokens) else directive.str += tok2->str(); } - mDirectives.push_back(std::move(directive)); + directives.push_back(std::move(directive)); } } + + return directives; } static std::string readcondition(const simplecpp::Token *iftok, const std::set &defined, const std::set &undefined) @@ -912,17 +914,6 @@ void Preprocessor::dump(std::ostream &out) const { // Create a xml dump. - out << " " << std::endl; - for (const Directive &dir : mDirectives) { - out << " ' which - // could result in invalid XML, so run it through toxml(). - << "str=\"" << ErrorLogger::toxml(dir.str) << "\"/>" << std::endl; - } - out << " " << std::endl; - if (!mMacroUsage.empty()) { out << " " << std::endl; for (const simplecpp::MacroUsage ¯oUsage: mMacroUsage) { diff --git a/lib/preprocessor.h b/lib/preprocessor.h index 03b84af5d67b..45e9b873f4f8 100644 --- a/lib/preprocessor.h +++ b/lib/preprocessor.h @@ -92,12 +92,7 @@ class CPPCHECKLIB Preprocessor { void inlineSuppressions(const simplecpp::TokenList &tokens, SuppressionList &suppressions); - void setDirectives(const simplecpp::TokenList &tokens); - - /** list of all directives met while preprocessing file */ - const std::list &getDirectives() const { - return mDirectives; - } + std::list createDirectives(const simplecpp::TokenList &tokens) const; std::set getConfigs(const simplecpp::TokenList &tokens) const; @@ -143,15 +138,10 @@ class CPPCHECKLIB Preprocessor { static bool hasErrors(const simplecpp::OutputList &outputList); - void setDirectives(const std::list &directives) { - mDirectives = directives; - } - const Settings& mSettings; ErrorLogger *mErrorLogger; /** list of all directives met while preprocessing file */ - std::list mDirectives; std::map mTokenLists; diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 56498b0d7869..464877277192 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -5913,6 +5913,26 @@ void Tokenizer::dump(std::ostream &out) const std::set containers; + outs += " "; + outs += '\n'; + for (const Directive &dir : mDirectives) { + outs += " ' which + // could result in invalid XML, so run it through toxml(). + outs += "str=\""; + outs += ErrorLogger::toxml(dir.str); + outs +="\"/>"; + outs += '\n'; + } + outs += " "; + outs += '\n'; + // tokens.. outs += " "; outs += '\n'; @@ -10649,12 +10669,17 @@ void Tokenizer::simplifyNamespaceAliases() } } -// TODO: how to move the Preprocessor dependency out of here? +void Tokenizer::setDirectives(std::list directives) +{ + mDirectives = std::move(directives); +} + bool Tokenizer::hasIfdef(const Token *start, const Token *end) const { assert(mPreprocessor); - return std::any_of(mPreprocessor->getDirectives().cbegin(), mPreprocessor->getDirectives().cend(), [&](const Directive& d) { + const auto& directives = mDirectives; + return std::any_of(directives.cbegin(), directives.cend(), [&](const Directive& d) { return startsWith(d.str, "#if") && d.linenr >= start->linenr() && d.linenr <= end->linenr() && @@ -10667,7 +10692,7 @@ bool Tokenizer::isPacked(const Token * bodyStart) const { assert(mPreprocessor); - const auto& directives = mPreprocessor->getDirectives(); + const auto& directives = mDirectives; // TODO: should this return true if the #pragma exists in any line before the start token? return std::any_of(directives.cbegin(), directives.cend(), [&](const Directive& d) { return d.linenr < bodyStart->linenr() && d.str == "#pragma pack(1)" && d.file == list.getFiles().front(); diff --git a/lib/tokenize.h b/lib/tokenize.h index 304f66f92eff..7a86b6c8d02c 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -37,6 +37,7 @@ class Token; class TemplateSimplifier; class ErrorLogger; class Preprocessor; +struct Directive; enum class Severity; /// @addtogroup Core @@ -624,6 +625,8 @@ class CPPCHECKLIB Tokenizer { /** Disable assignment operator */ Tokenizer &operator=(const Tokenizer &) = delete; + void setDirectives(std::list directives); + private: const Token *processFunc(const Token *tok2, bool inOperator) const; Token *processFunc(Token *tok2, bool inOperator); @@ -666,6 +669,8 @@ class CPPCHECKLIB Tokenizer { }; std::vector mTypedefInfo; + std::list mDirectives; + /** variable count */ nonneg int mVarId{}; diff --git a/test/helpers.cpp b/test/helpers.cpp index 43117a7a3a8b..e63d00c014e4 100644 --- a/test/helpers.cpp +++ b/test/helpers.cpp @@ -124,7 +124,6 @@ std::string PreprocessorHelper::getcode(Preprocessor &preprocessor, const std::s tokens1.removeComments(); preprocessor.simplifyPragmaAsm(&tokens1); preprocessor.removeComments(); - preprocessor.setDirectives(tokens1); preprocessor.reportOutput(outputList, true); @@ -179,5 +178,6 @@ void PreprocessorHelper::preprocess(Preprocessor &preprocessor, const char code[ // Tokenizer.. tokenizer.list.createTokens(std::move(tokens2)); - preprocessor.setDirectives(tokens1); + std::list directives = preprocessor.createDirectives(tokens1); + tokenizer.setDirectives(std::move(directives)); } diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index 2144d0a98851..825186d9a92e 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -245,10 +245,6 @@ class TestPreprocessor : public TestFixture { TEST_CASE(wrongPathOnErrorDirective); - TEST_CASE(testDirectiveIncludeTypes); - TEST_CASE(testDirectiveIncludeLocations); - TEST_CASE(testDirectiveIncludeComments); - TEST_CASE(testMissingInclude); TEST_CASE(testMissingInclude2); TEST_CASE(testMissingInclude3); @@ -275,7 +271,7 @@ class TestPreprocessor : public TestFixture { tokens.removeComments(); preprocessor0.simplifyPragmaAsm(&tokens); preprocessor0.removeComments(); - preprocessor0.setDirectives(tokens); + std::list directives = preprocessor0.createDirectives(tokens); // TODO preprocessor0.reportOutput(outputList, true); @@ -2313,88 +2309,6 @@ class TestPreprocessor : public TestFixture { ASSERT_EQUALS("[test.c:1]: (error) #error hello world!\n", errout_str()); } - void testDirectiveIncludeTypes() { - const char filedata[] = "#define macro some definition\n" - "#undef macro\n" - "#ifdef macro\n" - "#elif some (complex) condition\n" - "#else\n" - "#endif\n" - "#if some other condition\n" - "#pragma some proprietary content\n" - "#\n" /* may appear in old C code */ - "#ident some text\n" /* may appear in old C code */ - "#unknownmacro some unpredictable text\n" - "#warning some warning message\n" - "#error some error message\n"; - const char dumpdata[] = " \n" - - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n"; - - std::ostringstream ostr; - Preprocessor preprocessor(settings0, this); - PreprocessorHelper::getcode(preprocessor, filedata, "", "test.c"); - preprocessor.dump(ostr); - ASSERT_EQUALS(dumpdata, ostr.str()); - } - - void testDirectiveIncludeLocations() { - const char filedata[] = "#define macro1 val\n" - "#file \"inc1.h\"\n" - "#define macro2 val\n" - "#file \"inc2.h\"\n" - "#define macro3 val\n" - "#endfile\n" - "#define macro4 val\n" - "#endfile\n" - "#define macro5 val\n"; - const char dumpdata[] = " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n"; - - std::ostringstream ostr; - Preprocessor preprocessor(settings0, this); - PreprocessorHelper::getcode(preprocessor, filedata, "", "test.c"); - preprocessor.dump(ostr); - ASSERT_EQUALS(dumpdata, ostr.str()); - } - - void testDirectiveIncludeComments() { - const char filedata[] = "#ifdef macro2 /* this will be removed */\n" - "#else /* this will be removed too */\n" - "#endif /* this will also be removed */\n"; - const char dumpdata[] = " \n" - " \n" - " \n" - " \n" - " \n"; - - std::ostringstream ostr; - Preprocessor preprocessor(settings0, this); - PreprocessorHelper::getcode(preprocessor, filedata, "", "test.c"); - preprocessor.dump(ostr); - ASSERT_EQUALS(dumpdata, ostr.str()); - } - // test for existing local include void testMissingInclude() { /*const*/ Settings settings; diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 81c850b36136..eab7901084b8 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -449,6 +449,10 @@ class TestTokenizer : public TestFixture { TEST_CASE(cpp20_default_bitfield_initializer); TEST_CASE(cpp11init); + + TEST_CASE(testDirectiveIncludeTypes); + TEST_CASE(testDirectiveIncludeLocations); + TEST_CASE(testDirectiveIncludeComments); } #define tokenizeAndStringify(...) tokenizeAndStringify_(__FILE__, __LINE__, __VA_ARGS__) @@ -497,6 +501,21 @@ class TestTokenizer : public TestFixture { return tokenizer.tokens()->stringifyList(true,true,true,true,false); } + void directiveDump(const char filedata[], std::ostream& ostr) { + Preprocessor preprocessor(settingsDefault, this); + std::istringstream istr(filedata); + simplecpp::OutputList outputList; + std::vector files; + simplecpp::TokenList tokens1(istr, files, "test.c", &outputList); + std::list directives = preprocessor.createDirectives(tokens1); + + const Settings s = settingsBuilder().severity(Severity::information).build(); + Tokenizer tokenizer(s, this); + tokenizer.setDirectives(std::move(directives)); + + tokenizer.dump(ostr); + } + void tokenize1() { const char code[] = "void f ( )\n" "{ if ( p . y ( ) > yof ) { } }"; @@ -7895,6 +7914,88 @@ class TestTokenizer : public TestFixture { ASSERT_EQUALS("[test.cpp:2]: (debug) auto token with no type.\n", errout_str()); #undef testIsCpp11init } + + void testDirectiveIncludeTypes() { + const char filedata[] = "#define macro some definition\n" + "#undef macro\n" + "#ifdef macro\n" + "#elif some (complex) condition\n" + "#else\n" + "#endif\n" + "#if some other condition\n" + "#pragma some proprietary content\n" + "#\n" /* may appear in old C code */ + "#ident some text\n" /* may appear in old C code */ + "#unknownmacro some unpredictable text\n" + "#warning some warning message\n" + "#error some error message\n"; + const char dumpdata[] = " \n" + + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n"; + + std::ostringstream ostr; + directiveDump(filedata, ostr); + ASSERT_EQUALS(dumpdata, ostr.str()); + } + + void testDirectiveIncludeLocations() { + const char filedata[] = "#define macro1 val\n" + "#file \"inc1.h\"\n" + "#define macro2 val\n" + "#file \"inc2.h\"\n" + "#define macro3 val\n" + "#endfile\n" + "#define macro4 val\n" + "#endfile\n" + "#define macro5 val\n"; + const char dumpdata[] = " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n"; + + std::ostringstream ostr; + directiveDump(filedata, ostr); + ASSERT_EQUALS(dumpdata, ostr.str()); + } + + void testDirectiveIncludeComments() { + const char filedata[] = "#ifdef macro2 /* this will be removed */\n" + "#else /* this will be removed too */\n" + "#endif /* this will also be removed */\n"; + const char dumpdata[] = " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n"; + + std::ostringstream ostr; + directiveDump(filedata, ostr); + ASSERT_EQUALS(dumpdata, ostr.str()); + } }; REGISTER_TEST(TestTokenizer) diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 24639cb97bec..ba87fe9e1602 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -261,13 +261,13 @@ class TestUnusedVar : public TestFixture { #define checkStructMemberUsage(...) checkStructMemberUsage_(__FILE__, __LINE__, __VA_ARGS__) void checkStructMemberUsage_(const char* file, int line, const char code[], const std::list* directives = nullptr, const Settings *s = nullptr) { Preprocessor preprocessor(settings); - if (directives) - preprocessor.setDirectives(*directives); const Settings *settings1 = s ? s : &settings; // Tokenize.. SimpleTokenizer tokenizer(*settings1, *this, &preprocessor); + if (directives) + tokenizer.setDirectives(*directives); ASSERT_LOC(tokenizer.tokenize(code), file, line); // Check for unused variables..