From 8b5af40b34f846c0a0d7d7e7e851db9179442f06 Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 22 Feb 2025 07:39:04 -0500 Subject: GenEx: Fix evaluation of LINK_LIBRARIES as custom transitive property Fix logic from commit b9ee79b8a1 (GenEx: Add support for custom transitive compile properties, 2024-05-09, v3.30.0-rc1~82^2~1) to more precisely know when we are computing the link dependency graph. Issue: #20416 Issue: #26709 --- Source/cmGeneratorExpressionDAGChecker.cxx | 12 +++++++++--- Source/cmGeneratorExpressionDAGChecker.h | 16 +++++++++++++++- Source/cmGeneratorExpressionNode.cxx | 3 +-- Source/cmGeneratorTarget.h | 3 ++- Source/cmGeneratorTarget_Link.cxx | 18 ++++++++++++++++-- Source/cmGeneratorTarget_TransitiveProperty.cxx | 9 ++++----- Tests/CustomTransitiveProperties/CMakeLists.txt | 8 +++----- Tests/CustomTransitiveProperties/check.cmake | 6 +++--- 8 files changed, 53 insertions(+), 22 deletions(-) diff --git a/Source/cmGeneratorExpressionDAGChecker.cxx b/Source/cmGeneratorExpressionDAGChecker.cxx index 8e28b42..c275ec6 100644 --- a/Source/cmGeneratorExpressionDAGChecker.cxx +++ b/Source/cmGeneratorExpressionDAGChecker.cxx @@ -21,21 +21,22 @@ cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( cmGeneratorTarget const* target, std::string property, GeneratorExpressionContent const* content, cmGeneratorExpressionDAGChecker* parent, cmLocalGenerator const* contextLG, - std::string const& contextConfig, cmListFileBacktrace backtrace) + std::string const& contextConfig, cmListFileBacktrace backtrace, + ComputingLinkLibraries computingLinkLibraries) : Parent(parent) , Top(parent ? parent->Top : this) , Target(target) , Property(std::move(property)) , Content(content) , Backtrace(std::move(backtrace)) + , ComputingLinkLibraries_(computingLinkLibraries) { if (parent) { this->TopIsTransitiveProperty = parent->TopIsTransitiveProperty; } else { this->TopIsTransitiveProperty = this->Target - ->IsTransitiveProperty(this->Property, contextLG, contextConfig, - this->EvaluatingLinkLibraries()) + ->IsTransitiveProperty(this->Property, contextLG, contextConfig, this) .has_value(); } @@ -193,6 +194,11 @@ bool cmGeneratorExpressionDAGChecker::EvaluatingLinkerLauncher() const "_LINKER_LAUNCHER"_s; } +bool cmGeneratorExpressionDAGChecker::IsComputingLinkLibraries() const +{ + return this->Top->ComputingLinkLibraries_ == ComputingLinkLibraries::Yes; +} + bool cmGeneratorExpressionDAGChecker::EvaluatingLinkLibraries( cmGeneratorTarget const* tgt, ForGenex genex) const { diff --git a/Source/cmGeneratorExpressionDAGChecker.h b/Source/cmGeneratorExpressionDAGChecker.h index 5b4e6d6..064804f 100644 --- a/Source/cmGeneratorExpressionDAGChecker.h +++ b/Source/cmGeneratorExpressionDAGChecker.h @@ -17,12 +17,19 @@ class cmLocalGenerator; struct cmGeneratorExpressionDAGChecker { + enum class ComputingLinkLibraries + { + No, + Yes, + }; cmGeneratorExpressionDAGChecker( cmGeneratorTarget const* target, std::string property, GeneratorExpressionContent const* content, cmGeneratorExpressionDAGChecker* parent, cmLocalGenerator const* contextLG, std::string const& contextConfig, - cmListFileBacktrace backtrace = cmListFileBacktrace()); + cmListFileBacktrace backtrace = cmListFileBacktrace(), + ComputingLinkLibraries computingLinkLibraries = + ComputingLinkLibraries::No); enum Result { @@ -45,6 +52,11 @@ struct cmGeneratorExpressionDAGChecker bool EvaluatingLinkOptionsExpression() const; bool EvaluatingLinkerLauncher() const; + /** Returns true only when computing the actual link dependency + graph for cmGeneratorTarget::GetLinkImplementationLibraries + or cmGeneratorTarget::GetLinkInterfaceLibraries. */ + bool IsComputingLinkLibraries() const; + enum class ForGenex { ANY, @@ -78,4 +90,6 @@ private: bool TransitivePropertiesOnly = false; bool CMP0131 = false; bool TopIsTransitiveProperty = false; + ComputingLinkLibraries const ComputingLinkLibraries_ = + ComputingLinkLibraries::No; }; diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index 8bd263d..0713bc7 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -2957,8 +2957,7 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode if (cm::optional transitiveProp = target->IsTransitiveProperty(propertyName, context->LG, - context->Config, - evaluatingLinkLibraries)) { + context->Config, dagCheckerParent)) { interfacePropertyName = std::string(transitiveProp->InterfaceName); isInterfaceProperty = transitiveProp->InterfaceName == propertyName; usage = transitiveProp->Usage; diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 1e6ff78..0a92252 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -935,7 +935,8 @@ public: cm::optional IsTransitiveProperty( cm::string_view prop, cmLocalGenerator const* lg, - std::string const& config, bool evaluatingLinkLibraries) const; + std::string const& config, + cmGeneratorExpressionDAGChecker const* dagChecker) const; bool HaveInstallTreeRPATH(const std::string& config) const; diff --git a/Source/cmGeneratorTarget_Link.cxx b/Source/cmGeneratorTarget_Link.cxx index e1db657..74e77de 100644 --- a/Source/cmGeneratorTarget_Link.cxx +++ b/Source/cmGeneratorTarget_Link.cxx @@ -565,7 +565,14 @@ void cmGeneratorTarget::ExpandLinkItems(std::string const& prop, } // Keep this logic in sync with ComputeLinkImplementationLibraries. cmGeneratorExpressionDAGChecker dagChecker{ - this, prop, nullptr, nullptr, this->LocalGenerator, config, + this, + prop, + nullptr, + nullptr, + this->LocalGenerator, + config, + cmListFileBacktrace(), + cmGeneratorExpressionDAGChecker::ComputingLinkLibraries::Yes, }; // The $ expression may be in a link interface to specify // private link dependencies that are otherwise excluded from usage @@ -1322,7 +1329,14 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries( for (auto const& entry : entryRange) { // Keep this logic in sync with ExpandLinkItems. cmGeneratorExpressionDAGChecker dagChecker{ - this, "LINK_LIBRARIES", nullptr, nullptr, this->LocalGenerator, config, + this, + "LINK_LIBRARIES", + nullptr, + nullptr, + this->LocalGenerator, + config, + cmListFileBacktrace(), + cmGeneratorExpressionDAGChecker::ComputingLinkLibraries::Yes, }; // The $ expression may be used to specify link dependencies // that are otherwise excluded from usage requirements. diff --git a/Source/cmGeneratorTarget_TransitiveProperty.cxx b/Source/cmGeneratorTarget_TransitiveProperty.cxx index 19300cc..7c197fc 100644 --- a/Source/cmGeneratorTarget_TransitiveProperty.cxx +++ b/Source/cmGeneratorTarget_TransitiveProperty.cxx @@ -183,10 +183,9 @@ std::string cmGeneratorTarget::EvaluateInterfaceProperty( } cm::optional -cmGeneratorTarget::IsTransitiveProperty(cm::string_view prop, - cmLocalGenerator const* lg, - std::string const& config, - bool evaluatingLinkLibraries) const +cmGeneratorTarget::IsTransitiveProperty( + cm::string_view prop, cmLocalGenerator const* lg, std::string const& config, + cmGeneratorExpressionDAGChecker const* dagChecker) const { cm::optional result; static const cm::string_view kINTERFACE_ = "INTERFACE_"_s; @@ -215,7 +214,7 @@ cmGeneratorTarget::IsTransitiveProperty(cm::string_view prop, result = TransitiveProperty{ "INTERFACE_COMPILE_DEFINITIONS"_s, UseTo::Compile }; } - } else if (!evaluatingLinkLibraries) { + } else if (!dagChecker || !dagChecker->IsComputingLinkLibraries()) { // Honor TRANSITIVE_COMPILE_PROPERTIES and TRANSITIVE_LINK_PROPERTIES // from the link closure when we are not evaluating the closure itself. CustomTransitiveProperties const& ctp = diff --git a/Tests/CustomTransitiveProperties/CMakeLists.txt b/Tests/CustomTransitiveProperties/CMakeLists.txt index 128687f..a9ac2b8 100644 --- a/Tests/CustomTransitiveProperties/CMakeLists.txt +++ b/Tests/CustomTransitiveProperties/CMakeLists.txt @@ -200,14 +200,12 @@ add_custom_target(check ALL VERBATIM # / \ # "static10[iface11];iface11[iface10]" "$" "iface11;iface10" - "$" "static10;iface11;iface11;iface10;iface10" - # / / \ \ \___ extra! + "$" "static10;iface11;iface11;iface10" # __/ __/ \__ \__________ # / / \ \ # "static11[static10;iface11];static10[iface11;iface11[iface10]]" - "$" "static10;iface11;iface11;iface10;iface10" - "$" "static11;static10;static10;iface11;iface11;iface10;iface10;iface11;iface10" - # / / | | \ \ \_______\_______\____ extra! + "$" "static10;iface11;iface11;iface10" + "$" "static11;static10;static10;iface11;iface11;iface10" # _______/ _______/ | | \______ \______________ # / / | | \ \ # "main10[static11;static10];static11[static10;iface11;static10[iface11;iface11[iface10]]]" diff --git a/Tests/CustomTransitiveProperties/check.cmake b/Tests/CustomTransitiveProperties/check.cmake index 31a5c9f..0798542 100644 --- a/Tests/CustomTransitiveProperties/check.cmake +++ b/Tests/CustomTransitiveProperties/check.cmake @@ -47,9 +47,9 @@ iface11 LINK_LIBRARIES: '' iface11 INTERFACE_LINK_LIBRARIES: 'iface10' static10 LINK_LIBRARIES: 'iface11;iface10' static10 INTERFACE_LINK_LIBRARIES: 'iface11;iface10' -static11 LINK_LIBRARIES: 'static10;iface11;iface11;iface10;iface10' -static11 INTERFACE_LINK_LIBRARIES: 'static10;iface11;iface11;iface10;iface10' -main10 LINK_LIBRARIES: 'static11;static10;static10;iface11;iface11;iface10;iface10;iface11;iface10' +static11 LINK_LIBRARIES: 'static10;iface11;iface11;iface10' +static11 INTERFACE_LINK_LIBRARIES: 'static10;iface11;iface11;iface10' +main10 LINK_LIBRARIES: 'static11;static10;static10;iface11;iface11;iface10' main10 INTERFACE_LINK_LIBRARIES: '' ]]) string(REGEX REPLACE "\r\n" "\n" expect "${expect}") -- cgit v0.12