From 3d378541bb22f00e3a22bf5f12e97b7943a81294 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 7 Dec 2021 15:18:17 -0500 Subject: cmMessenger: Adopt backtrace printing functions Move backtrace printing functions from `cmListFileBacktrace` over to `cmMessenger`, their primary caller. Thread `cmMessenger` instances through APIs needed to update other call sites. --- Source/cmGlobVerificationManager.cxx | 19 ++++--- Source/cmGlobVerificationManager.h | 6 ++- Source/cmListFileCache.cxx | 48 ------------------ Source/cmListFileCache.h | 6 --- Source/cmMessenger.cxx | 58 +++++++++++++++++++++- Source/cmMessenger.h | 5 ++ Source/cmState.cxx | 20 ++++---- Source/cmState.h | 5 +- Source/cmake.cxx | 5 +- ...LOB-error-CONFIGURE_DEPENDS-modified-stderr.txt | 22 +++++--- 10 files changed, 107 insertions(+), 87 deletions(-) diff --git a/Source/cmGlobVerificationManager.cxx b/Source/cmGlobVerificationManager.cxx index 9ac5cd5..89c5b01 100644 --- a/Source/cmGlobVerificationManager.cxx +++ b/Source/cmGlobVerificationManager.cxx @@ -8,11 +8,14 @@ #include "cmGeneratedFileStream.h" #include "cmListFileCache.h" +#include "cmMessageType.h" +#include "cmMessenger.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmVersion.h" -bool cmGlobVerificationManager::SaveVerificationScript(const std::string& path) +bool cmGlobVerificationManager::SaveVerificationScript(const std::string& path, + cmMessenger* messenger) { if (this->Cache.empty()) { return true; @@ -52,7 +55,7 @@ bool cmGlobVerificationManager::SaveVerificationScript(const std::string& path) for (auto const& bt : v.Backtraces) { verifyScriptFile << "# " << std::get<0>(bt); - std::get<1>(bt).PrintTitle(verifyScriptFile); + messenger->PrintBacktraceTitle(verifyScriptFile, std::get<1>(bt)); verifyScriptFile << "\n"; } @@ -145,7 +148,7 @@ void cmGlobVerificationManager::AddCacheEntry( const bool recurse, const bool listDirectories, const bool followSymlinks, const std::string& relative, const std::string& expression, const std::vector& files, const std::string& variable, - const cmListFileBacktrace& backtrace) + const cmListFileBacktrace& backtrace, cmMessenger* messenger) { CacheEntryKey key = CacheEntryKey(recurse, listDirectories, followSymlinks, relative, expression); @@ -157,17 +160,17 @@ void cmGlobVerificationManager::AddCacheEntry( } else if (value.Initialized && value.Files != files) { std::ostringstream message; message << std::boolalpha; - message << "The glob expression\n"; + message << "The glob expression\n "; key.PrintGlobCommand(message, variable); - backtrace.PrintTitle(message); - message << "\nwas already present in the glob cache but the directory\n" + message << "\nwas already present in the glob cache but the directory " "contents have changed during the configuration run.\n"; message << "Matching glob expressions:"; for (auto const& bt : value.Backtraces) { message << "\n " << std::get<0>(bt); - std::get<1>(bt).PrintTitle(message); + messenger->PrintBacktraceTitle(message, std::get<1>(bt)); } - cmSystemTools::Error(message.str()); + messenger->IssueMessage(MessageType::FATAL_ERROR, message.str(), + backtrace); } else { value.Backtraces.emplace_back(variable, backtrace); } diff --git a/Source/cmGlobVerificationManager.h b/Source/cmGlobVerificationManager.h index b618fb0..52d71aa 100644 --- a/Source/cmGlobVerificationManager.h +++ b/Source/cmGlobVerificationManager.h @@ -12,6 +12,8 @@ #include "cmListFileCache.h" +class cmMessenger; + /** \class cmGlobVerificationManager * \brief Class for expressing build-time dependencies on glob expressions. * @@ -23,7 +25,7 @@ class cmGlobVerificationManager protected: //! Save verification script for given makefile. //! Saves to output //VerifyGlobs.cmake - bool SaveVerificationScript(const std::string& path); + bool SaveVerificationScript(const std::string& path, cmMessenger* messenger); //! Add an entry into the glob cache void AddCacheEntry(bool recurse, bool listDirectories, bool followSymlinks, @@ -31,7 +33,7 @@ protected: const std::string& expression, const std::vector& files, const std::string& variable, - const cmListFileBacktrace& bt); + const cmListFileBacktrace& bt, cmMessenger* messenger); //! Clear the glob cache for state reset. void Reset(); diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index 2e444f2..4c434a3 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -14,7 +14,6 @@ #include "cmListFileLexer.h" #include "cmMessageType.h" #include "cmMessenger.h" -#include "cmState.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" @@ -540,53 +539,6 @@ cmListFileContext const& cmListFileBacktrace::Top() const return this->TopEntry->Context; } -void cmListFileBacktrace::PrintTitle(std::ostream& out) const -{ - // The title exists only if we have a call on top of the bottom. - if (!this->TopEntry || this->TopEntry->IsBottom()) { - return; - } - cmListFileContext lfc = this->TopEntry->Context; - cmStateSnapshot bottom = this->GetBottom(); - if (bottom.GetState()->GetProjectKind() == cmState::ProjectKind::Normal) { - lfc.FilePath = cmSystemTools::RelativeIfUnder( - bottom.GetState()->GetSourceDirectory(), lfc.FilePath); - } - out << (lfc.Line ? " at " : " in ") << lfc; -} - -void cmListFileBacktrace::PrintCallStack(std::ostream& out) const -{ - // The call stack exists only if we have at least two calls on top - // of the bottom. - if (!this->TopEntry || this->TopEntry->IsBottom() || - this->TopEntry->Parent->IsBottom()) { - return; - } - - bool first = true; - cmStateSnapshot bottom = this->GetBottom(); - for (Entry const* cur = this->TopEntry->Parent.get(); !cur->IsBottom(); - cur = cur->Parent.get()) { - if (cur->Context.Name.empty() && - cur->Context.Line != cmListFileContext::DeferPlaceholderLine) { - // Skip this whole-file scope. When we get here we already will - // have printed a more-specific context within the file. - continue; - } - if (first) { - first = false; - out << "Call Stack (most recent call first):\n"; - } - cmListFileContext lfc = cur->Context; - if (bottom.GetState()->GetProjectKind() == cmState::ProjectKind::Normal) { - lfc.FilePath = cmSystemTools::RelativeIfUnder( - bottom.GetState()->GetSourceDirectory(), lfc.FilePath); - } - out << " " << lfc << "\n"; - } -} - size_t cmListFileBacktrace::Depth() const { size_t depth = 0; diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index 98cb4a7..0e2e299 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -191,12 +191,6 @@ public: // This may be called only if Empty() would return false. cmListFileContext const& Top() const; - // Print the top of the backtrace. - void PrintTitle(std::ostream& out) const; - - // Print the call stack below the top of the backtrace. - void PrintCallStack(std::ostream& out) const; - // Get the number of 'frames' in this backtrace size_t Depth() const; diff --git a/Source/cmMessenger.cxx b/Source/cmMessenger.cxx index 2eead6b..52c9dd0 100644 --- a/Source/cmMessenger.cxx +++ b/Source/cmMessenger.cxx @@ -4,6 +4,8 @@ #include "cmDocumentationFormatter.h" #include "cmMessageMetadata.h" +#include "cmState.h" +#include "cmStateSnapshot.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" @@ -151,6 +153,42 @@ static void displayMessage(MessageType t, std::ostringstream& msg) } } +namespace { +void PrintCallStack(std::ostream& out, cmListFileBacktrace bt) +{ + // The call stack exists only if we have at least two calls on top + // of the bottom. + if (bt.Empty()) { + return; + } + bt = bt.Pop(); + if (bt.Empty()) { + return; + } + + bool first = true; + cmStateSnapshot bottom = bt.GetBottom(); + for (; !bt.Empty(); bt = bt.Pop()) { + cmListFileContext lfc = bt.Top(); + if (lfc.Name.empty() && + lfc.Line != cmListFileContext::DeferPlaceholderLine) { + // Skip this whole-file scope. When we get here we already will + // have printed a more-specific context within the file. + continue; + } + if (first) { + first = false; + out << "Call Stack (most recent call first):\n"; + } + if (bottom.GetState()->GetProjectKind() == cmState::ProjectKind::Normal) { + lfc.FilePath = cmSystemTools::RelativeIfUnder( + bottom.GetState()->GetSourceDirectory(), lfc.FilePath); + } + out << " " << lfc << "\n"; + } +} +} + void cmMessenger::IssueMessage(MessageType t, const std::string& text, const cmListFileBacktrace& backtrace) const { @@ -176,12 +214,28 @@ void cmMessenger::DisplayMessage(MessageType t, const std::string& text, } // Add the immediate context. - backtrace.PrintTitle(msg); + this->PrintBacktraceTitle(msg, backtrace); printMessageText(msg, text); // Add the rest of the context. - backtrace.PrintCallStack(msg); + PrintCallStack(msg, backtrace); displayMessage(t, msg); } + +void cmMessenger::PrintBacktraceTitle(std::ostream& out, + cmListFileBacktrace const& bt) const +{ + // The title exists only if we have a call on top of the bottom. + if (bt.Empty()) { + return; + } + cmListFileContext lfc = bt.Top(); + cmStateSnapshot bottom = bt.GetBottom(); + if (bottom.GetState()->GetProjectKind() == cmState::ProjectKind::Normal) { + lfc.FilePath = cmSystemTools::RelativeIfUnder( + bottom.GetState()->GetSourceDirectory(), lfc.FilePath); + } + out << (lfc.Line ? " at " : " in ") << lfc; +} diff --git a/Source/cmMessenger.h b/Source/cmMessenger.h index b6f5712..8287e70 100644 --- a/Source/cmMessenger.h +++ b/Source/cmMessenger.h @@ -4,6 +4,7 @@ #include "cmConfigure.h" // IWYU pragma: keep +#include #include #include "cmListFileCache.h" @@ -47,6 +48,10 @@ public: return this->DeprecatedWarningsAsErrors; } + // Print the top of a backtrace. + void PrintBacktraceTitle(std::ostream& out, + cmListFileBacktrace const& bt) const; + private: bool IsMessageTypeVisible(MessageType t) const; MessageType ConvertMessageType(MessageType t) const; diff --git a/Source/cmState.cxx b/Source/cmState.cxx index e79949d..07b4759 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -228,22 +228,22 @@ std::string const& cmState::GetGlobVerifyStamp() const return this->GlobVerificationManager->GetVerifyStamp(); } -bool cmState::SaveVerificationScript(const std::string& path) +bool cmState::SaveVerificationScript(const std::string& path, + cmMessenger* messenger) { - return this->GlobVerificationManager->SaveVerificationScript(path); + return this->GlobVerificationManager->SaveVerificationScript(path, + messenger); } -void cmState::AddGlobCacheEntry(bool recurse, bool listDirectories, - bool followSymlinks, - const std::string& relative, - const std::string& expression, - const std::vector& files, - const std::string& variable, - cmListFileBacktrace const& backtrace) +void cmState::AddGlobCacheEntry( + bool recurse, bool listDirectories, bool followSymlinks, + const std::string& relative, const std::string& expression, + const std::vector& files, const std::string& variable, + cmListFileBacktrace const& backtrace, cmMessenger* messenger) { this->GlobVerificationManager->AddCacheEntry( recurse, listDirectories, followSymlinks, relative, expression, files, - variable, backtrace); + variable, backtrace, messenger); } void cmState::RemoveCacheEntry(std::string const& key) diff --git a/Source/cmState.h b/Source/cmState.h index a1666ca..b834bba 100644 --- a/Source/cmState.h +++ b/Source/cmState.h @@ -238,13 +238,14 @@ private: bool DoWriteGlobVerifyTarget() const; std::string const& GetGlobVerifyScript() const; std::string const& GetGlobVerifyStamp() const; - bool SaveVerificationScript(const std::string& path); + bool SaveVerificationScript(const std::string& path, cmMessenger* messenger); void AddGlobCacheEntry(bool recurse, bool listDirectories, bool followSymlinks, const std::string& relative, const std::string& expression, const std::vector& files, const std::string& variable, - cmListFileBacktrace const& bt); + cmListFileBacktrace const& bt, + cmMessenger* messenger); cmPropertyDefinitionMap PropertyDefinitions; std::vector EnabledLanguages; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 8c6a2ec..d927d27 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -2155,7 +2155,8 @@ int cmake::ActualConfigure() "CMakeLists.txt ?"); } - this->State->SaveVerificationScript(this->GetHomeOutputDirectory()); + this->State->SaveVerificationScript(this->GetHomeOutputDirectory(), + this->Messenger.get()); this->SaveCache(this->GetHomeOutputDirectory()); if (cmSystemTools::GetErrorOccuredFlag()) { return -1; @@ -2452,7 +2453,7 @@ void cmake::AddGlobCacheEntry(bool recurse, bool listDirectories, { this->State->AddGlobCacheEntry(recurse, listDirectories, followSymlinks, relative, expression, files, variable, - backtrace); + backtrace, this->Messenger.get()); } std::vector cmake::GetAllExtensions() const diff --git a/Tests/RunCMake/file/GLOB-error-CONFIGURE_DEPENDS-modified-stderr.txt b/Tests/RunCMake/file/GLOB-error-CONFIGURE_DEPENDS-modified-stderr.txt index d7b36eb..4c1aa67 100644 --- a/Tests/RunCMake/file/GLOB-error-CONFIGURE_DEPENDS-modified-stderr.txt +++ b/Tests/RunCMake/file/GLOB-error-CONFIGURE_DEPENDS-modified-stderr.txt @@ -1,7 +1,15 @@ -^CMake Error: The glob expression -.* at GLOB-error-CONFIGURE_DEPENDS-modified\.cmake:[0-9]+ \(file\) -was already present in the glob cache but the directory -contents have changed during the configuration run. -Matching glob expressions: - CONTENT_LIST_1 at GLOB-error-CONFIGURE_DEPENDS-modified\.cmake:[0-9]+ \(file\) - CONTENT_LIST_2 at GLOB-error-CONFIGURE_DEPENDS-modified\.cmake:[0-9]+ \(file\)$ +^CMake Error at GLOB-error-CONFIGURE_DEPENDS-modified\.cmake:[0-9]+ \(file\): + The glob expression + + [^ +]* + + was already present in the glob cache but the directory contents have + changed during the configuration run. + + Matching glob expressions: + + CONTENT_LIST_1 at GLOB-error-CONFIGURE_DEPENDS-modified\.cmake:[0-9]+ \(file\) + CONTENT_LIST_2 at GLOB-error-CONFIGURE_DEPENDS-modified\.cmake:[0-9]+ \(file\) +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\)$ -- cgit v0.12 From c749982c13fa00a968fe0b171946b96d0884ea54 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 7 Dec 2021 15:23:53 -0500 Subject: cmTargetPropertyComputer: Simplify by restoring use of cmMakefile Logically revert commit 390a7d8647 (cmTargetPropertyComputer: Implement GetProperty without cmMakefile, 2016-10-13, v3.8.0-rc1~445^2~9). It relied on using `cmListFileBacktrace` to get a scope in which to look up policies. This does remove a backtrace from `LOCATION` property errors at generate time, but the backtrace we reported before was incorrect. It pointed at the addition of a target, not to the reference to the property. --- Source/cmGeneratorTarget.cxx | 9 +++---- Source/cmGetPropertyCommand.cxx | 8 ++----- Source/cmGetTargetPropertyCommand.cxx | 7 +----- Source/cmTarget.cxx | 15 +++++------- Source/cmTarget.h | 12 ++++++---- Source/cmTargetDepend.h | 2 ++ Source/cmTargetPropertyComputer.cxx | 10 ++++---- Source/cmTargetPropertyComputer.h | 28 ++++++++-------------- .../TARGET_PROPERTY-LOCATION-stderr.txt | 6 +---- 9 files changed, 37 insertions(+), 60 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 2fedbd1..e516e53 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -52,8 +52,6 @@ #include "cmTargetPropertyComputer.h" #include "cmake.h" -class cmMessenger; - namespace { const cmsys::RegularExpression FrameworkRegularExpression( "^(.*/)?([^/]*)\\.framework/(.*)$"); @@ -61,8 +59,7 @@ const cmsys::RegularExpression FrameworkRegularExpression( template <> cmValue cmTargetPropertyComputer::GetSources( - cmGeneratorTarget const* tgt, cmMessenger* /* messenger */, - cmListFileBacktrace const& /* context */) + cmGeneratorTarget const* tgt, cmMakefile const& /* mf */) { return tgt->GetSourcesProperty(); } @@ -439,8 +436,8 @@ std::string cmGeneratorTarget::GetExportName() const cmValue cmGeneratorTarget::GetProperty(const std::string& prop) const { - if (cmValue result = cmTargetPropertyComputer::GetProperty( - this, prop, this->Makefile->GetMessenger(), this->GetBacktrace())) { + if (cmValue result = + cmTargetPropertyComputer::GetProperty(this, prop, *this->Makefile)) { return result; } if (cmSystemTools::GetFatalErrorOccured()) { diff --git a/Source/cmGetPropertyCommand.cxx b/Source/cmGetPropertyCommand.cxx index 162860a..b74ed48 100644 --- a/Source/cmGetPropertyCommand.cxx +++ b/Source/cmGetPropertyCommand.cxx @@ -7,7 +7,6 @@ #include "cmExecutionStatus.h" #include "cmGlobalGenerator.h" #include "cmInstalledFile.h" -#include "cmListFileCache.h" #include "cmMakefile.h" #include "cmMessageType.h" #include "cmPolicies.h" @@ -23,8 +22,6 @@ #include "cmValue.h" #include "cmake.h" -class cmMessenger; - namespace { enum OutType { @@ -365,9 +362,8 @@ bool HandleTargetMode(cmExecutionStatus& status, const std::string& name, } return StoreResult(infoType, status.GetMakefile(), variable, nullptr); } - cmListFileBacktrace bt = status.GetMakefile().GetBacktrace(); - cmMessenger* messenger = status.GetMakefile().GetMessenger(); - cmValue prop = target->GetComputedProperty(propertyName, messenger, bt); + cmValue prop = + target->GetComputedProperty(propertyName, status.GetMakefile()); if (!prop) { prop = target->GetProperty(propertyName); } diff --git a/Source/cmGetTargetPropertyCommand.cxx b/Source/cmGetTargetPropertyCommand.cxx index 12c8221..e1ae9b2 100644 --- a/Source/cmGetTargetPropertyCommand.cxx +++ b/Source/cmGetTargetPropertyCommand.cxx @@ -6,15 +6,12 @@ #include "cmExecutionStatus.h" #include "cmGlobalGenerator.h" -#include "cmListFileCache.h" #include "cmMakefile.h" #include "cmMessageType.h" #include "cmPolicies.h" #include "cmTarget.h" #include "cmValue.h" -class cmMessenger; - bool cmGetTargetPropertyCommand(std::vector const& args, cmExecutionStatus& status) { @@ -43,9 +40,7 @@ bool cmGetTargetPropertyCommand(std::vector const& args, } } else if (!args[2].empty()) { cmValue prop_cstr = nullptr; - cmListFileBacktrace bt = mf.GetBacktrace(); - cmMessenger* messenger = mf.GetMessenger(); - prop_cstr = tgt->GetComputedProperty(args[2], messenger, bt); + prop_cstr = tgt->GetComputedProperty(args[2], mf); if (!prop_cstr) { prop_cstr = tgt->GetProperty(args[2]); } diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index efae691..ebf5bd0 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -27,7 +27,6 @@ #include "cmListFileCache.h" #include "cmMakefile.h" #include "cmMessageType.h" -#include "cmMessenger.h" #include "cmProperty.h" #include "cmPropertyMap.h" #include "cmRange.h" @@ -82,9 +81,8 @@ const std::string& cmTargetPropertyComputer::ComputeLocation( } template <> -cmValue cmTargetPropertyComputer::GetSources( - cmTarget const* tgt, cmMessenger* messenger, - cmListFileBacktrace const& context) +cmValue cmTargetPropertyComputer::GetSources(cmTarget const* tgt, + cmMakefile const& mf) { cmBTStringRange entries = tgt->GetSourceEntries(); if (entries.empty()) { @@ -111,7 +109,7 @@ cmValue cmTargetPropertyComputer::GetSources( bool noMessage = true; std::ostringstream e; MessageType messageType = MessageType::AUTHOR_WARNING; - switch (context.GetBottom().GetPolicy(cmPolicies::CMP0051)) { + switch (mf.GetPolicyStatus(cmPolicies::CMP0051)) { case cmPolicies::WARN: e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; noMessage = false; @@ -132,7 +130,7 @@ cmValue cmTargetPropertyComputer::GetSources( "time. Code reading that property needs to be adapted to " "ignore the generator expression using the string(GENEX_STRIP) " "command."; - messenger->IssueMessage(messageType, e.str(), context); + mf.IssueMessage(messageType, e.str()); } if (addContent) { ss << sep; @@ -1813,10 +1811,9 @@ void cmTarget::CheckProperty(const std::string& prop, } cmValue cmTarget::GetComputedProperty(const std::string& prop, - cmMessenger* messenger, - cmListFileBacktrace const& context) const + cmMakefile& mf) const { - return cmTargetPropertyComputer::GetProperty(this, prop, messenger, context); + return cmTargetPropertyComputer::GetProperty(this, prop, mf); } cmValue cmTarget::GetProperty(const std::string& prop) const diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 27b325a..c4314a4 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -12,7 +12,6 @@ #include #include "cmAlgorithms.h" -#include "cmListFileCache.h" #include "cmPolicies.h" #include "cmStateTypes.h" #include "cmStringAlgorithms.h" @@ -23,12 +22,18 @@ class cmCustomCommand; class cmFileSet; class cmGlobalGenerator; class cmInstallTargetGenerator; +class cmListFileBacktrace; +class cmListFileContext; class cmMakefile; -class cmMessenger; class cmPropertyMap; class cmSourceFile; class cmTargetInternals; +template +class BT; +template +class BTs; + /** \class cmTarget * \brief Represent a library or executable target loaded from a makefile. * @@ -184,8 +189,7 @@ public: std::string const& GetSafeProperty(std::string const& prop) const; bool GetPropertyAsBool(const std::string& prop) const; void CheckProperty(const std::string& prop, cmMakefile* context) const; - cmValue GetComputedProperty(const std::string& prop, cmMessenger* messenger, - cmListFileBacktrace const& context) const; + cmValue GetComputedProperty(const std::string& prop, cmMakefile& mf) const; //! Get all properties cmPropertyMap const& GetProperties() const; diff --git a/Source/cmTargetDepend.h b/Source/cmTargetDepend.h index 36702bd..9027409 100644 --- a/Source/cmTargetDepend.h +++ b/Source/cmTargetDepend.h @@ -6,6 +6,8 @@ #include +#include "cmListFileCache.h" + class cmGeneratorTarget; /** One edge in the global target dependency graph. diff --git a/Source/cmTargetPropertyComputer.cxx b/Source/cmTargetPropertyComputer.cxx index 9b94142..134b4b6 100644 --- a/Source/cmTargetPropertyComputer.cxx +++ b/Source/cmTargetPropertyComputer.cxx @@ -5,19 +5,17 @@ #include +#include "cmMakefile.h" #include "cmMessageType.h" -#include "cmMessenger.h" #include "cmPolicies.h" -#include "cmStateSnapshot.h" bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( - std::string const& tgtName, cmMessenger* messenger, - cmListFileBacktrace const& context) + std::string const& tgtName, cmMakefile const& mf) { std::ostringstream e; const char* modal = nullptr; MessageType messageType = MessageType::AUTHOR_WARNING; - switch (context.GetBottom().GetPolicy(cmPolicies::CMP0026)) { + switch (mf.GetPolicyStatus(cmPolicies::CMP0026)) { case cmPolicies::WARN: e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0026) << "\n"; modal = "should"; @@ -38,7 +36,7 @@ bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( << "\". Use the target name directly with " "add_custom_command, or use the generator expression $, " "as appropriate.\n"; - messenger->IssueMessage(messageType, e.str(), context); + mf.IssueMessage(messageType, e.str()); } return messageType != MessageType::FATAL_ERROR; diff --git a/Source/cmTargetPropertyComputer.h b/Source/cmTargetPropertyComputer.h index e61a1fc..82c6355 100644 --- a/Source/cmTargetPropertyComputer.h +++ b/Source/cmTargetPropertyComputer.h @@ -6,38 +6,35 @@ #include -#include "cmListFileCache.h" #include "cmStateTypes.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmValue.h" -class cmMessenger; +class cmMakefile; class cmTargetPropertyComputer { public: template static cmValue GetProperty(Target const* tgt, const std::string& prop, - cmMessenger* messenger, - cmListFileBacktrace const& context) + cmMakefile const& mf) { - if (cmValue loc = GetLocation(tgt, prop, messenger, context)) { + if (cmValue loc = GetLocation(tgt, prop, mf)) { return loc; } if (cmSystemTools::GetFatalErrorOccured()) { return nullptr; } if (prop == "SOURCES") { - return GetSources(tgt, messenger, context); + return GetSources(tgt, mf); } return nullptr; } private: static bool HandleLocationPropertyPolicy(std::string const& tgtName, - cmMessenger* messenger, - cmListFileBacktrace const& context); + cmMakefile const& mf); template static const std::string& ComputeLocationForBuild(Target const* tgt); @@ -47,8 +44,7 @@ private: template static cmValue GetLocation(Target const* tgt, std::string const& prop, - cmMessenger* messenger, - cmListFileBacktrace const& context) + cmMakefile const& mf) { // Watch for special "computed" properties that are dependent on @@ -61,8 +57,7 @@ private: static const std::string propLOCATION = "LOCATION"; if (prop == propLOCATION) { if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, - context)) { + !HandleLocationPropertyPolicy(tgt->GetName(), mf)) { return nullptr; } return cmValue(ComputeLocationForBuild(tgt)); @@ -71,8 +66,7 @@ private: // Support "LOCATION_". if (cmHasLiteralPrefix(prop, "LOCATION_")) { if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, - context)) { + !HandleLocationPropertyPolicy(tgt->GetName(), mf)) { return nullptr; } std::string configName = prop.substr(9); @@ -85,8 +79,7 @@ private: std::string configName(prop.c_str(), prop.size() - 9); if (configName != "IMPORTED") { if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, - context)) { + !HandleLocationPropertyPolicy(tgt->GetName(), mf)) { return nullptr; } return cmValue(ComputeLocation(tgt, configName)); @@ -97,6 +90,5 @@ private: } template - static cmValue GetSources(Target const* tgt, cmMessenger* messenger, - cmListFileBacktrace const& context); + static cmValue GetSources(Target const* tgt, cmMakefile const& mf); }; diff --git a/Tests/RunCMake/GeneratorExpression/TARGET_PROPERTY-LOCATION-stderr.txt b/Tests/RunCMake/GeneratorExpression/TARGET_PROPERTY-LOCATION-stderr.txt index d4e5b29..a4c8dcd 100644 --- a/Tests/RunCMake/GeneratorExpression/TARGET_PROPERTY-LOCATION-stderr.txt +++ b/Tests/RunCMake/GeneratorExpression/TARGET_PROPERTY-LOCATION-stderr.txt @@ -1,4 +1,4 @@ -CMake Warning \(dev\) at TARGET_PROPERTY-LOCATION.cmake:2 \(add_library\): +CMake Warning \(dev\) in CMakeLists\.txt: Policy CMP0026 is not set: Disallow use of the LOCATION target property. Run "cmake --help-policy CMP0026" for policy details. Use the cmake_policy command to set the policy and suppress this warning. @@ -6,7 +6,3 @@ CMake Warning \(dev\) at TARGET_PROPERTY-LOCATION.cmake:2 \(add_library\): The LOCATION property should not be read from target "foo". Use the target name directly with add_custom_command, or use the generator expression \$, as appropriate. - -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\) -This warning is for project developers. Use -Wno-dev to suppress it. -- cgit v0.12 From 56dc22d48829860b50a441dcc26de14150ad724c Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 7 Dec 2021 16:05:55 -0500 Subject: cmMessenger: Make relative path conversion more explicit Move the decision to convert to call stacks to relative paths out to the client. Avoid using `cmState` to make the decision ourselves. --- Source/cmMessenger.cxx | 26 ++++++++++++++------------ Source/cmMessenger.h | 6 ++++++ Source/cmake.cxx | 6 ++++++ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/Source/cmMessenger.cxx b/Source/cmMessenger.cxx index 52c9dd0..6dd192e 100644 --- a/Source/cmMessenger.cxx +++ b/Source/cmMessenger.cxx @@ -4,8 +4,6 @@ #include "cmDocumentationFormatter.h" #include "cmMessageMetadata.h" -#include "cmState.h" -#include "cmStateSnapshot.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" @@ -14,6 +12,7 @@ #endif #include +#include #include "cmsys/Terminal.h" @@ -154,7 +153,8 @@ static void displayMessage(MessageType t, std::ostringstream& msg) } namespace { -void PrintCallStack(std::ostream& out, cmListFileBacktrace bt) +void PrintCallStack(std::ostream& out, cmListFileBacktrace bt, + cm::optional const& topSource) { // The call stack exists only if we have at least two calls on top // of the bottom. @@ -167,7 +167,6 @@ void PrintCallStack(std::ostream& out, cmListFileBacktrace bt) } bool first = true; - cmStateSnapshot bottom = bt.GetBottom(); for (; !bt.Empty(); bt = bt.Pop()) { cmListFileContext lfc = bt.Top(); if (lfc.Name.empty() && @@ -180,9 +179,8 @@ void PrintCallStack(std::ostream& out, cmListFileBacktrace bt) first = false; out << "Call Stack (most recent call first):\n"; } - if (bottom.GetState()->GetProjectKind() == cmState::ProjectKind::Normal) { - lfc.FilePath = cmSystemTools::RelativeIfUnder( - bottom.GetState()->GetSourceDirectory(), lfc.FilePath); + if (topSource) { + lfc.FilePath = cmSystemTools::RelativeIfUnder(*topSource, lfc.FilePath); } out << " " << lfc << "\n"; } @@ -219,7 +217,7 @@ void cmMessenger::DisplayMessage(MessageType t, const std::string& text, printMessageText(msg, text); // Add the rest of the context. - PrintCallStack(msg, backtrace); + PrintCallStack(msg, backtrace, this->TopSource); displayMessage(t, msg); } @@ -232,10 +230,14 @@ void cmMessenger::PrintBacktraceTitle(std::ostream& out, return; } cmListFileContext lfc = bt.Top(); - cmStateSnapshot bottom = bt.GetBottom(); - if (bottom.GetState()->GetProjectKind() == cmState::ProjectKind::Normal) { - lfc.FilePath = cmSystemTools::RelativeIfUnder( - bottom.GetState()->GetSourceDirectory(), lfc.FilePath); + if (this->TopSource) { + lfc.FilePath = + cmSystemTools::RelativeIfUnder(*this->TopSource, lfc.FilePath); } out << (lfc.Line ? " at " : " in ") << lfc; } + +void cmMessenger::SetTopSource(cm::optional topSource) +{ + this->TopSource = std::move(topSource); +} diff --git a/Source/cmMessenger.h b/Source/cmMessenger.h index 8287e70..451add0 100644 --- a/Source/cmMessenger.h +++ b/Source/cmMessenger.h @@ -7,6 +7,8 @@ #include #include +#include + #include "cmListFileCache.h" #include "cmMessageType.h" @@ -20,6 +22,8 @@ public: void DisplayMessage(MessageType t, std::string const& text, cmListFileBacktrace const& backtrace) const; + void SetTopSource(cm::optional topSource); + void SetSuppressDevWarnings(bool suppress) { this->SuppressDevWarnings = suppress; @@ -56,6 +60,8 @@ private: bool IsMessageTypeVisible(MessageType t) const; MessageType ConvertMessageType(MessageType t) const; + cm::optional TopSource; + bool SuppressDevWarnings = false; bool SuppressDeprecatedWarnings = false; bool DevWarningsAsErrors = false; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index d927d27..9efdb54 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -1706,6 +1706,12 @@ void cmake::SetHomeDirectory(const std::string& dir) if (this->CurrentSnapshot.IsValid()) { this->CurrentSnapshot.SetDefinition("CMAKE_SOURCE_DIR", dir); } + + if (this->State->GetProjectKind() == cmState::ProjectKind::Normal) { + this->Messenger->SetTopSource(this->GetHomeDirectory()); + } else { + this->Messenger->SetTopSource(cm::nullopt); + } } std::string const& cmake::GetHomeDirectory() const -- cgit v0.12 From 7b677dbb9279a575ec6b5f79daa78acfec241b6a Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 7 Dec 2021 16:26:58 -0500 Subject: cmListFileBacktrace: Remove unused "bottom" entry All uses of `GetBottom` by clients have been removed, so drop the method and its supporting infrastructure. --- Source/CTest/cmCTestTestHandler.cxx | 2 +- Source/cmListFileCache.cxx | 50 +++---------------------------------- Source/cmListFileCache.h | 13 +--------- Source/cmMakefile.cxx | 1 - 4 files changed, 6 insertions(+), 60 deletions(-) diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index 02db0c6..75777ab 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -2183,7 +2183,7 @@ bool cmCTestTestHandler::SetTestsProperties( // Ensure we have complete triples otherwise the data is corrupt. if (triples.size() % 3 == 0) { cmState state(cmState::Unknown); - rt.Backtrace = cmListFileBacktrace(state.CreateBaseSnapshot()); + rt.Backtrace = cmListFileBacktrace(); // the first entry represents the top of the trace so we need to // reconstruct the backtrace in reverse diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index 4c434a3..c167db5 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -443,45 +443,19 @@ cm::optional cmListFileParser::CheckNesting() const return cm::nullopt; } -// We hold either the bottom scope of a directory or a call/file context. -// Discriminate these cases via the parent pointer. +// We hold a call/file context. struct cmListFileBacktrace::Entry { - Entry(cmStateSnapshot bottom) - : Bottom(bottom) - { - } - Entry(std::shared_ptr parent, cmListFileContext lfc) : Context(std::move(lfc)) , Parent(std::move(parent)) { } - ~Entry() - { - if (this->Parent) { - this->Context.~cmListFileContext(); - } else { - this->Bottom.~cmStateSnapshot(); - } - } - - bool IsBottom() const { return !this->Parent; } - - union - { - cmStateSnapshot Bottom; - cmListFileContext Context; - }; + cmListFileContext Context; std::shared_ptr Parent; }; -cmListFileBacktrace::cmListFileBacktrace(cmStateSnapshot const& snapshot) - : TopEntry(std::make_shared(snapshot.GetCallStackBottom())) -{ -} - /* NOLINTNEXTLINE(performance-unnecessary-value-param) */ cmListFileBacktrace::cmListFileBacktrace(std::shared_ptr parent, cmListFileContext const& lfc) @@ -494,18 +468,6 @@ cmListFileBacktrace::cmListFileBacktrace(std::shared_ptr top) { } -cmStateSnapshot cmListFileBacktrace::GetBottom() const -{ - cmStateSnapshot bottom; - if (Entry const* cur = this->TopEntry.get()) { - while (Entry const* parent = cur->Parent.get()) { - cur = parent; - } - bottom = cur->Bottom; - } - return bottom; -} - cmListFileBacktrace cmListFileBacktrace::Push(std::string const& file) const { // We are entering a file-level scope but have not yet reached @@ -520,22 +482,18 @@ cmListFileBacktrace cmListFileBacktrace::Push(std::string const& file) const cmListFileBacktrace cmListFileBacktrace::Push( cmListFileContext const& lfc) const { - assert(this->TopEntry); - assert(!this->TopEntry->IsBottom() || this->TopEntry->Bottom.IsValid()); return cmListFileBacktrace(this->TopEntry, lfc); } cmListFileBacktrace cmListFileBacktrace::Pop() const { assert(this->TopEntry); - assert(!this->TopEntry->IsBottom()); return cmListFileBacktrace(this->TopEntry->Parent); } cmListFileContext const& cmListFileBacktrace::Top() const { assert(this->TopEntry); - assert(!this->TopEntry->IsBottom()); return this->TopEntry->Context; } @@ -543,7 +501,7 @@ size_t cmListFileBacktrace::Depth() const { size_t depth = 0; if (Entry const* cur = this->TopEntry.get()) { - for (; !cur->IsBottom(); cur = cur->Parent.get()) { + for (; cur; cur = cur->Parent.get()) { ++depth; } } @@ -552,7 +510,7 @@ size_t cmListFileBacktrace::Depth() const bool cmListFileBacktrace::Empty() const { - return !this->TopEntry || this->TopEntry->IsBottom(); + return !this->TopEntry; } std::ostream& operator<<(std::ostream& os, cmListFileContext const& lfc) diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index 0e2e299..aaf672c 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -13,7 +13,6 @@ #include -#include "cmStateSnapshot.h" #include "cmSystemTools.h" /** \class cmListFileCache @@ -164,23 +163,13 @@ private: class cmListFileBacktrace { public: - // Default-constructed backtrace may not be used until after - // set via assignment from a backtrace constructed with a - // valid snapshot. + // Default-constructed backtrace is empty. cmListFileBacktrace() = default; - // Construct an empty backtrace whose bottom sits in the directory - // indicated by the given valid snapshot. - cmListFileBacktrace(cmStateSnapshot const& snapshot); - - cmStateSnapshot GetBottom() const; - // Get a backtrace with the given file scope added to the top. - // May not be called until after construction with a valid snapshot. cmListFileBacktrace Push(std::string const& file) const; // Get a backtrace with the given call context added to the top. - // May not be called until after construction with a valid snapshot. cmListFileBacktrace Push(cmListFileContext const& lfc) const; // Get a backtrace with the top level removed. diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 661cb05..950fa7b 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -79,7 +79,6 @@ cmMakefile::cmMakefile(cmGlobalGenerator* globalGenerator, cmStateSnapshot const& snapshot) : GlobalGenerator(globalGenerator) , StateSnapshot(snapshot) - , Backtrace(snapshot) { this->IsSourceFileTryCompile = false; -- cgit v0.12 From d0ceb409ff2d9eecb992e7d37780137f9330a748 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 7 Dec 2021 17:22:45 -0500 Subject: cmListFileBacktrace: Remove unused "Depth" method --- Source/cmListFileCache.cxx | 11 ----------- Source/cmListFileCache.h | 4 ---- 2 files changed, 15 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index c167db5..9f8a18f 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -497,17 +497,6 @@ cmListFileContext const& cmListFileBacktrace::Top() const return this->TopEntry->Context; } -size_t cmListFileBacktrace::Depth() const -{ - size_t depth = 0; - if (Entry const* cur = this->TopEntry.get()) { - for (; cur; cur = cur->Parent.get()) { - ++depth; - } - } - return depth; -} - bool cmListFileBacktrace::Empty() const { return !this->TopEntry; diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index aaf672c..65b4772 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -4,7 +4,6 @@ #include "cmConfigure.h" // IWYU pragma: keep -#include #include #include #include @@ -180,9 +179,6 @@ public: // This may be called only if Empty() would return false. cmListFileContext const& Top() const; - // Get the number of 'frames' in this backtrace - size_t Depth() const; - // Return true if this backtrace is empty. bool Empty() const; -- cgit v0.12 From 61c46c95aecd09e335dfae7c3123053c3e2ef381 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 7 Dec 2021 17:35:44 -0500 Subject: cmListFileContext: Simplify explicit rule-of-five members --- Source/cmListFileCache.h | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index 65b4772..4a52876 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -80,6 +80,18 @@ public: cm::optional DeferId; cmListFileContext() = default; + cmListFileContext(cmListFileContext&& /*other*/) = default; + cmListFileContext(const cmListFileContext& /*other*/) = default; + cmListFileContext& operator=(const cmListFileContext& /*other*/) = default; +#if __cplusplus >= 201703L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201703L) + cmListFileContext& operator=(cmListFileContext&& /*other*/) = default; +#else + // The move assignment operators for several STL classes did not become + // noexcept until C++17, which causes some tools to warn about this move + // assignment operator throwing an exception when it shouldn't. + cmListFileContext& operator=(cmListFileContext&& /*other*/) = delete; +#endif + cmListFileContext(std::string name, std::string filePath, long line) : Name(std::move(name)) , FilePath(std::move(filePath)) @@ -87,14 +99,6 @@ public: { } -#if __cplusplus < 201703L && (!defined(_MSVC_LANG) || _MSVC_LANG < 201703L) - cmListFileContext(const cmListFileContext& /*other*/) = default; - cmListFileContext(cmListFileContext&& /*other*/) = default; - - cmListFileContext& operator=(const cmListFileContext& /*other*/) = default; - cmListFileContext& operator=(cmListFileContext&& /*other*/) = delete; -#endif - static cmListFileContext FromCommandContext( cmCommandContext const& lfcc, std::string const& fileName, cm::optional deferId = {}) -- cgit v0.12