diff --git a/Help/manual/cmake-generator-expressions.7.rst b/Help/manual/cmake-generator-expressions.7.rst index 26a4a60e2c..a6da67d8cf 100644 --- a/Help/manual/cmake-generator-expressions.7.rst +++ b/Help/manual/cmake-generator-expressions.7.rst @@ -1877,6 +1877,14 @@ These expressions look up the values of rather than the directory of the consuming target for which the expression is being evaluated. + .. versionchanged:: 3.31 + Generator expressions for transitive interface properties, such as + ``$``, now correctly handle + repeated evaluations within nested generator expressions. + Previously, these repeated evaluations returned empty values due + to an optimization for transitive closures. + This change ensures consistent evaluation for non-union operations. + .. genex:: $ :target: TARGET_PROPERTY:prop diff --git a/Source/cmGeneratorExpressionDAGChecker.cxx b/Source/cmGeneratorExpressionDAGChecker.cxx index aad25f032d..92a76dc708 100644 --- a/Source/cmGeneratorExpressionDAGChecker.cxx +++ b/Source/cmGeneratorExpressionDAGChecker.cxx @@ -24,7 +24,7 @@ cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( std::string const& contextConfig) : cmGeneratorExpressionDAGChecker(cmListFileBacktrace(), target, std::move(property), content, parent, - contextLG, contextConfig) + contextLG, contextConfig, INHERIT) { } @@ -33,8 +33,20 @@ cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( std::string property, const GeneratorExpressionContent* content, cmGeneratorExpressionDAGChecker* parent, cmLocalGenerator const* contextLG, std::string const& contextConfig) + : cmGeneratorExpressionDAGChecker(std::move(backtrace), target, + std::move(property), content, parent, + contextLG, contextConfig, INHERIT) +{ +} + +cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( + cmListFileBacktrace backtrace, cmGeneratorTarget const* target, + std::string property, const GeneratorExpressionContent* content, + cmGeneratorExpressionDAGChecker* parent, cmLocalGenerator const* contextLG, + std::string const& contextConfig, TransitiveClosure closure) : Parent(parent) , Top(parent ? parent->Top : this) + , Closure((closure == SUBGRAPH || !parent) ? this : parent->Closure) , Target(target) , Property(std::move(property)) , Content(content) @@ -53,16 +65,16 @@ cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( this->CheckResult = this->CheckGraph(); if (this->CheckResult == DAG && this->EvaluatingTransitiveProperty()) { - const auto* top = this->Top; - auto it = top->Seen.find(this->Target); - if (it != top->Seen.end()) { + const auto* transitiveClosure = this->Closure; + auto it = transitiveClosure->Seen.find(this->Target); + if (it != transitiveClosure->Seen.end()) { const std::set& propSet = it->second; if (propSet.find(this->Property) != propSet.end()) { this->CheckResult = ALREADY_SEEN; return; } } - top->Seen[this->Target].insert(this->Property); + transitiveClosure->Seen[this->Target].insert(this->Property); } } diff --git a/Source/cmGeneratorExpressionDAGChecker.h b/Source/cmGeneratorExpressionDAGChecker.h index 8b0eea76b4..03f6b399aa 100644 --- a/Source/cmGeneratorExpressionDAGChecker.h +++ b/Source/cmGeneratorExpressionDAGChecker.h @@ -17,6 +17,17 @@ class cmLocalGenerator; struct cmGeneratorExpressionDAGChecker { + enum TransitiveClosure + { + INHERIT, + SUBGRAPH, + }; + + cmGeneratorExpressionDAGChecker( + cmListFileBacktrace backtrace, cmGeneratorTarget const* target, + std::string property, const GeneratorExpressionContent* content, + cmGeneratorExpressionDAGChecker* parent, cmLocalGenerator const* contextLG, + std::string const& contextConfig, TransitiveClosure closure); cmGeneratorExpressionDAGChecker(cmListFileBacktrace backtrace, cmGeneratorTarget const* target, std::string property, @@ -76,6 +87,7 @@ private: const cmGeneratorExpressionDAGChecker* const Parent; const cmGeneratorExpressionDAGChecker* const Top; + const cmGeneratorExpressionDAGChecker* const Closure; cmGeneratorTarget const* Target; const std::string Property; mutable std::map> Seen; diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index 84521cadde..918dc288fe 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -2983,8 +2983,9 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode if (isInterfaceProperty) { return cmGeneratorExpression::StripEmptyListElements( - target->EvaluateInterfaceProperty(propertyName, context, - dagCheckerParent, usage)); + target->EvaluateInterfaceProperty( + propertyName, context, dagCheckerParent, usage, + cmGeneratorTarget::TransitiveClosure::Subgraph)); } cmGeneratorExpressionDAGChecker dagChecker( diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 85e69113c8..1068c8977f 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -918,11 +918,23 @@ public: const std::string& report, const std::string& compatibilityType) const; + /** Configures TransitiveClosureOptimization. Re-evaluation of a + transitive property will only be optimized within a subgraph. */ + enum class TransitiveClosure + { + Inherit, // node is in the same subgraph as its' parent + Subgraph, // the current node spans a new subgraph + }; + class TargetPropertyEntry; std::string EvaluateInterfaceProperty( std::string const& prop, cmGeneratorExpressionContext* context, cmGeneratorExpressionDAGChecker* dagCheckerParent, UseTo usage) const; + std::string EvaluateInterfaceProperty( + std::string const& prop, cmGeneratorExpressionContext* context, + cmGeneratorExpressionDAGChecker* dagCheckerParent, UseTo usage, + TransitiveClosure closure) const; struct TransitiveProperty { diff --git a/Source/cmGeneratorTarget_TransitiveProperty.cxx b/Source/cmGeneratorTarget_TransitiveProperty.cxx index ac929ebc6e..8c2b8a96ba 100644 --- a/Source/cmGeneratorTarget_TransitiveProperty.cxx +++ b/Source/cmGeneratorTarget_TransitiveProperty.cxx @@ -98,6 +98,15 @@ bool cmGeneratorTarget::MaybeHaveInterfaceProperty( std::string cmGeneratorTarget::EvaluateInterfaceProperty( std::string const& prop, cmGeneratorExpressionContext* context, cmGeneratorExpressionDAGChecker* dagCheckerParent, UseTo usage) const +{ + return EvaluateInterfaceProperty(prop, context, dagCheckerParent, usage, + TransitiveClosure::Inherit); +} + +std::string cmGeneratorTarget::EvaluateInterfaceProperty( + std::string const& prop, cmGeneratorExpressionContext* context, + cmGeneratorExpressionDAGChecker* dagCheckerParent, UseTo usage, + TransitiveClosure closure) const { std::string result; @@ -106,12 +115,15 @@ std::string cmGeneratorTarget::EvaluateInterfaceProperty( return result; } + auto const dagClosure = closure == TransitiveClosure::Inherit + ? cmGeneratorExpressionDAGChecker::INHERIT + : cmGeneratorExpressionDAGChecker::SUBGRAPH; // Evaluate $ as if it were compiled. This is // a subset of TargetPropertyNode::Evaluate without stringify/parse steps // but sufficient for transitive interface properties. cmGeneratorExpressionDAGChecker dagChecker( context->Backtrace, this, prop, nullptr, dagCheckerParent, - this->LocalGenerator, context->Config); + this->LocalGenerator, context->Config, dagClosure); switch (dagChecker.Check()) { case cmGeneratorExpressionDAGChecker::SELF_REFERENCE: dagChecker.ReportError( diff --git a/Tests/GeneratorExpression/CMakeLists.txt b/Tests/GeneratorExpression/CMakeLists.txt index be750e1809..b0064ae257 100644 --- a/Tests/GeneratorExpression/CMakeLists.txt +++ b/Tests/GeneratorExpression/CMakeLists.txt @@ -112,6 +112,15 @@ target_link_libraries(empty3 LINK_PUBLIC empty2 empty4) add_library(empty5 empty.cpp) target_include_directories(empty5 PRIVATE /empty5/private1 /empty5/private2) +add_library(interface1 INTERFACE) +target_sources(interface1 INTERFACE foo.c bar.cpp) + +set(interface1Sources $) +set(interface1MergeSources $,$>) + +add_library(interface2 INTERFACE) +target_sources(interface2 INTERFACE ${interface1MergeSources}) + add_custom_target(check-part2 ALL COMMAND ${msys2_no_conv} ${CMAKE_COMMAND} -Dtest_incomplete_1=$< @@ -147,6 +156,8 @@ add_custom_target(check-part2 ALL -Dtest_target_includes6=$ -Dtest_target_includes7=$ -Dtest_target_includes8=$ + -Dtest_target_closure1=$,$> + -Dtest_target_closure2=$>,$> -Dtest_arbitrary_content_comma_1=$<1:a,> -Dtest_arbitrary_content_comma_2=$<1:,a> -Dtest_arbitrary_content_comma_3=$<1:a,,> diff --git a/Tests/GeneratorExpression/check-part2.cmake b/Tests/GeneratorExpression/check-part2.cmake index a1db5f63d5..e9c5e5c06f 100644 --- a/Tests/GeneratorExpression/check-part2.cmake +++ b/Tests/GeneratorExpression/check-part2.cmake @@ -34,6 +34,8 @@ check(test_target_includes5 "/empty2/public;/empty3/public;/empty2/public;/empty check(test_target_includes6 "/empty3/public;/empty3/private;/empty2/public;/empty3/public;/empty4/public") check(test_target_includes7 "/empty1/public;/empty2/public;/empty3/public;/empty4/public") check(test_target_includes8 "/empty5/private1;/empty5/private2") +check(test_target_closure1 "bar.cpp,foo.c") +check(test_target_closure2 "bar.cpp,foo.c") check(test_arbitrary_content_comma_1 "a,") check(test_arbitrary_content_comma_2 ",a") check(test_arbitrary_content_comma_3 "a,,")