GenEx: Revert "Limit TARGET_PROPERTY transitive closure optimization"

Revert commit 4a11772618 (GenEx: Limit TARGET_PROPERTY transitive
closure optimization to subgraphs, 2024-05-31, v3.31.0-rc1~114^2).
The change caused substantial performance regressions in some
existing use cases.  Revert it pending further investigation.

Issue: #25728
Fixes: #26457

Help/manual/cmake-generator-expressions.7.rst

@@ -1877,14 +1877,6 @@ 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, INHERIT)
+                                    contextLG, contextConfig)
 {
 }
 
@@ -33,20 +33,8 @@ 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)
@@ -65,16 +53,16 @@ cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker(
   this->CheckResult = this->CheckGraph();
 
   if (this->CheckResult == DAG && this->EvaluatingTransitiveProperty()) {
-    const auto* transitiveClosure = this->Closure;
-    auto it = transitiveClosure->Seen.find(this->Target);
-    if (it != transitiveClosure->Seen.end()) {
+    const auto* top = this->Top;
+    auto it = top->Seen.find(this->Target);
+    if (it != top->Seen.end()) {
       const std::set<std::string>& propSet = it->second;
       if (propSet.find(this->Property) != propSet.end()) {
         this->CheckResult = ALREADY_SEEN;
         return;
       }
     }
-    transitiveClosure->Seen[this->Target].insert(this->Property);
+    top->Seen[this->Target].insert(this->Property);
   }
 }
 

Source/cmGeneratorExpressionDAGChecker.h

@@ -17,17 +17,6 @@ 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,
@@ -87,7 +76,6 @@ 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,9 +2983,8 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
 
     if (isInterfaceProperty) {
       return cmGeneratorExpression::StripEmptyListElements(
-        target->EvaluateInterfaceProperty(
-          propertyName, context, dagCheckerParent, usage,
-          cmGeneratorTarget::TransitiveClosure::Subgraph));
+        target->EvaluateInterfaceProperty(propertyName, context,
+                                          dagCheckerParent, usage));
     }
 
     cmGeneratorExpressionDAGChecker dagChecker(

Source/cmGeneratorTarget.h

@@ -977,23 +977,11 @@ 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,15 +98,6 @@ 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;
 
@@ -115,15 +106,12 @@ 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, dagClosure);
+    this->LocalGenerator, context->Config);
   switch (dagChecker.Check()) {
     case cmGeneratorExpressionDAGChecker::SELF_REFERENCE:
       dagChecker.ReportError(

Tests/GeneratorExpression/check-part2.cmake

@@ -35,7 +35,7 @@ check(test_target_includes6 "/empty3/public;/empty3/private;/empty2/public;/empt
 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_target_closure2 "foo.c") # FIXME(#25728): This should be "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,,")