GenEx: Limit TARGET_PROPERTY transitive closure optimization to subgraphs

Fixes: #25728

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
+    ``$<TARGET_PROPERTY:target,INTERFACE_*>``, 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_PROPERTY:prop>
   :target: TARGET_PROPERTY:prop
 

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<std::string>& 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);
   }
 }
 

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<cmGeneratorTarget const*, std::set<std::string>> Seen;

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(

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
   {

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 $<TARGET_PROPERTY:this,prop> 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(

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 $<TARGET_PROPERTY:interface1,INTERFACE_SOURCES>)
+set(interface1MergeSources $<LIST:APPEND,$<LIST:FILTER,${interface1Sources},INCLUDE,.*\\.c$>,$<LIST:FILTER,${interface1Sources},EXCLUDE,.*\\.c$>>)
+
+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=$<TARGET_PROPERTY:empty3,INCLUDE_DIRECTORIES>
     -Dtest_target_includes7=$<TARGET_PROPERTY:empty1,INTERFACE_INCLUDE_DIRECTORIES>
     -Dtest_target_includes8=$<TARGET_PROPERTY:empty5,INCLUDE_DIRECTORIES>
+    -Dtest_target_closure1=$<JOIN:$<LIST:SORT,${interface1MergeSources}>,$<COMMA>>
+    -Dtest_target_closure2=$<JOIN:$<LIST:SORT,$<TARGET_PROPERTY:interface2,INTERFACE_SOURCES>>,$<COMMA>>
     -Dtest_arbitrary_content_comma_1=$<1:a,>
     -Dtest_arbitrary_content_comma_2=$<1:,a>
     -Dtest_arbitrary_content_comma_3=$<1:a,,>

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,,")