From b9a5a2b494e4140471e62bbfebe0f2eb95caffdb Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 00:37:36 +0400 Subject: [PATCH 01/13] cmMakefile: Replace ternary with bool => int cast --- Source/cmMakefile.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index f4e0f3b39a..b6fb5e2dd4 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -370,9 +370,9 @@ void cmMakefile::PrintCommandTrace(cmListFileFunction const& lff, val["args"].append(arg); } val["time"] = cmSystemTools::GetTime(); - val["frame"] = (missing == CommandMissingFromStack::Yes ? 1 : 0) + + val["frame"] = int(missing == CommandMissingFromStack::Yes) + static_cast(this->ExecutionStatusStack.size()); - val["global_frame"] = (missing == CommandMissingFromStack::Yes ? 1 : 0) + + val["global_frame"] = int(missing == CommandMissingFromStack::Yes) + static_cast(this->RecursionDepth); msg << Json::writeString(builder, val); #endif From 33986a1c60b0446d6a10160ce1c04480176ff322 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 00:38:17 +0400 Subject: [PATCH 02/13] cmMakefile: Remove redundant << stream operators --- Source/cmMakefile.cxx | 181 +++++++++++++++++++++--------------------- 1 file changed, 92 insertions(+), 89 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index b6fb5e2dd4..0d46cf14ff 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -204,7 +204,7 @@ bool cmMakefile::CheckCMP0037(std::string const& targetName, switch (this->GetPolicyStatus(cmPolicies::CMP0037)) { case cmPolicies::WARN: if (targetType != cmStateEnums::INTERFACE_LIBRARY) { - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0037) << "\n"; + e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0037) << '\n'; issueMessage = true; } CM_FALLTHROUGH; @@ -237,14 +237,18 @@ void cmMakefile::MaybeWarnCMP0074(std::string const& rootVar, cmValue rootDef, // Warn if a _ROOT variable we may use is set. if ((rootDef || rootEnv) && this->WarnedCMP0074.insert(rootVar).second) { std::ostringstream w; - w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0074) << "\n"; + w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0074) << '\n'; if (rootDef) { - w << "CMake variable " << rootVar << " is set to:\n" - << " " << *rootDef << "\n"; + w << "CMake variable " << rootVar + << " is set to:\n" + " " + << *rootDef << '\n'; } if (rootEnv) { - w << "Environment variable " << rootVar << " is set to:\n" - << " " << *rootEnv << "\n"; + w << "Environment variable " << rootVar + << " is set to:\n" + " " + << *rootEnv << '\n'; } w << "For compatibility, CMake is ignoring the variable."; this->IssueMessage(MessageType::AUTHOR_WARNING, w.str()); @@ -257,14 +261,18 @@ void cmMakefile::MaybeWarnCMP0144(std::string const& rootVAR, cmValue rootDEF, // Warn if a _ROOT variable we may use is set. if ((rootDEF || rootENV) && this->WarnedCMP0144.insert(rootVAR).second) { std::ostringstream w; - w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0144) << "\n"; + w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0144) << '\n'; if (rootDEF) { - w << "CMake variable " << rootVAR << " is set to:\n" - << " " << *rootDEF << "\n"; + w << "CMake variable " << rootVAR + << " is set to:\n" + " " + << *rootDEF << '\n'; } if (rootENV) { - w << "Environment variable " << rootVAR << " is set to:\n" - << " " << *rootENV << "\n"; + w << "Environment variable " << rootVAR + << " is set to:\n" + " " + << *rootENV << '\n'; } w << "For compatibility, find_package is ignoring the variable, but " "code in a .cmake module might still use it."; @@ -379,16 +387,16 @@ void cmMakefile::PrintCommandTrace(cmListFileFunction const& lff, break; } case cmake::TraceFormat::Human: - msg << full_path << "(" << lff.Line() << "):"; + msg << full_path << '(' << lff.Line() << "):"; if (deferId) { - msg << "DEFERRED:" << *deferId << ":"; + msg << "DEFERRED:" << *deferId << ':'; } - msg << " " << lff.OriginalName() << "("; + msg << " " << lff.OriginalName() << '('; for (std::string const& arg : args) { - msg << arg << " "; + msg << arg << ' '; } - msg << ")"; + msg << ')'; break; case cmake::TraceFormat::Undefined: msg << "INTERNAL ERROR: Trace format is Undefined"; @@ -653,12 +661,14 @@ void cmMakefile::IncludeScope::EnforceCMP0011() // Warn because the user did not set this policy. { std::ostringstream w; + /* clang-format off */ w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0011) << "\n" - << "The included script\n " + "The included script\n " << this->Makefile->GetBacktrace().Top().FilePath << "\n" - << "affects policy settings. " - << "CMake is implying the NO_POLICY_SCOPE option for compatibility, " - << "so the effects are applied to the including context."; + "affects policy settings. " + "CMake is implying the NO_POLICY_SCOPE option for compatibility, " + "so the effects are applied to the including context."; + /* clang-format on */ this->Makefile->IssueMessage(MessageType::AUTHOR_WARNING, w.str()); } break; @@ -667,9 +677,9 @@ void cmMakefile::IncludeScope::EnforceCMP0011() std::ostringstream e; /* clang-format off */ e << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0011) << "\n" - << "The included script\n " + "The included script\n " << this->Makefile->GetBacktrace().Top().FilePath << "\n" - << "affects policy settings, so it requires this policy to be set."; + "affects policy settings, so it requires this policy to be set."; /* clang-format on */ this->Makefile->IssueMessage(MessageType::FATAL_ERROR, e.str()); } break; @@ -957,15 +967,17 @@ void cmMakefile::EnforceDirectoryLevelRules() const // Diagnose a violation of CMP0000 if necessary. if (this->CheckCMP0000) { std::ostringstream msg; + /* clang-format off */ msg << "No cmake_minimum_required command is present. " - << "A line of code such as\n" - << " cmake_minimum_required(VERSION " << cmVersion::GetMajorVersion() + "A line of code such as\n" + " cmake_minimum_required(VERSION " << cmVersion::GetMajorVersion() << "." << cmVersion::GetMinorVersion() << ")\n" - << "should be added at the top of the file. " - << "The version specified may be lower if you wish to " - << "support older CMake versions for this project. " - << "For more information run " - << "\"cmake --help-policy CMP0000\"."; + "should be added at the top of the file. " + "The version specified may be lower if you wish to " + "support older CMake versions for this project. " + "For more information run " + "\"cmake --help-policy CMP0000\"."; + /* clang-format on */ switch (this->GetPolicyStatus(cmPolicies::CMP0000)) { case cmPolicies::WARN: // Warn because the user did not provide a minimum required @@ -1131,7 +1143,7 @@ bool cmMakefile::ValidateCustomCommand( for (cmCustomCommandLine const& cl : commandLines) { if (!cl.empty() && !cl[0].empty() && cl[0][0] == '"') { std::ostringstream e; - e << "COMMAND may not contain literal quotes:\n " << cl[0] << "\n"; + e << "COMMAND may not contain literal quotes:\n " << cl[0] << '\n'; this->IssueMessage(MessageType::FATAL_ERROR, e.str()); return false; } @@ -1159,7 +1171,7 @@ cmTarget* cmMakefile::GetCustomCommandTarget( std::ostringstream e; switch (this->GetPolicyStatus(cmPolicies::CMP0040)) { case cmPolicies::WARN: - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0040) << "\n"; + e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0040) << '\n'; issueMessage = true; CM_FALLTHROUGH; case cmPolicies::OLD: @@ -1891,19 +1903,18 @@ void cmMakefile::ConfigureSubDirectory(cmMakefile* mf) // The file is missing. Check policy CMP0014. std::ostringstream e; /* clang-format off */ - e << "The source directory\n" - << " " << currentStart << "\n" - << "does not contain a CMakeLists.txt file."; + e << "The source directory\n " << currentStart << "\n" + "does not contain a CMakeLists.txt file."; /* clang-format on */ switch (this->GetPolicyStatus(cmPolicies::CMP0014)) { case cmPolicies::WARN: // Print the warning. /* clang-format off */ e << "\n" - << "CMake does not support this case but it used " - << "to work accidentally and is being allowed for " - << "compatibility." - << "\n" + "CMake does not support this case but it used " + "to work accidentally and is being allowed for " + "compatibility." + "\n" << cmPolicies::GetPolicyWarning(cmPolicies::CMP0014); /* clang-format on */ this->IssueMessage(MessageType::AUTHOR_WARNING, e.str()); @@ -1913,7 +1924,7 @@ void cmMakefile::ConfigureSubDirectory(cmMakefile* mf) break; case cmPolicies::REQUIRED_IF_USED: case cmPolicies::REQUIRED_ALWAYS: - e << "\n" << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0014); + e << '\n' << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0014); CM_FALLTHROUGH; case cmPolicies::NEW: // NEW behavior prints the error. @@ -2130,7 +2141,7 @@ void cmMakefile::MaybeWarnUninitialized(std::string const& variable, if (this->CheckSystemVars || (sourceFilename && this->IsProjectFile(sourceFilename))) { std::ostringstream msg; - msg << "uninitialized variable \'" << variable << "\'"; + msg << "uninitialized variable \'" << variable << '\''; this->IssueMessage(MessageType::AUTHOR_WARNING, msg.str()); } } @@ -2436,9 +2447,9 @@ void cmMakefile::ExpandVariablesCMP0019() if (pol == cmPolicies::WARN && dirs != *includeDirs) { /* clang-format off */ w << "Evaluated directory INCLUDE_DIRECTORIES\n" - << " " << *includeDirs << "\n" - << "as\n" - << " " << dirs << "\n"; + " " << *includeDirs << "\n" + "as\n" + " " << dirs << '\n'; /* clang-format on */ } this->SetProperty("INCLUDE_DIRECTORIES", dirs); @@ -2458,9 +2469,9 @@ void cmMakefile::ExpandVariablesCMP0019() if (pol == cmPolicies::WARN && dirs != *includeDirs) { /* clang-format off */ w << "Evaluated target " << t.GetName() << " INCLUDE_DIRECTORIES\n" - << " " << *includeDirs << "\n" - << "as\n" - << " " << dirs << "\n"; + " " << *includeDirs << "\n" + "as\n" + " " << dirs << '\n'; /* clang-format on */ } t.SetProperty("INCLUDE_DIRECTORIES", dirs); @@ -2475,9 +2486,9 @@ void cmMakefile::ExpandVariablesCMP0019() if (pol == cmPolicies::WARN && d != orig) { /* clang-format off */ w << "Evaluated link directories\n" - << " " << orig << "\n" - << "as\n" - << " " << d << "\n"; + " " << orig << "\n" + "as\n" + " " << d << '\n'; /* clang-format on */ } } @@ -2498,9 +2509,9 @@ void cmMakefile::ExpandVariablesCMP0019() if (pol == cmPolicies::WARN && libName != orig) { /* clang-format off */ w << "Evaluated link library\n" - << " " << orig << "\n" - << "as\n" - << " " << libName << "\n"; + " " << orig << "\n" + "as\n" + " " << libName << '\n'; /* clang-format on */ } } @@ -2512,7 +2523,7 @@ void cmMakefile::ExpandVariablesCMP0019() /* clang-format off */ m << cmPolicies::GetPolicyWarning(cmPolicies::CMP0019) << "\n" - << "The following variable evaluations were encountered:\n" + "The following variable evaluations were encountered:\n" << w.str(); /* clang-format on */ this->GetCMakeInstance()->IssueMessage(MessageType::AUTHOR_WARNING, @@ -2930,12 +2941,9 @@ MessageType cmMakefile::ExpandVariablesInStringOld( // This filename and line number may be more specific than the // command context because one command invocation can have // arguments on multiple lines. - error << "at\n" - << " " << filename << ":" << line << "\n"; + error << "at\n " << filename << ':' << line << '\n'; } - error << "when parsing string\n" - << " " << source << "\n"; - error << emsg; + error << "when parsing string\n " << source << '\n' << emsg; // If the parser failed ("res" is false) then this is a real // argument parsing error, so the policy applies. Otherwise the @@ -2948,7 +2956,7 @@ MessageType cmMakefile::ExpandVariablesInStringOld( // decide whether it is an error. switch (this->GetPolicyStatus(cmPolicies::CMP0010)) { case cmPolicies::WARN: - error << "\n" << cmPolicies::GetPolicyWarning(cmPolicies::CMP0010); + error << '\n' << cmPolicies::GetPolicyWarning(cmPolicies::CMP0010); CM_FALLTHROUGH; case cmPolicies::OLD: // OLD behavior is to just warn and continue. @@ -2956,7 +2964,7 @@ MessageType cmMakefile::ExpandVariablesInStringOld( break; case cmPolicies::REQUIRED_IF_USED: case cmPolicies::REQUIRED_ALWAYS: - error << "\n" + error << '\n' << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0010); break; case cmPolicies::NEW: @@ -3311,12 +3319,9 @@ MessageType cmMakefile::ExpandVariablesInStringNew( // This filename and line number may be more specific than the // command context because one command invocation can have // arguments on multiple lines. - emsg << "at\n" - << " " << filename << ":" << line << "\n"; + emsg << "at\n " << filename << ':' << line << '\n'; } - emsg << "when parsing string\n" - << " " << source << "\n"; - emsg << errorstr; + emsg << "when parsing string\n " << source << '\n' << errorstr; mtype = MessageType::FATAL_ERROR; errorstr = emsg.str(); } else { @@ -3426,8 +3431,8 @@ void cmMakefile::PopFunctionBlockerBarrier(bool reportError) std::ostringstream e; /* clang-format off */ e << "A logical block opening on the line\n" - << " " << lfc << "\n" - << "is not closed."; + " " << lfc << "\n" + "is not closed."; /* clang-format on */ this->IssueMessage(MessageType::FATAL_ERROR, e.str()); reportError = false; @@ -4309,10 +4314,7 @@ std::string cmMakefile::FormatListFileStack() const tmp << "\n "; } --it; - tmp << "["; - tmp << depth; - tmp << "]\t"; - tmp << *it; + tmp << '[' << depth << "]\t" << *it; depth--; } while (it != listFiles.begin()); } @@ -4486,7 +4488,7 @@ bool cmMakefile::EnforceUniqueName(std::string const& name, std::string& msg, std::ostringstream e; e << "cannot create target \"" << name << "\" because another target with the same name already exists. " - << "The existing target is "; + "The existing target is "; switch (existing->GetType()) { case cmStateEnums::EXECUTABLE: e << "an executable "; @@ -4510,8 +4512,9 @@ bool cmMakefile::EnforceUniqueName(std::string const& name, std::string& msg, break; } e << "created in source directory \"" - << existing->GetMakefile()->GetCurrentSourceDirectory() << "\". " - << "See documentation for policy CMP0002 for more details."; + << existing->GetMakefile()->GetCurrentSourceDirectory() + << "\". " + "See documentation for policy CMP0002 for more details."; msg = e.str(); return false; } @@ -4533,15 +4536,15 @@ bool cmMakefile::EnforceUniqueDir(const std::string& srcPath, /* clang-format off */ e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0013) << "\n" - << "The binary directory\n" - << " " << binPath << "\n" - << "is already used to build a source directory. " - << "This command uses it to build source directory\n" - << " " << srcPath << "\n" - << "which can generate conflicting build files. " - << "CMake does not support this use case but it used " - << "to work accidentally and is being allowed for " - << "compatibility."; + "The binary directory\n" + " " << binPath << "\n" + "is already used to build a source directory. " + "This command uses it to build source directory\n" + " " << srcPath << "\n" + "which can generate conflicting build files. " + "CMake does not support this use case but it used " + "to work accidentally and is being allowed for " + "compatibility."; /* clang-format on */ this->IssueMessage(MessageType::AUTHOR_WARNING, e.str()); CM_FALLTHROUGH; @@ -4550,17 +4553,17 @@ bool cmMakefile::EnforceUniqueDir(const std::string& srcPath, return true; case cmPolicies::REQUIRED_IF_USED: case cmPolicies::REQUIRED_ALWAYS: - e << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0013) << "\n"; + e << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0013) << '\n'; CM_FALLTHROUGH; case cmPolicies::NEW: // NEW behavior prints the error. /* clang-format off */ e << "The binary directory\n" - << " " << binPath << "\n" - << "is already used to build a source directory. " - << "It cannot be used to build source directory\n" - << " " << srcPath << "\n" - << "Specify a unique binary directory name."; + " " << binPath << "\n" + "is already used to build a source directory. " + "It cannot be used to build source directory\n" + " " << srcPath << "\n" + "Specify a unique binary directory name."; /* clang-format on */ this->IssueMessage(MessageType::FATAL_ERROR, e.str()); break; From 1df30b6bf29f117080b71363b704d528015e4dd8 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 01:15:51 +0400 Subject: [PATCH 03/13] cmMakefile: Use cmStrCat instead of std::stringstream --- Source/cmMakefile.cxx | 429 +++++++++++++++++++----------------------- 1 file changed, 193 insertions(+), 236 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 0d46cf14ff..a117849773 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -199,12 +199,12 @@ bool cmMakefile::CheckCMP0037(std::string const& targetName, cmStateEnums::TargetType targetType) const { MessageType messageType = MessageType::AUTHOR_WARNING; - std::ostringstream e; + std::string e; bool issueMessage = false; switch (this->GetPolicyStatus(cmPolicies::CMP0037)) { case cmPolicies::WARN: if (targetType != cmStateEnums::INTERFACE_LIBRARY) { - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0037) << '\n'; + e = cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0037), '\n'); issueMessage = true; } CM_FALLTHROUGH; @@ -218,11 +218,12 @@ bool cmMakefile::CheckCMP0037(std::string const& targetName, break; } if (issueMessage) { - e << "The target name \"" << targetName - << "\" is reserved or not valid for certain " - "CMake features, such as generator expressions, and may result " - "in undefined behavior."; - this->IssueMessage(messageType, e.str()); + e += + cmStrCat("The target name \"", targetName, + "\" is reserved or not valid for certain " + "CMake features, such as generator expressions, and may result " + "in undefined behavior."); + this->IssueMessage(messageType, e); if (messageType == MessageType::FATAL_ERROR) { return false; @@ -236,47 +237,37 @@ void cmMakefile::MaybeWarnCMP0074(std::string const& rootVar, cmValue rootDef, { // Warn if a _ROOT variable we may use is set. if ((rootDef || rootEnv) && this->WarnedCMP0074.insert(rootVar).second) { - std::ostringstream w; - w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0074) << '\n'; + auto e = cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0074), '\n'); if (rootDef) { - w << "CMake variable " << rootVar - << " is set to:\n" - " " - << *rootDef << '\n'; + e += cmStrCat("CMake variable ", rootVar, " is set to:\n ", *rootDef, + '\n'); } if (rootEnv) { - w << "Environment variable " << rootVar - << " is set to:\n" - " " - << *rootEnv << '\n'; + e += cmStrCat("Environment variable ", rootVar, " is set to:\n ", + *rootEnv, '\n'); } - w << "For compatibility, CMake is ignoring the variable."; - this->IssueMessage(MessageType::AUTHOR_WARNING, w.str()); + e += "For compatibility, CMake is ignoring the variable."; + this->IssueMessage(MessageType::AUTHOR_WARNING, e); } } -void cmMakefile::MaybeWarnCMP0144(std::string const& rootVAR, cmValue rootDEF, - cm::optional const& rootENV) +void cmMakefile::MaybeWarnCMP0144(std::string const& rootVar, cmValue rootDef, + cm::optional const& rootEnv) { // Warn if a _ROOT variable we may use is set. - if ((rootDEF || rootENV) && this->WarnedCMP0144.insert(rootVAR).second) { - std::ostringstream w; - w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0144) << '\n'; - if (rootDEF) { - w << "CMake variable " << rootVAR - << " is set to:\n" - " " - << *rootDEF << '\n'; + if ((rootDef || rootEnv) && this->WarnedCMP0144.insert(rootVar).second) { + auto e = cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0144), '\n'); + if (rootDef) { + e += cmStrCat("CMake variable ", rootVar, " is set to:\n ", *rootDef, + '\n'); } - if (rootENV) { - w << "Environment variable " << rootVAR - << " is set to:\n" - " " - << *rootENV << '\n'; + if (rootEnv) { + e += cmStrCat("Environment variable ", rootVar, " is set to:\n ", + *rootEnv, '\n'); } - w << "For compatibility, find_package is ignoring the variable, but " + e += "For compatibility, find_package is ignoring the variable, but " "code in a .cmake module might still use it."; - this->IssueMessage(MessageType::AUTHOR_WARNING, w.str()); + this->IssueMessage(MessageType::AUTHOR_WARNING, e); } } @@ -341,7 +332,6 @@ void cmMakefile::PrintCommandTrace(cmListFileFunction const& lff, } } - std::ostringstream msg; std::vector args; std::string temp; bool expand = this->GetCMakeInstance()->GetTraceExpand(); @@ -358,6 +348,7 @@ void cmMakefile::PrintCommandTrace(cmListFileFunction const& lff, } cm::optional const& deferId = bt.Top().DeferId; + std::ostringstream msg; switch (this->GetCMakeInstance()->GetTraceFormat()) { case cmake::TraceFormat::JSONv1: { #ifndef CMAKE_BOOTSTRAP @@ -506,9 +497,9 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff, // Check for maximum recursion depth. size_t depthLimit = this->GetRecursionDepthLimit(); if (this->RecursionDepth > depthLimit) { - std::ostringstream e; - e << "Maximum recursion depth of " << depthLimit << " exceeded"; - this->IssueMessage(MessageType::FATAL_ERROR, e.str()); + this->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("Maximum recursion depth of ", depthLimit, " exceeded")); cmSystemTools::SetFatalErrorOccurred(); return false; } @@ -660,28 +651,29 @@ void cmMakefile::IncludeScope::EnforceCMP0011() case cmPolicies::WARN: // Warn because the user did not set this policy. { - std::ostringstream w; - /* clang-format off */ - w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0011) << "\n" - "The included script\n " - << this->Makefile->GetBacktrace().Top().FilePath << "\n" - "affects policy settings. " - "CMake is implying the NO_POLICY_SCOPE option for compatibility, " - "so the effects are applied to the including context."; - /* clang-format on */ - this->Makefile->IssueMessage(MessageType::AUTHOR_WARNING, w.str()); + auto e = cmStrCat( + cmPolicies::GetPolicyWarning(cmPolicies::CMP0011), + "\n" + "The included script\n" + " ", + this->Makefile->GetBacktrace().Top().FilePath, + "\n" + "affects policy settings. " + "CMake is implying the NO_POLICY_SCOPE option for compatibility, " + "so the effects are applied to the including context."); + this->Makefile->IssueMessage(MessageType::AUTHOR_WARNING, e); } break; case cmPolicies::REQUIRED_IF_USED: case cmPolicies::REQUIRED_ALWAYS: { - std::ostringstream e; - /* clang-format off */ - e << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0011) << "\n" - "The included script\n " - << this->Makefile->GetBacktrace().Top().FilePath << "\n" - "affects policy settings, so it requires this policy to be set."; - /* clang-format on */ - this->Makefile->IssueMessage(MessageType::FATAL_ERROR, e.str()); + auto e = cmStrCat( + cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0011), + "\n" + "The included script\n ", + this->Makefile->GetBacktrace().Top().FilePath, + "\n" + "affects policy settings, so it requires this policy to be set."); + this->Makefile->IssueMessage(MessageType::FATAL_ERROR, e); } break; case cmPolicies::OLD: case cmPolicies::NEW: @@ -966,24 +958,23 @@ void cmMakefile::EnforceDirectoryLevelRules() const { // Diagnose a violation of CMP0000 if necessary. if (this->CheckCMP0000) { - std::ostringstream msg; - /* clang-format off */ - msg << "No cmake_minimum_required command is present. " - "A line of code such as\n" - " cmake_minimum_required(VERSION " << cmVersion::GetMajorVersion() - << "." << cmVersion::GetMinorVersion() << ")\n" - "should be added at the top of the file. " - "The version specified may be lower if you wish to " - "support older CMake versions for this project. " - "For more information run " - "\"cmake --help-policy CMP0000\"."; - /* clang-format on */ + std::string e = + cmStrCat("No cmake_minimum_required command is present. " + "A line of code such as\n" + " cmake_minimum_required(VERSION ", + cmVersion::GetMajorVersion(), '.', cmVersion::GetMinorVersion(), + ")\n" + "should be added at the top of the file. " + "The version specified may be lower if you wish to " + "support older CMake versions for this project. " + "For more information run " + "\"cmake --help-policy CMP0000\"."); switch (this->GetPolicyStatus(cmPolicies::CMP0000)) { case cmPolicies::WARN: // Warn because the user did not provide a minimum required // version. - this->GetCMakeInstance()->IssueMessage(MessageType::AUTHOR_WARNING, - msg.str(), this->Backtrace); + this->GetCMakeInstance()->IssueMessage(MessageType::AUTHOR_WARNING, e, + this->Backtrace); CM_FALLTHROUGH; case cmPolicies::OLD: // OLD behavior is to use policy version 2.4 set in @@ -993,8 +984,8 @@ void cmMakefile::EnforceDirectoryLevelRules() const case cmPolicies::REQUIRED_ALWAYS: case cmPolicies::NEW: // NEW behavior is to issue an error. - this->GetCMakeInstance()->IssueMessage(MessageType::FATAL_ERROR, - msg.str(), this->Backtrace); + this->GetCMakeInstance()->IssueMessage(MessageType::FATAL_ERROR, e, + this->Backtrace); cmSystemTools::SetFatalErrorOccurred(); break; } @@ -1142,9 +1133,9 @@ bool cmMakefile::ValidateCustomCommand( // TODO: More strict? for (cmCustomCommandLine const& cl : commandLines) { if (!cl.empty() && !cl[0].empty() && cl[0][0] == '"') { - std::ostringstream e; - e << "COMMAND may not contain literal quotes:\n " << cl[0] << '\n'; - this->IssueMessage(MessageType::FATAL_ERROR, e.str()); + this->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("COMMAND may not contain literal quotes:\n ", cl[0], '\n')); return false; } } @@ -1168,10 +1159,10 @@ cmTarget* cmMakefile::GetCustomCommandTarget( if (ti == this->Targets.end()) { MessageType messageType = MessageType::AUTHOR_WARNING; bool issueMessage = false; - std::ostringstream e; + std::string e; switch (this->GetPolicyStatus(cmPolicies::CMP0040)) { case cmPolicies::WARN: - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0040) << '\n'; + e = cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0040), '\n'); issueMessage = true; CM_FALLTHROUGH; case cmPolicies::OLD: @@ -1187,16 +1178,17 @@ cmTarget* cmMakefile::GetCustomCommandTarget( if (issueMessage) { if (cmTarget const* t = this->FindTargetToUse(target)) { if (t->IsImported()) { - e << "TARGET '" << target - << "' is IMPORTED and does not build here."; + e += cmStrCat("TARGET '", target, + "' is IMPORTED and does not build here."); } else { - e << "TARGET '" << target << "' was not created in this directory."; + e += cmStrCat("TARGET '", target, + "' was not created in this directory."); } } else { - e << "No TARGET '" << target - << "' has been created in this directory."; + e += cmStrCat("No TARGET '", target, + "' has been created in this directory."); } - this->GetCMakeInstance()->IssueMessage(messageType, e.str(), lfbt); + this->GetCMakeInstance()->IssueMessage(messageType, e, lfbt); } return nullptr; @@ -1205,21 +1197,19 @@ cmTarget* cmMakefile::GetCustomCommandTarget( cmTarget* t = &ti->second; if (objLibCommands == cmObjectLibraryCommands::Reject && t->GetType() == cmStateEnums::OBJECT_LIBRARY) { - std::ostringstream e; - e << "Target \"" << target - << "\" is an OBJECT library " - "that may not have PRE_BUILD, PRE_LINK, or POST_BUILD commands."; - this->GetCMakeInstance()->IssueMessage(MessageType::FATAL_ERROR, e.str(), - lfbt); + auto e = cmStrCat( + "Target \"", target, + "\" is an OBJECT library " + "that may not have PRE_BUILD, PRE_LINK, or POST_BUILD commands."); + this->GetCMakeInstance()->IssueMessage(MessageType::FATAL_ERROR, e, lfbt); return nullptr; } if (t->GetType() == cmStateEnums::INTERFACE_LIBRARY) { - std::ostringstream e; - e << "Target \"" << target - << "\" is an INTERFACE library " - "that may not have PRE_BUILD, PRE_LINK, or POST_BUILD commands."; - this->GetCMakeInstance()->IssueMessage(MessageType::FATAL_ERROR, e.str(), - lfbt); + auto e = cmStrCat( + "Target \"", target, + "\" is an INTERFACE library " + "that may not have PRE_BUILD, PRE_LINK, or POST_BUILD commands."); + this->GetCMakeInstance()->IssueMessage(MessageType::FATAL_ERROR, e, lfbt); return nullptr; } @@ -1901,34 +1891,31 @@ void cmMakefile::ConfigureSubDirectory(cmMakefile* mf) cmStrCat(currentStart, "/CMakeLists.txt"); if (!cmSystemTools::FileExists(currentStartFile, true)) { // The file is missing. Check policy CMP0014. - std::ostringstream e; - /* clang-format off */ - e << "The source directory\n " << currentStart << "\n" - "does not contain a CMakeLists.txt file."; + auto e = cmStrCat("The source directory\n ", currentStart, + "\n" + "does not contain a CMakeLists.txt file."); /* clang-format on */ switch (this->GetPolicyStatus(cmPolicies::CMP0014)) { case cmPolicies::WARN: // Print the warning. - /* clang-format off */ - e << "\n" - "CMake does not support this case but it used " - "to work accidentally and is being allowed for " - "compatibility." - "\n" - << cmPolicies::GetPolicyWarning(cmPolicies::CMP0014); - /* clang-format on */ - this->IssueMessage(MessageType::AUTHOR_WARNING, e.str()); + e += cmStrCat("\n" + "CMake does not support this case but it used " + "to work accidentally and is being allowed for " + "compatibility.\n", + cmPolicies::GetPolicyWarning(cmPolicies::CMP0014)); + this->IssueMessage(MessageType::AUTHOR_WARNING, e); CM_FALLTHROUGH; case cmPolicies::OLD: // OLD behavior does not warn. break; case cmPolicies::REQUIRED_IF_USED: case cmPolicies::REQUIRED_ALWAYS: - e << '\n' << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0014); + e += cmStrCat('\n', + cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0014)); CM_FALLTHROUGH; case cmPolicies::NEW: // NEW behavior prints the error. - this->IssueMessage(MessageType::FATAL_ERROR, e.str()); + this->IssueMessage(MessageType::FATAL_ERROR, e); break; } return; @@ -1937,7 +1924,7 @@ void cmMakefile::ConfigureSubDirectory(cmMakefile* mf) mf->Configure(); if (this->GetCMakeInstance()->GetDebugOutput()) { - std::string msg = + auto msg = cmStrCat(" Returning to ", this->GetCurrentSourceDirectory()); cmSystemTools::Message(msg); } @@ -2140,9 +2127,9 @@ void cmMakefile::MaybeWarnUninitialized(std::string const& variable, !this->VariableInitialized(variable)) { if (this->CheckSystemVars || (sourceFilename && this->IsProjectFile(sourceFilename))) { - std::ostringstream msg; - msg << "uninitialized variable \'" << variable << '\''; - this->IssueMessage(MessageType::AUTHOR_WARNING, msg.str()); + this->IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat("uninitialized variable \'", variable, '\'')); } } } @@ -2438,19 +2425,16 @@ void cmMakefile::ExpandVariablesCMP0019() if (pol != cmPolicies::OLD && pol != cmPolicies::WARN) { return; } - std::ostringstream w; + + std::string e; cmValue includeDirs = this->GetProperty("INCLUDE_DIRECTORIES"); if (includeDirs && mightExpandVariablesCMP0019(includeDirs->c_str())) { std::string dirs = *includeDirs; this->ExpandVariablesInString(dirs, true, true); if (pol == cmPolicies::WARN && dirs != *includeDirs) { - /* clang-format off */ - w << "Evaluated directory INCLUDE_DIRECTORIES\n" - " " << *includeDirs << "\n" - "as\n" - " " << dirs << '\n'; - /* clang-format on */ + e = cmStrCat("Evaluated directory INCLUDE_DIRECTORIES\n ", *includeDirs, + "\nas\n ", dirs, '\n'); } this->SetProperty("INCLUDE_DIRECTORIES", dirs); } @@ -2467,12 +2451,9 @@ void cmMakefile::ExpandVariablesCMP0019() std::string dirs = *includeDirs; this->ExpandVariablesInString(dirs, true, true); if (pol == cmPolicies::WARN && dirs != *includeDirs) { - /* clang-format off */ - w << "Evaluated target " << t.GetName() << " INCLUDE_DIRECTORIES\n" - " " << *includeDirs << "\n" - "as\n" - " " << dirs << '\n'; - /* clang-format on */ + e += cmStrCat("Evaluated target ", t.GetName(), + " INCLUDE_DIRECTORIES\n ", *includeDirs, "\nas\n ", + dirs, '\n'); } t.SetProperty("INCLUDE_DIRECTORIES", dirs); } @@ -2484,12 +2465,8 @@ void cmMakefile::ExpandVariablesCMP0019() const std::string orig = d; this->ExpandVariablesInString(d, true, true); if (pol == cmPolicies::WARN && d != orig) { - /* clang-format off */ - w << "Evaluated link directories\n" - " " << orig << "\n" - "as\n" - " " << d << '\n'; - /* clang-format on */ + e += cmStrCat("Evaluated link directories\n ", orig, "\nas\n ", d, + '\n'); } } } @@ -2507,27 +2484,20 @@ void cmMakefile::ExpandVariablesCMP0019() const std::string orig = libName; this->ExpandVariablesInString(libName, true, true); if (pol == cmPolicies::WARN && libName != orig) { - /* clang-format off */ - w << "Evaluated link library\n" - " " << orig << "\n" - "as\n" - " " << libName << '\n'; - /* clang-format on */ + e += cmStrCat("Evaluated link library\n ", orig, "\nas\n ", + libName, '\n'); } } } } - if (!w.str().empty()) { - std::ostringstream m; - /* clang-format off */ - m << cmPolicies::GetPolicyWarning(cmPolicies::CMP0019) - << "\n" - "The following variable evaluations were encountered:\n" - << w.str(); - /* clang-format on */ - this->GetCMakeInstance()->IssueMessage(MessageType::AUTHOR_WARNING, - m.str(), this->Backtrace); + if (!e.empty()) { + auto m = cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0019), + "\n" + "The following variable evaluations were encountered:\n", + e); + this->GetCMakeInstance()->IssueMessage(MessageType::AUTHOR_WARNING, m, + this->Backtrace); } } @@ -2829,33 +2799,28 @@ const std::string& cmMakefile::ExpandVariablesInString( } // ...otherwise, see if there's a difference that needs to be warned about. else if (compareResults && (newResult != source || newError != mtype)) { - std::string msg = + auto msg = cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0053), '\n'); std::string msg_input = original; cmSystemTools::ReplaceString(msg_input, "\n", "\n "); - msg += "For input:\n '"; - msg += msg_input; - msg += "'\n"; std::string msg_old = source; cmSystemTools::ReplaceString(msg_old, "\n", "\n "); - msg += "the old evaluation rules produce:\n '"; - msg += msg_old; - msg += "'\n"; + + msg += cmStrCat("For input:\n '", msg_input, "'\n", + "the old evaluation rules produce:\n '", msg_old, "'\n"); if (newError == mtype) { std::string msg_new = newResult; cmSystemTools::ReplaceString(msg_new, "\n", "\n "); - msg += "but the new evaluation rules produce:\n '"; - msg += msg_new; - msg += "'\n"; + msg += + cmStrCat("but the new evaluation rules produce:\n '", msg_new, "'\n"); } else { std::string msg_err = newErrorstr; cmSystemTools::ReplaceString(msg_err, "\n", "\n "); - msg += "but the new evaluation rules produce an error:\n "; - msg += msg_err; - msg += "\n"; + msg += cmStrCat("but the new evaluation rules produce an error:\n ", + msg_err, '\n'); } msg += @@ -2935,15 +2900,14 @@ MessageType cmMakefile::ExpandVariablesInStringOld( source = parser.GetResult(); } else { // Construct the main error message. - std::ostringstream error; - error << "Syntax error in cmake code "; + std::string error = "Syntax error in cmake code "; if (filename && line > 0) { // This filename and line number may be more specific than the // command context because one command invocation can have // arguments on multiple lines. - error << "at\n " << filename << ':' << line << '\n'; + error += cmStrCat("at\n ", filename, ':', line, '\n'); } - error << "when parsing string\n " << source << '\n' << emsg; + error += cmStrCat("when parsing string\n ", source, '\n', emsg); // If the parser failed ("res" is false) then this is a real // argument parsing error, so the policy applies. Otherwise the @@ -2956,7 +2920,8 @@ MessageType cmMakefile::ExpandVariablesInStringOld( // decide whether it is an error. switch (this->GetPolicyStatus(cmPolicies::CMP0010)) { case cmPolicies::WARN: - error << '\n' << cmPolicies::GetPolicyWarning(cmPolicies::CMP0010); + error += + cmStrCat('\n', cmPolicies::GetPolicyWarning(cmPolicies::CMP0010)); CM_FALLTHROUGH; case cmPolicies::OLD: // OLD behavior is to just warn and continue. @@ -2964,15 +2929,15 @@ MessageType cmMakefile::ExpandVariablesInStringOld( break; case cmPolicies::REQUIRED_IF_USED: case cmPolicies::REQUIRED_ALWAYS: - error << '\n' - << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0010); + error += cmStrCat( + '\n', cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0010)); break; case cmPolicies::NEW: // NEW behavior is to report the error. break; } } - errorstr = error.str(); + errorstr = std::move(error); } return mtype; } @@ -3313,17 +3278,15 @@ MessageType cmMakefile::ExpandVariablesInStringNew( } if (error) { - std::ostringstream emsg; - emsg << "Syntax error in cmake code "; + std::string e = "Syntax error in cmake code "; if (filename) { // This filename and line number may be more specific than the // command context because one command invocation can have // arguments on multiple lines. - emsg << "at\n " << filename << ':' << line << '\n'; + e += cmStrCat("at\n ", filename, ':', line, '\n'); } - emsg << "when parsing string\n " << source << '\n' << errorstr; + errorstr = cmStrCat(e, "when parsing string\n ", source, '\n', errorstr); mtype = MessageType::FATAL_ERROR; - errorstr = emsg.str(); } else { // Append the rest of the unchanged part of the string. result.append(last); @@ -3573,13 +3536,9 @@ void cmMakefile::SetScriptModeFile(std::string const& scriptfile) void cmMakefile::SetArgcArgv(const std::vector& args) { this->AddDefinition("CMAKE_ARGC", std::to_string(args.size())); - // this->MarkVariableAsUsed("CMAKE_ARGC"); - for (unsigned int t = 0; t < args.size(); ++t) { - std::ostringstream tmpStream; - tmpStream << "CMAKE_ARGV" << t; - this->AddDefinition(tmpStream.str(), args[t]); - // this->MarkVariableAsUsed(tmpStream.str().c_str()); + for (auto i = 0u; i < args.size(); ++i) { + this->AddDefinition(cmStrCat("CMAKE_ARGV", i), args[i]); } } @@ -3985,16 +3944,12 @@ std::string cmMakefile::GetModulesFile(cm::string_view filename, bool& system, if (currentFile && cmSystemTools::IsSubDirectory(*currentFile, mods)) { switch (this->GetPolicyStatus(cmPolicies::CMP0017)) { case cmPolicies::WARN: { - std::ostringstream e; - /* clang-format off */ - e << "File " << *currentFile << " includes " - << moduleInCMakeModulePath - << " (found via CMAKE_MODULE_PATH) which shadows " - << moduleInCMakeRoot << ". This may cause errors later on .\n" - << cmPolicies::GetPolicyWarning(cmPolicies::CMP0017); - /* clang-format on */ - - this->IssueMessage(MessageType::AUTHOR_WARNING, e.str()); + auto e = cmStrCat( + "File ", *currentFile, " includes ", moduleInCMakeModulePath, + " (found via CMAKE_MODULE_PATH) which shadows ", moduleInCMakeRoot, + ". This may cause errors later on .\n", + cmPolicies::GetPolicyWarning(cmPolicies::CMP0017)); + this->IssueMessage(MessageType::AUTHOR_WARNING, e); CM_FALLTHROUGH; } case cmPolicies::OLD: @@ -4157,10 +4112,10 @@ int cmMakefile::ConfigureFile(const std::string& infile, cmsys::FStream::BOM bom = cmsys::FStream::ReadBOM(fin); if (bom != cmsys::FStream::BOM_None && bom != cmsys::FStream::BOM_UTF8) { - std::ostringstream e; - e << "File starts with a Byte-Order-Mark that is not UTF-8:\n " - << sinfile; - this->IssueMessage(MessageType::FATAL_ERROR, e.str()); + this->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("File starts with a Byte-Order-Mark that is not UTF-8:\n ", + sinfile)); return 0; } // rewind to copy BOM to output file @@ -4350,9 +4305,9 @@ void cmMakefile::RaiseScope(const std::string& var, const char* varDef) } if (!this->StateSnapshot.RaiseScope(var, varDef)) { - std::ostringstream m; - m << "Cannot set \"" << var << "\": current scope has no parent."; - this->IssueMessage(MessageType::AUTHOR_WARNING, m.str()); + this->IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat("Cannot set \"", var, "\": current scope has no parent.")); return; } @@ -4437,10 +4392,8 @@ bool cmMakefile::EnforceUniqueName(std::string const& name, std::string& msg, bool isCustom) const { if (this->IsAlias(name)) { - std::ostringstream e; - e << "cannot create target \"" << name - << "\" because an alias with the same name already exists."; - msg = e.str(); + msg = cmStrCat("cannot create target \"", name, + "\" because an alias with the same name already exists."); return false; } if (cmTarget* existing = this->FindTargetToUse(name)) { @@ -4449,10 +4402,9 @@ bool cmMakefile::EnforceUniqueName(std::string const& name, std::string& msg, if (existing->IsImported()) { // Imported targets were not supported in previous versions. // This is new code, so we can make it an error. - std::ostringstream e; - e << "cannot create target \"" << name - << "\" because an imported target with the same name already exists."; - msg = e.str(); + msg = cmStrCat( + "cannot create target \"", name, + "\" because an imported target with the same name already exists."); return false; } // target names must be globally unique @@ -4529,43 +4481,48 @@ bool cmMakefile::EnforceUniqueDir(const std::string& srcPath, if (gg->BinaryDirectoryIsNew(binPath)) { return true; } - std::ostringstream e; + std::string e; switch (this->GetPolicyStatus(cmPolicies::CMP0013)) { case cmPolicies::WARN: // Print the warning. - /* clang-format off */ - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0013) - << "\n" - "The binary directory\n" - " " << binPath << "\n" - "is already used to build a source directory. " - "This command uses it to build source directory\n" - " " << srcPath << "\n" - "which can generate conflicting build files. " - "CMake does not support this use case but it used " - "to work accidentally and is being allowed for " - "compatibility."; - /* clang-format on */ - this->IssueMessage(MessageType::AUTHOR_WARNING, e.str()); + e = cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0013), + "\n" + "The binary directory\n" + " ", + binPath, + "\n" + "is already used to build a source directory. " + "This command uses it to build source directory\n" + " ", + srcPath, + "\n" + "which can generate conflicting build files. " + "CMake does not support this use case but it used " + "to work accidentally and is being allowed for " + "compatibility."); + this->IssueMessage(MessageType::AUTHOR_WARNING, e); CM_FALLTHROUGH; case cmPolicies::OLD: // OLD behavior does not warn. return true; case cmPolicies::REQUIRED_IF_USED: case cmPolicies::REQUIRED_ALWAYS: - e << cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0013) << '\n'; + e = cmStrCat(cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0013), + '\n'); CM_FALLTHROUGH; case cmPolicies::NEW: // NEW behavior prints the error. - /* clang-format off */ - e << "The binary directory\n" - " " << binPath << "\n" - "is already used to build a source directory. " - "It cannot be used to build source directory\n" - " " << srcPath << "\n" - "Specify a unique binary directory name."; - /* clang-format on */ - this->IssueMessage(MessageType::FATAL_ERROR, e.str()); + e += cmStrCat("The binary directory\n" + " ", + binPath, + "\n" + "is already used to build a source directory. " + "It cannot be used to build source directory\n" + " ", + srcPath, + "\n" + "Specify a unique binary directory name."); + this->IssueMessage(MessageType::FATAL_ERROR, e); break; } @@ -4648,9 +4605,9 @@ bool cmMakefile::SetPolicy(const char* id, cmPolicies::PolicyStatus status) { cmPolicies::PolicyID pid; if (!cmPolicies::GetPolicyID(id, /* out */ pid)) { - std::ostringstream e; - e << "Policy \"" << id << "\" is not known to this version of CMake."; - this->IssueMessage(MessageType::FATAL_ERROR, e.str()); + this->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("Policy \"", id, "\" is not known to this version of CMake.")); return false; } return this->SetPolicy(pid, status); From d4faff52d3bab9156de13323ef6249c00dd5a99f Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 01:16:46 +0400 Subject: [PATCH 04/13] cmMakefile: strnspn => std::strnspn --- Source/cmMakefile.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index a117849773..5a1c45f101 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3218,10 +3218,10 @@ MessageType cmMakefile::ExpandVariablesInStringNew( if (nextAt && nextAt != in + 1 && nextAt == in + 1 + - strspn(in + 1, - "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - "abcdefghijklmnopqrstuvwxyz" - "0123456789/_.+-")) { + std::strspn(in + 1, + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789/_.+-")) { std::string variable(in + 1, nextAt - in - 1); std::string varresult; From b8fb4b778cb99c593fd35f567cd6501568e8ea1a Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 01:24:45 +0400 Subject: [PATCH 05/13] cmMakefile::ConfigureFile: Convert 'else' block to early return and deindent --- Source/cmMakefile.cxx | 108 +++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 5a1c45f101..4fc33402b1 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4084,68 +4084,70 @@ int cmMakefile::ConfigureFile(const std::string& infile, if (!cmSystemTools::SetPermissions(soutfile, permissions)) { this->IssueMessage(MessageType::FATAL_ERROR, cmSystemTools::GetLastSystemError()); - return 0; + res = 0; } + return res; + } + + std::string newLineCharacters; + std::ios::openmode omode = std::ios::out | std::ios::trunc; + if (newLine.IsValid()) { + newLineCharacters = newLine.GetCharacters(); + omode |= std::ios::binary; } else { - std::string newLineCharacters; - std::ios::openmode omode = std::ios::out | std::ios::trunc; - if (newLine.IsValid()) { - newLineCharacters = newLine.GetCharacters(); - omode |= std::ios::binary; - } else { - newLineCharacters = "\n"; - } - std::string tempOutputFile = cmStrCat(soutfile, ".tmp"); - cmsys::ofstream fout(tempOutputFile.c_str(), omode); - if (!fout) { - cmSystemTools::Error("Could not open file for write in copy operation " + - tempOutputFile); - cmSystemTools::ReportLastSystemError(""); - return 0; - } - cmsys::ifstream fin(sinfile.c_str()); - if (!fin) { - cmSystemTools::Error("Could not open file for read in copy operation " + - sinfile); - return 0; - } + newLineCharacters = "\n"; + } + std::string tempOutputFile = cmStrCat(soutfile, ".tmp"); + cmsys::ofstream fout(tempOutputFile.c_str(), omode); + if (!fout) { + cmSystemTools::Error("Could not open file for write in copy operation " + + tempOutputFile); + cmSystemTools::ReportLastSystemError(""); + return 0; + } + cmsys::ifstream fin(sinfile.c_str()); + if (!fin) { + cmSystemTools::Error("Could not open file for read in copy operation " + + sinfile); + return 0; + } - cmsys::FStream::BOM bom = cmsys::FStream::ReadBOM(fin); - if (bom != cmsys::FStream::BOM_None && bom != cmsys::FStream::BOM_UTF8) { - this->IssueMessage( - MessageType::FATAL_ERROR, - cmStrCat("File starts with a Byte-Order-Mark that is not UTF-8:\n ", - sinfile)); - return 0; - } - // rewind to copy BOM to output file - fin.seekg(0); + cmsys::FStream::BOM bom = cmsys::FStream::ReadBOM(fin); + if (bom != cmsys::FStream::BOM_None && bom != cmsys::FStream::BOM_UTF8) { + this->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("File starts with a Byte-Order-Mark that is not UTF-8:\n ", + sinfile)); + return 0; + } + // rewind to copy BOM to output file + fin.seekg(0); - // now copy input to output and expand variables in the - // input file at the same time - std::string inLine; - std::string outLine; - while (cmSystemTools::GetLineFromStream(fin, inLine)) { - outLine.clear(); - this->ConfigureString(inLine, outLine, atOnly, escapeQuotes); - fout << outLine << newLineCharacters; - } - // close the files before attempting to copy - fin.close(); - fout.close(); - if (!cmSystemTools::CopyFileIfDifferent(tempOutputFile, soutfile)) { + // now copy input to output and expand variables in the + // input file at the same time + std::string inLine; + std::string outLine; + while (cmSystemTools::GetLineFromStream(fin, inLine)) { + outLine.clear(); + this->ConfigureString(inLine, outLine, atOnly, escapeQuotes); + fout << outLine << newLineCharacters; + } + // close the files before attempting to copy + fin.close(); + fout.close(); + if (!cmSystemTools::CopyFileIfDifferent(tempOutputFile, soutfile)) { + this->IssueMessage(MessageType::FATAL_ERROR, + cmSystemTools::GetLastSystemError()); + res = 0; + } else { + if (!cmSystemTools::SetPermissions(soutfile, permissions)) { this->IssueMessage(MessageType::FATAL_ERROR, cmSystemTools::GetLastSystemError()); res = 0; - } else { - if (!cmSystemTools::SetPermissions(soutfile, permissions)) { - this->IssueMessage(MessageType::FATAL_ERROR, - cmSystemTools::GetLastSystemError()); - res = 0; - } } - cmSystemTools::RemoveFile(tempOutputFile); } + cmSystemTools::RemoveFile(tempOutputFile); + return res; } From 29fb6058226757d42fb792b455cfc7890c28c24a Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 19:08:04 +0400 Subject: [PATCH 06/13] cmMakefile::FormatListFileStack: Refactor 'while' into 'for' --- Source/cmMakefile.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 4fc33402b1..6bbfc7cafd 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4256,11 +4256,11 @@ void cmMakefile::AddCMakeDependFilesFromUser() std::string cmMakefile::FormatListFileStack() const { std::vector listFiles; - cmStateSnapshot snp = this->StateSnapshot; - while (snp.IsValid()) { - listFiles.push_back(snp.GetExecutionListFile()); - snp = snp.GetCallStackParent(); + for (auto snp = this->StateSnapshot; snp.IsValid(); + snp = snp.GetCallStackParent()) { + listFiles.emplace_back(snp.GetExecutionListFile()); } + std::reverse(listFiles.begin(), listFiles.end()); std::ostringstream tmp; size_t depth = listFiles.size(); From ce991188f39c657487bbda88ce450bc9c641248b Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 19:09:36 +0400 Subject: [PATCH 07/13] cmMakefile::FormatListFileStack: Exit early on empty stack trace And deindent the rest of the function. --- Source/cmMakefile.cxx | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 6bbfc7cafd..9952bd6d0a 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4261,20 +4261,23 @@ std::string cmMakefile::FormatListFileStack() const listFiles.emplace_back(snp.GetExecutionListFile()); } + if (listFiles.empty()) { + return {}; + } + std::reverse(listFiles.begin(), listFiles.end()); std::ostringstream tmp; size_t depth = listFiles.size(); - if (depth > 0) { - auto it = listFiles.end(); - do { - if (depth != listFiles.size()) { - tmp << "\n "; - } - --it; - tmp << '[' << depth << "]\t" << *it; - depth--; - } while (it != listFiles.begin()); - } + auto it = listFiles.end(); + do { + if (depth != listFiles.size()) { + tmp << "\n "; + } + --it; + tmp << '[' << depth << "]\t" << *it; + depth--; + } while (it != listFiles.begin()); + return tmp.str(); } From 2e16b58b7c72525f78b8aa1eebb7ccd9a0a24512 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 20:29:20 +0400 Subject: [PATCH 08/13] cmStringAlgorithms: Move generic strings join function to public API Move `cmJoinImpl` from `cmStringAlgorithms.cxx` to the `cmStringAlgorithms.h` as `cmJoinStrings`. Two existing overloads are not suitable for reverse iterators due to the hardcoded type of the first parameter. The signature is similar to the generic (template) `cmJoin`. With `enable_if` "magic", `cmJoinString` could be renamed to `cmJoin` in the future. Also, replace `getJoinedLength` with `std::accumulate`. --- Source/cmStringAlgorithms.cxx | 42 ++--------------------------------- Source/cmStringAlgorithms.h | 34 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 40 deletions(-) diff --git a/Source/cmStringAlgorithms.cxx b/Source/cmStringAlgorithms.cxx index e352d8dd22..332bd8d5ae 100644 --- a/Source/cmStringAlgorithms.cxx +++ b/Source/cmStringAlgorithms.cxx @@ -7,7 +7,6 @@ #include // IWYU pragma: keep #include #include -#include std::string cmTrimWhitespace(cm::string_view str) { @@ -239,51 +238,14 @@ bool cmStrToULongLong(std::string const& str, unsigned long long* value) return cmStrToULongLong(str.c_str(), value); } -template -std::size_t getJoinedLength(Range const& rng, cm::string_view separator) -{ - std::size_t rangeLength{}; - for (auto const& item : rng) { - rangeLength += item.size(); - } - - auto const separatorsLength = (rng.size() - 1) * separator.size(); - - return rangeLength + separatorsLength; -} - -template -std::string cmJoinImpl(Range const& rng, cm::string_view separator, - cm::string_view initial) -{ - if (rng.empty()) { - return { std::begin(initial), std::end(initial) }; - } - - std::string result; - result.reserve(initial.size() + getJoinedLength(rng, separator)); - result.append(std::begin(initial), std::end(initial)); - - auto begin = std::begin(rng); - auto end = std::end(rng); - result += *begin; - - for (++begin; begin != end; ++begin) { - result.append(std::begin(separator), std::end(separator)); - result += *begin; - } - - return result; -} - std::string cmJoin(std::vector const& rng, cm::string_view separator, cm::string_view initial) { - return cmJoinImpl(rng, separator, initial); + return cmJoinStrings(rng, separator, initial); } std::string cmJoin(cmStringRange const& rng, cm::string_view separator, cm::string_view initial) { - return cmJoinImpl(rng, separator, initial); + return cmJoinStrings(rng, separator, initial); } diff --git a/Source/cmStringAlgorithms.h b/Source/cmStringAlgorithms.h index 55a1e46125..3d7f9b0839 100644 --- a/Source/cmStringAlgorithms.h +++ b/Source/cmStringAlgorithms.h @@ -7,6 +7,8 @@ #include #include #include +#include +#include #include #include #include @@ -77,6 +79,38 @@ std::string cmJoin(Range const& rng, cm::string_view separator) return os.str(); } +/** Generic function to join strings range with separator + * and initial leading string into a single string. + */ +template +std::string cmJoinStrings(Range const& rng, cm::string_view separator, + cm::string_view initial) +{ + if (rng.empty()) { + return { std::begin(initial), std::end(initial) }; + } + + std::string result; + result.reserve( + std::accumulate(std::begin(rng), std::end(rng), + initial.size() + (rng.size() - 1) * separator.size(), + [](std::size_t sum, const std::string& item) { + return sum + item.size(); + })); + result.append(std::begin(initial), std::end(initial)); + + auto begin = std::begin(rng); + auto end = std::end(rng); + result += *begin; + + for (++begin; begin != end; ++begin) { + result.append(std::begin(separator), std::end(separator)); + result += *begin; + } + + return result; +} + /** * Faster overloads for std::string ranges. * If @a initial is provided, it prepends the resulted string without From 206961bc6089031483667f91b0d50e19ed8b3604 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 20:48:32 +0400 Subject: [PATCH 09/13] cmMakefile::FormatListFileStack: Refactor "manual" loop into algorithms --- Source/cmMakefile.cxx | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 9952bd6d0a..10a7c99b38 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4265,20 +4265,14 @@ std::string cmMakefile::FormatListFileStack() const return {}; } - std::reverse(listFiles.begin(), listFiles.end()); - std::ostringstream tmp; - size_t depth = listFiles.size(); - auto it = listFiles.end(); - do { - if (depth != listFiles.size()) { - tmp << "\n "; - } - --it; - tmp << '[' << depth << "]\t" << *it; - depth--; - } while (it != listFiles.begin()); + auto depth = 1; + std::transform(listFiles.begin(), listFiles.end(), listFiles.begin(), + [&depth](const std::string& file) { + return cmStrCat('[', depth++, "]\t", file); + }); - return tmp.str(); + return cmJoinStrings(cmMakeRange(listFiles.rbegin(), listFiles.rend()), + "\n "_s, {}); } void cmMakefile::PushScope() From e0294fa310c33da6a072df12a2127b489cd3e1c6 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Sat, 6 Jul 2024 10:20:08 +0400 Subject: [PATCH 10/13] cmMakefile: Replace single-char string literal with char for cmStrCat Also, remove redundant quote escapes. --- Source/cmMakefile.cxx | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 10a7c99b38..7c472d328e 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -2127,9 +2127,8 @@ void cmMakefile::MaybeWarnUninitialized(std::string const& variable, !this->VariableInitialized(variable)) { if (this->CheckSystemVars || (sourceFilename && this->IsProjectFile(sourceFilename))) { - this->IssueMessage( - MessageType::AUTHOR_WARNING, - cmStrCat("uninitialized variable \'", variable, '\'')); + this->IssueMessage(MessageType::AUTHOR_WARNING, + cmStrCat("uninitialized variable '", variable, '\'')); } } } @@ -3253,11 +3252,10 @@ MessageType cmMakefile::ExpandVariablesInStringNew( if (!openstack.empty() && !(isalnum(inc) || inc == '_' || inc == '/' || inc == '.' || inc == '+' || inc == '-')) { - errorstr += "Invalid character (\'"; + errorstr += "Invalid character ('"; errorstr += inc; result.append(last, in - last); - errorstr += cmStrCat("\') in a variable name: " - "'", + errorstr += cmStrCat("') in a variable name: '", result.substr(openstack.back().loc), '\''); mtype = MessageType::FATAL_ERROR; error = true; @@ -3912,7 +3910,7 @@ std::string cmMakefile::GetModulesFile(cm::string_view filename, bool& system, break; } if (debug) { - debugBuffer = cmStrCat(debugBuffer, " ", itempl, "\n"); + debugBuffer = cmStrCat(debugBuffer, " ", itempl, '\n'); } } } @@ -3923,7 +3921,7 @@ std::string cmMakefile::GetModulesFile(cm::string_view filename, bool& system, cmSystemTools::ConvertToUnixSlashes(moduleInCMakeRoot); if (!cmSystemTools::FileExists(moduleInCMakeRoot)) { if (debug) { - debugBuffer = cmStrCat(debugBuffer, " ", moduleInCMakeRoot, "\n"); + debugBuffer = cmStrCat(debugBuffer, " ", moduleInCMakeRoot, '\n'); } moduleInCMakeRoot.clear(); } @@ -3996,8 +3994,8 @@ void cmMakefile::ConfigureString(const std::string& input, std::string& output, if (!def.IsOff()) { const std::string indentation = this->cmDefineRegex.match(1); cmSystemTools::ReplaceString(line, - cmStrCat("#", indentation, "cmakedefine"), - cmStrCat("#", indentation, "define")); + cmStrCat('#', indentation, "cmakedefine"), + cmStrCat('#', indentation, "define")); output += line; } else { output += "/* #undef "; @@ -4008,8 +4006,8 @@ void cmMakefile::ConfigureString(const std::string& input, std::string& output, const std::string indentation = this->cmDefine01Regex.match(1); cmValue def = this->GetDefinition(this->cmDefine01Regex.match(2)); cmSystemTools::ReplaceString(line, - cmStrCat("#", indentation, "cmakedefine01"), - cmStrCat("#", indentation, "define")); + cmStrCat('#', indentation, "cmakedefine01"), + cmStrCat('#', indentation, "define")); output += line; if (!def.IsOff()) { output += " 1"; @@ -4021,7 +4019,7 @@ void cmMakefile::ConfigureString(const std::string& input, std::string& output, } if (haveNewline) { - output += "\n"; + output += '\n'; } // Move to the next line. From 0a0a826b862c320c0af11a55e8fa05b0c393a9aa Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Sat, 6 Jul 2024 10:55:02 +0400 Subject: [PATCH 11/13] cmMakefile: Use find_if instead of manual loop ...in the `cmMakefile::ValidateCustomCommand()` as it was a warning issued by `clang-tidy`. --- Source/cmMakefile.cxx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 7c472d328e..d496d36cc5 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1131,15 +1131,17 @@ bool cmMakefile::ValidateCustomCommand( const cmCustomCommandLines& commandLines) const { // TODO: More strict? - for (cmCustomCommandLine const& cl : commandLines) { - if (!cl.empty() && !cl[0].empty() && cl[0][0] == '"') { - this->IssueMessage( - MessageType::FATAL_ERROR, - cmStrCat("COMMAND may not contain literal quotes:\n ", cl[0], '\n')); - return false; - } + const auto it = + std::find_if(commandLines.begin(), commandLines.end(), + [](const cmCustomCommandLine& cl) { + return !cl.empty() && !cl[0].empty() && cl[0][0] == '"'; + }); + if (it != commandLines.end()) { + this->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("COMMAND may not contain literal quotes:\n ", (*it)[0], '\n')); + return false; } - return true; } From dc38f812378b5feedf29f9ea23ba764f62ea3b04 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Wed, 10 Jul 2024 21:46:31 +0400 Subject: [PATCH 12/13] cmSystemTools: Revise MoveFileIfDifferent to return cmsys::Status Help callers recover errors without relying on global state. --- Source/cmSystemTools.cxx | 10 ++++++---- Source/cmSystemTools.h | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 6e32bd92a8..8e59a358ec 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -1319,16 +1319,18 @@ cmSystemTools::RenameResult cmSystemTools::RenameFile( #endif } -void cmSystemTools::MoveFileIfDifferent(const std::string& source, - const std::string& destination) +cmsys::Status cmSystemTools::MoveFileIfDifferent( + const std::string& source, const std::string& destination) { + cmsys::Status res = {}; if (FilesDiffer(source, destination)) { if (RenameFile(source, destination)) { - return; + return res; } - CopyFileAlways(source, destination); + res = CopyFileAlways(source, destination); } RemoveFile(source); + return res; } void cmSystemTools::Glob(const std::string& directory, diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index 00a6c707d2..a07c749985 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -211,8 +211,8 @@ public: std::string* err = nullptr); //! Rename a file if contents are different, delete the source otherwise - static void MoveFileIfDifferent(const std::string& source, - const std::string& destination); + static cmsys::Status MoveFileIfDifferent(const std::string& source, + const std::string& destination); /** * Run a single executable command From 62bb8a77a590c705b73ea80783d2d2693e70cfa8 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Mon, 1 Jul 2024 01:52:47 +0400 Subject: [PATCH 13/13] cmMakefile: Teach ConfigureFile to move the temp file instead of copying it In the variables substitution mode `cmMakefile::ConfigureFile` was copy file and then remove temporary rendered file. With updated signature of `MoveFileIfDifferent` it can be used instead of `CopyFileIfDifferent`. --- Source/cmMakefile.cxx | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index d496d36cc5..2803279beb 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4076,15 +4076,23 @@ int cmMakefile::ConfigureFile(const std::string& infile, } if (copyonly) { - if (!cmSystemTools::CopyFileIfDifferent(sinfile, soutfile)) { - this->IssueMessage(MessageType::FATAL_ERROR, - cmSystemTools::GetLastSystemError()); - return 0; - } - if (!cmSystemTools::SetPermissions(soutfile, permissions)) { - this->IssueMessage(MessageType::FATAL_ERROR, - cmSystemTools::GetLastSystemError()); + const auto copy_status = + cmSystemTools::CopyFileIfDifferent(sinfile, soutfile); + if (!copy_status) { + this->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("Fail to copy ", + copy_status.Path == cmsys::SystemTools::CopyStatus::SourcePath + ? "source" + : "destination", + "file: ", copy_status.GetString())); res = 0; + } else { + const auto status = cmSystemTools::SetPermissions(soutfile, permissions); + if (!status) { + this->IssueMessage(MessageType::FATAL_ERROR, status.GetString()); + res = 0; + } } return res; } @@ -4135,18 +4143,18 @@ int cmMakefile::ConfigureFile(const std::string& infile, // close the files before attempting to copy fin.close(); fout.close(); - if (!cmSystemTools::CopyFileIfDifferent(tempOutputFile, soutfile)) { - this->IssueMessage(MessageType::FATAL_ERROR, - cmSystemTools::GetLastSystemError()); + + auto status = cmSystemTools::MoveFileIfDifferent(tempOutputFile, soutfile); + if (!status) { + this->IssueMessage(MessageType::FATAL_ERROR, status.GetString()); res = 0; } else { - if (!cmSystemTools::SetPermissions(soutfile, permissions)) { - this->IssueMessage(MessageType::FATAL_ERROR, - cmSystemTools::GetLastSystemError()); + status = cmSystemTools::SetPermissions(soutfile, permissions); + if (!status) { + this->IssueMessage(MessageType::FATAL_ERROR, status.GetString()); res = 0; } } - cmSystemTools::RemoveFile(tempOutputFile); return res; }