From 459c01d52099e73d3487b5270b2f85db9179729b Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 22 Jul 2024 20:08:54 +0400 Subject: [PATCH 1/5] cmListFileCache: move cmListFileParser into an anonymous namespace Avoid the `cmListFileParser` methods intermixed with the `cmListFile` methods. --- Source/cmListFileCache.cxx | 114 +++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 56 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index 97f5de9697..c13542d9de 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -17,6 +17,30 @@ #include "cmMessenger.h" #include "cmSystemTools.h" +namespace { + +enum class NestingStateEnum +{ + If, + Else, + While, + Foreach, + Function, + Macro, + Block +}; + +struct NestingState +{ + NestingStateEnum State; + cmListFileContext Context; +}; + +bool TopIs(std::vector& stack, NestingStateEnum state) +{ + return !stack.empty() && stack.back().State == state; +} + struct cmListFileParser { cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt, @@ -181,38 +205,6 @@ bool cmListFileParser::Parse() return true; } -bool cmListFile::ParseFile(const char* filename, cmMessenger* messenger, - cmListFileBacktrace const& lfbt) -{ - if (!cmSystemTools::FileExists(filename) || - cmSystemTools::FileIsDirectory(filename)) { - return false; - } - - bool parseError = false; - - { - cmListFileParser parser(this, lfbt, messenger); - parseError = !parser.ParseFile(filename); - } - - return !parseError; -} - -bool cmListFile::ParseString(const char* str, const char* virtual_filename, - cmMessenger* messenger, - const cmListFileBacktrace& lfbt) -{ - bool parseError = false; - - { - cmListFileParser parser(this, lfbt, messenger); - parseError = !parser.ParseString(str, virtual_filename); - } - - return !parseError; -} - bool cmListFileParser::ParseFunction(const char* name, long line) { // Ininitialize a new function call. @@ -338,30 +330,6 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token, return true; } -namespace { -enum class NestingStateEnum -{ - If, - Else, - While, - Foreach, - Function, - Macro, - Block -}; - -struct NestingState -{ - NestingStateEnum State; - cmListFileContext Context; -}; - -bool TopIs(std::vector& stack, NestingStateEnum state) -{ - return !stack.empty() && stack.back().State == state; -} -} - cm::optional cmListFileParser::CheckNesting() const { std::vector stack; @@ -455,6 +423,40 @@ cm::optional cmListFileParser::CheckNesting() const return cm::nullopt; } +} // anonymous namespace + +bool cmListFile::ParseFile(const char* filename, cmMessenger* messenger, + cmListFileBacktrace const& lfbt) +{ + if (!cmSystemTools::FileExists(filename) || + cmSystemTools::FileIsDirectory(filename)) { + return false; + } + + bool parseError = false; + + { + cmListFileParser parser(this, lfbt, messenger); + parseError = !parser.ParseFile(filename); + } + + return !parseError; +} + +bool cmListFile::ParseString(const char* str, const char* virtual_filename, + cmMessenger* messenger, + const cmListFileBacktrace& lfbt) +{ + bool parseError = false; + + { + cmListFileParser parser(this, lfbt, messenger); + parseError = !parser.ParseString(str, virtual_filename); + } + + return !parseError; +} + #include "cmConstStack.tcc" template class cmConstStack; From 1bf4900df7e3ccbf755a278d14f046de4c50cea2 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 22 Jul 2024 20:15:17 +0400 Subject: [PATCH 2/5] cmListFileCache: avoid redundant operator<< calls Also, remove unneeded `clang-format off` comments. --- Source/cmListFileCache.cxx | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index c13542d9de..a88cd0970f 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -217,12 +217,8 @@ bool cmListFileParser::ParseFunction(const char* name, long line) token->type == cmListFileLexer_Token_Space) { } if (!token) { - std::ostringstream error; - /* clang-format off */ - error << "Unexpected end of file.\n" - << "Parse error. Function missing opening \"(\"."; - /* clang-format on */ - this->IssueError(error.str()); + this->IssueError("Unexpected end of file.\n" + "Parse error. Function missing opening \"(\"."); return false; } if (token->type != cmListFileLexer_Token_ParenLeft) { @@ -282,7 +278,7 @@ bool cmListFileParser::ParseFunction(const char* name, long line) // Error. std::ostringstream error; error << "Parse error. Function missing ending \")\". " - << "Instead found " + "Instead found " << cmListFileLexer_GetTypeAsString(this->Lexer, token->type) << " with text \"" << token->text << "\"."; this->IssueError(error.str()); @@ -290,15 +286,16 @@ bool cmListFileParser::ParseFunction(const char* name, long line) } } - std::ostringstream error; cmListFileContext lfc; lfc.FilePath = this->FileName; lfc.Line = line; cmListFileBacktrace lfbt = this->Backtrace; lfbt = lfbt.Push(lfc); - error << "Parse error. Function missing ending \")\". " - << "End of file reached."; - this->Messenger->IssueMessage(MessageType::FATAL_ERROR, error.str(), lfbt); + this->Messenger->IssueMessage( + MessageType::FATAL_ERROR, + "Parse error. Function missing ending \")\". " + "End of file reached.", + lfbt); return false; } @@ -317,11 +314,10 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token, lfc.Line = token->line; cmListFileBacktrace lfbt = this->Backtrace; lfbt = lfbt.Push(lfc); - - m << "Syntax " << (isError ? "Error" : "Warning") << " in cmake code at " - << "column " << token->column << "\n" - << "Argument not separated from preceding token by whitespace."; - /* clang-format on */ + m << "Syntax " << (isError ? "Error" : "Warning") + << " in cmake code at column " << token->column + << "\n" + "Argument not separated from preceding token by whitespace."; if (isError) { this->Messenger->IssueMessage(MessageType::FATAL_ERROR, m.str(), lfbt); return false; @@ -464,9 +460,9 @@ std::ostream& operator<<(std::ostream& os, cmListFileContext const& lfc) { os << lfc.FilePath; if (lfc.Line > 0) { - os << ":" << lfc.Line; + os << ':' << lfc.Line; if (!lfc.Name.empty()) { - os << " (" << lfc.Name << ")"; + os << " (" << lfc.Name << ')'; } } else if (lfc.Line == cmListFileContext::DeferPlaceholderLine) { os << ":DEFERRED"; From 63f8134744505373c4c92b18f6177ad12ac45ea7 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 22 Jul 2024 21:07:20 +0400 Subject: [PATCH 3/5] cmListFileCache: convert cmListFileParser from struct to class --- Source/cmListFileCache.cxx | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index a88cd0970f..a14549b6cf 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -41,22 +41,35 @@ bool TopIs(std::vector& stack, NestingStateEnum state) return !stack.empty() && stack.back().State == state; } -struct cmListFileParser +class cmListFileParser { +public: cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt, cmMessenger* messenger); ~cmListFileParser(); cmListFileParser(const cmListFileParser&) = delete; cmListFileParser& operator=(const cmListFileParser&) = delete; - void IssueFileOpenError(std::string const& text) const; - void IssueError(std::string const& text) const; + bool ParseFile(const char* filename); bool ParseString(const char* str, const char* virtual_filename); + +private: bool Parse(); bool ParseFunction(const char* name, long line); bool AddArgument(cmListFileLexer_Token* token, cmListFileArgument::Delimiter delim); + void IssueFileOpenError(std::string const& text) const; + void IssueError(std::string const& text) const; + cm::optional CheckNesting() const; + + enum + { + SeparationOkay, + SeparationWarning, + SeparationError + } Separation; + cmListFile* ListFile; cmListFileBacktrace Backtrace; cmMessenger* Messenger; @@ -66,12 +79,6 @@ struct cmListFileParser long FunctionLine; long FunctionLineEnd; std::vector FunctionArguments; - enum - { - SeparationOkay, - SeparationWarning, - SeparationError - } Separation; }; cmListFileParser::cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt, From 55a4a585fa7bb1985d21b1c792cc1506a44758df Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 22 Jul 2024 21:22:34 +0400 Subject: [PATCH 4/5] cmListFileParser: use unique_ptr to own cmListFileLexer instance --- Source/cmListFileCache.cxx | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index a14549b6cf..4425845598 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -46,7 +46,6 @@ class cmListFileParser public: cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt, cmMessenger* messenger); - ~cmListFileParser(); cmListFileParser(const cmListFileParser&) = delete; cmListFileParser& operator=(const cmListFileParser&) = delete; @@ -74,7 +73,7 @@ private: cmListFileBacktrace Backtrace; cmMessenger* Messenger; const char* FileName = nullptr; - cmListFileLexer* Lexer; + std::unique_ptr Lexer; std::string FunctionName; long FunctionLine; long FunctionLineEnd; @@ -86,15 +85,10 @@ cmListFileParser::cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt, : ListFile(lf) , Backtrace(std::move(lfbt)) , Messenger(messenger) - , Lexer(cmListFileLexer_New()) + , Lexer(cmListFileLexer_New(), cmListFileLexer_Delete) { } -cmListFileParser::~cmListFileParser() -{ - cmListFileLexer_Delete(this->Lexer); -} - void cmListFileParser::IssueFileOpenError(const std::string& text) const { this->Messenger->IssueMessage(MessageType::FATAL_ERROR, text, @@ -105,7 +99,7 @@ void cmListFileParser::IssueError(const std::string& text) const { cmListFileContext lfc; lfc.FilePath = this->FileName; - lfc.Line = cmListFileLexer_GetCurrentLine(this->Lexer); + lfc.Line = cmListFileLexer_GetCurrentLine(this->Lexer.get()); cmListFileBacktrace lfbt = this->Backtrace; lfbt = lfbt.Push(lfc); this->Messenger->IssueMessage(MessageType::FATAL_ERROR, text, lfbt); @@ -124,13 +118,13 @@ bool cmListFileParser::ParseFile(const char* filename) // Open the file. cmListFileLexer_BOM bom; - if (!cmListFileLexer_SetFileName(this->Lexer, filename, &bom)) { + if (!cmListFileLexer_SetFileName(this->Lexer.get(), filename, &bom)) { this->IssueFileOpenError("cmListFileCache: error can not open file."); return false; } if (bom == cmListFileLexer_BOM_Broken) { - cmListFileLexer_SetFileName(this->Lexer, nullptr, nullptr); + cmListFileLexer_SetFileName(this->Lexer.get(), nullptr, nullptr); this->IssueFileOpenError("Error while reading Byte-Order-Mark. " "File not seekable?"); return false; @@ -138,7 +132,7 @@ bool cmListFileParser::ParseFile(const char* filename) // Verify the Byte-Order-Mark, if any. if (bom != cmListFileLexer_BOM_None && bom != cmListFileLexer_BOM_UTF8) { - cmListFileLexer_SetFileName(this->Lexer, nullptr, nullptr); + cmListFileLexer_SetFileName(this->Lexer.get(), nullptr, nullptr); this->IssueFileOpenError( "File starts with a Byte-Order-Mark that is not UTF-8."); return false; @@ -152,7 +146,7 @@ bool cmListFileParser::ParseString(const char* str, { this->FileName = virtual_filename; - if (!cmListFileLexer_SetString(this->Lexer, str)) { + if (!cmListFileLexer_SetString(this->Lexer.get(), str)) { this->IssueFileOpenError("cmListFileCache: cannot allocate buffer."); return false; } @@ -165,7 +159,8 @@ bool cmListFileParser::Parse() // Use a simple recursive-descent parser to process the token // stream. bool haveNewline = true; - while (cmListFileLexer_Token* token = cmListFileLexer_Scan(this->Lexer)) { + while (cmListFileLexer_Token* token = + cmListFileLexer_Scan(this->Lexer.get())) { if (token->type == cmListFileLexer_Token_Space) { } else if (token->type == cmListFileLexer_Token_Newline) { haveNewline = true; @@ -184,7 +179,8 @@ bool cmListFileParser::Parse() } else { std::ostringstream error; error << "Parse error. Expected a newline, got " - << cmListFileLexer_GetTypeAsString(this->Lexer, token->type) + << cmListFileLexer_GetTypeAsString(this->Lexer.get(), + token->type) << " with text \"" << token->text << "\"."; this->IssueError(error.str()); return false; @@ -192,7 +188,7 @@ bool cmListFileParser::Parse() } else { std::ostringstream error; error << "Parse error. Expected a command name, got " - << cmListFileLexer_GetTypeAsString(this->Lexer, token->type) + << cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type) << " with text \"" << token->text << "\"."; this->IssueError(error.str()); return false; @@ -220,7 +216,7 @@ bool cmListFileParser::ParseFunction(const char* name, long line) // Command name has already been parsed. Read the left paren. cmListFileLexer_Token* token; - while ((token = cmListFileLexer_Scan(this->Lexer)) && + while ((token = cmListFileLexer_Scan(this->Lexer.get())) && token->type == cmListFileLexer_Token_Space) { } if (!token) { @@ -231,7 +227,7 @@ bool cmListFileParser::ParseFunction(const char* name, long line) if (token->type != cmListFileLexer_Token_ParenLeft) { std::ostringstream error; error << "Parse error. Expected \"(\", got " - << cmListFileLexer_GetTypeAsString(this->Lexer, token->type) + << cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type) << " with text \"" << token->text << "\"."; this->IssueError(error.str()); return false; @@ -240,7 +236,7 @@ bool cmListFileParser::ParseFunction(const char* name, long line) // Arguments. unsigned long parenDepth = 0; this->Separation = SeparationOkay; - while ((token = cmListFileLexer_Scan(this->Lexer))) { + while ((token = cmListFileLexer_Scan(this->Lexer.get()))) { if (token->type == cmListFileLexer_Token_Space || token->type == cmListFileLexer_Token_Newline) { this->Separation = SeparationOkay; @@ -286,7 +282,7 @@ bool cmListFileParser::ParseFunction(const char* name, long line) std::ostringstream error; error << "Parse error. Function missing ending \")\". " "Instead found " - << cmListFileLexer_GetTypeAsString(this->Lexer, token->type) + << cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type) << " with text \"" << token->text << "\"."; this->IssueError(error.str()); return false; From e947e7b6e23fe740d85b1843fb459d2eab6820aa Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 22 Jul 2024 21:39:19 +0400 Subject: [PATCH 5/5] cmListFileCache: use cmStrCat instead of string stream --- Source/cmListFileCache.cxx | 60 +++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index 4425845598..cf869c873c 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -4,7 +4,7 @@ #include "cmListFileCache.h" #include -#include +#include #include #ifdef _WIN32 @@ -15,6 +15,7 @@ #include "cmListFileLexer.h" #include "cmMessageType.h" #include "cmMessenger.h" +#include "cmStringAlgorithms.h" #include "cmSystemTools.h" namespace { @@ -177,20 +178,19 @@ bool cmListFileParser::Parse() return false; } } else { - std::ostringstream error; - error << "Parse error. Expected a newline, got " - << cmListFileLexer_GetTypeAsString(this->Lexer.get(), - token->type) - << " with text \"" << token->text << "\"."; - this->IssueError(error.str()); + auto error = cmStrCat( + "Parse error. Expected a newline, got ", + cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type), + " with text \"", token->text, "\"."); + this->IssueError(error); return false; } } else { - std::ostringstream error; - error << "Parse error. Expected a command name, got " - << cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type) - << " with text \"" << token->text << "\"."; - this->IssueError(error.str()); + auto error = cmStrCat( + "Parse error. Expected a command name, got ", + cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type), + " with text \"", token->text, "\"."); + this->IssueError(error); return false; } } @@ -225,11 +225,11 @@ bool cmListFileParser::ParseFunction(const char* name, long line) return false; } if (token->type != cmListFileLexer_Token_ParenLeft) { - std::ostringstream error; - error << "Parse error. Expected \"(\", got " - << cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type) - << " with text \"" << token->text << "\"."; - this->IssueError(error.str()); + auto error = + cmStrCat("Parse error. Expected \"(\", got ", + cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type), + " with text \"", token->text, "\"."); + this->IssueError(error); return false; } @@ -279,12 +279,12 @@ bool cmListFileParser::ParseFunction(const char* name, long line) this->Separation = SeparationError; } else { // Error. - std::ostringstream error; - error << "Parse error. Function missing ending \")\". " - "Instead found " - << cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type) - << " with text \"" << token->text << "\"."; - this->IssueError(error.str()); + auto error = cmStrCat( + "Parse error. Function missing ending \")\". " + "Instead found ", + cmListFileLexer_GetTypeAsString(this->Lexer.get(), token->type), + " with text \"", token->text, "\"."); + this->IssueError(error); return false; } } @@ -311,21 +311,21 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token, } bool isError = (this->Separation == SeparationError || delim == cmListFileArgument::Bracket); - std::ostringstream m; cmListFileContext lfc; lfc.FilePath = this->FileName; lfc.Line = token->line; cmListFileBacktrace lfbt = this->Backtrace; lfbt = lfbt.Push(lfc); - m << "Syntax " << (isError ? "Error" : "Warning") - << " in cmake code at column " << token->column - << "\n" - "Argument not separated from preceding token by whitespace."; + auto msg = + cmStrCat("Syntax ", (isError ? "Error" : "Warning"), + " in cmake code at column ", token->column, + "\n" + "Argument not separated from preceding token by whitespace."); if (isError) { - this->Messenger->IssueMessage(MessageType::FATAL_ERROR, m.str(), lfbt); + this->Messenger->IssueMessage(MessageType::FATAL_ERROR, msg, lfbt); return false; } - this->Messenger->IssueMessage(MessageType::AUTHOR_WARNING, m.str(), lfbt); + this->Messenger->IssueMessage(MessageType::AUTHOR_WARNING, msg, lfbt); return true; }