Merge topic 'optimize-usage-requirements'

1d3841b600 Genex: Memoize usage requirement TARGET_PROPERTY existence
b5460f9931 cmLinkItem: Expose HadHeadSensitiveCondition in cmLinkInterfaceLibraries
ad2b3a32d1 Genex: Optimize build setting TARGET_PROPERTY evaluation
11fa818ecd Genex: Optimize usage requirement TARGET_PROPERTY recursion
0239bf8ac8 Genex: In TARGET_PROPERTY check for usage reqs in link libs earlier
7caebeb0e4 Genex: Re-order TARGET_PROPERTY logic to de-duplicate checks
b2785a0fbd Genex: Move TARGET_PROPERTY linked targets evaluation to end

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Cristian Adam <cristian.adam@gmail.com>
Merge-request: !3589

4 files changed, 231 insertions, 137 deletions

diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx
index d828dac..49d0c47 100644
--- a/Source/cmGeneratorExpressionNode.cxx
+++ b/Source/cmGeneratorExpressionNode.cxx
@@ -1038,45 +1038,38 @@ static const struct CompileLanguageAndIdNode : public cmGeneratorExpressionNode
   }
 } languageAndIdNode;
 
-#define TRANSITIVE_PROPERTY_NAME(PROPERTY) , "INTERFACE_" #PROPERTY
-
-static const char* targetPropertyTransitiveWhitelist[] = {
-  nullptr CM_FOR_EACH_TRANSITIVE_PROPERTY_NAME(TRANSITIVE_PROPERTY_NAME)
-};
-
-#undef TRANSITIVE_PROPERTY_NAME
-
-template <typename T>
 std::string getLinkedTargetsContent(
-  std::vector<T> const& libraries, cmGeneratorTarget const* target,
-  cmGeneratorTarget const* headTarget, cmGeneratorExpressionContext* context,
-  cmGeneratorExpressionDAGChecker* dagChecker,
-  const std::string& interfacePropertyName)
-{
-  std::string linkedTargetsContent;
-  std::string sep;
-  std::string depString;
-  for (T const& l : libraries) {
-    // Broken code can have a target in its own link interface.
-    // Don't follow such link interface entries so as not to create a
-    // self-referencing loop.
-    if (l.Target && l.Target != target) {
-      std::string uniqueName =
-        target->GetGlobalGenerator()->IndexGeneratorTargetUniquely(l.Target);
-      depString += sep + "$<TARGET_PROPERTY:" + std::move(uniqueName) + "," +
-        interfacePropertyName + ">";
-      sep = ";";
-    }
-  }
-  if (!depString.empty()) {
-    linkedTargetsContent =
-      cmGeneratorExpressionNode::EvaluateDependentExpression(
-        depString, target->GetLocalGenerator(), context, headTarget, target,
-        dagChecker);
-  }
-  linkedTargetsContent =
-    cmGeneratorExpression::StripEmptyListElements(linkedTargetsContent);
-  return linkedTargetsContent;
+  cmGeneratorTarget const* target, std::string const& prop,
+  cmGeneratorExpressionContext* context,
+  cmGeneratorExpressionDAGChecker* dagChecker)
+{
+  std::string result;
+  if (cmLinkImplementationLibraries const* impl =
+        target->GetLinkImplementationLibraries(context->Config)) {
+    for (cmLinkImplItem const& lib : impl->Libraries) {
+      if (lib.Target) {
+        // Pretend $<TARGET_PROPERTY:lib.Target,prop> appeared in our
+        // caller's property and hand-evaluate it as if it were compiled.
+        // Create a context as cmCompiledGeneratorExpression::Evaluate does.
+        cmGeneratorExpressionContext libContext(
+          target->GetLocalGenerator(), context->Config, context->Quiet, target,
+          target, context->EvaluateForBuildsystem, lib.Backtrace,
+          context->Language);
+        std::string libResult =
+          lib.Target->EvaluateInterfaceProperty(prop, &libContext, dagChecker);
+        if (!libResult.empty()) {
+          if (result.empty()) {
+            result = std::move(libResult);
+          } else {
+            result.reserve(result.size() + 1 + libResult.size());
+            result += ";";
+            result += libResult;
+          }
+        }
+      }
+    }
+  }
+  return result;
 }
 
 static const struct TargetPropertyNode : public cmGeneratorExpressionNode
@@ -1205,44 +1198,37 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
       return target->GetLinkerLanguage(context->Config);
     }
 
-    cmGeneratorExpressionDAGChecker dagChecker(
-      context->Backtrace, target, propertyName, content, dagCheckerParent);
+    std::string interfacePropertyName;
+    bool isInterfaceProperty = false;
 
-    switch (dagChecker.Check()) {
-      case cmGeneratorExpressionDAGChecker::SELF_REFERENCE:
-        dagChecker.ReportError(context, content->GetOriginalExpression());
-        return std::string();
-      case cmGeneratorExpressionDAGChecker::CYCLIC_REFERENCE:
-        // No error. We just skip cyclic references.
-        return std::string();
-      case cmGeneratorExpressionDAGChecker::ALREADY_SEEN:
-        for (size_t i = 1; i < cm::size(targetPropertyTransitiveWhitelist);
-             ++i) {
-          if (targetPropertyTransitiveWhitelist[i] == propertyName) {
-            // No error. We're not going to find anything new here.
-            return std::string();
-          }
-        }
-      case cmGeneratorExpressionDAGChecker::DAG:
-        break;
-    }
+#define POPULATE_INTERFACE_PROPERTY_NAME(prop)                                \
+  if (propertyName == #prop) {                                                \
+    interfacePropertyName = "INTERFACE_" #prop;                               \
+  } else if (propertyName == "INTERFACE_" #prop) {                            \
+    interfacePropertyName = "INTERFACE_" #prop;                               \
+    isInterfaceProperty = true;                                               \
+  } else
 
-    std::string prop;
-    bool haveProp = false;
-    if (const char* p = target->GetProperty(propertyName)) {
-      prop = p;
-      haveProp = true;
+    CM_FOR_EACH_TRANSITIVE_PROPERTY_NAME(POPULATE_INTERFACE_PROPERTY_NAME)
+    // Note that the above macro terminates with an else
+    /* else */ if (cmHasLiteralPrefix(propertyName, "COMPILE_DEFINITIONS_")) {
+      cmPolicies::PolicyStatus polSt =
+        context->LG->GetPolicyStatus(cmPolicies::CMP0043);
+      if (polSt == cmPolicies::WARN || polSt == cmPolicies::OLD) {
+        interfacePropertyName = "INTERFACE_COMPILE_DEFINITIONS";
+      }
     }
+#undef POPULATE_INTERFACE_PROPERTY_NAME
+
+    bool evaluatingLinkLibraries = false;
 
     if (dagCheckerParent) {
       if (dagCheckerParent->EvaluatingGenexExpression() ||
           dagCheckerParent->EvaluatingPICExpression()) {
         // No check required.
       } else if (dagCheckerParent->EvaluatingLinkLibraries()) {
-#define TRANSITIVE_PROPERTY_COMPARE(PROPERTY)                                 \
-  (#PROPERTY == propertyName || "INTERFACE_" #PROPERTY == propertyName) ||
-        if (CM_FOR_EACH_TRANSITIVE_PROPERTY_NAME(
-              TRANSITIVE_PROPERTY_COMPARE) false) { // NOLINT(*)
+        evaluatingLinkLibraries = true;
+        if (!interfacePropertyName.empty()) {
           reportError(
             context, content->GetOriginalExpression(),
             "$<TARGET_PROPERTY:...> expression in link libraries "
@@ -1250,69 +1236,47 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
             "over the link libraries, creating a recursion.");
           return std::string();
         }
-#undef TRANSITIVE_PROPERTY_COMPARE
-
-        if (!haveProp) {
-          return std::string();
-        }
       } else {
 #define ASSERT_TRANSITIVE_PROPERTY_METHOD(METHOD) dagCheckerParent->METHOD() ||
-
         assert(CM_FOR_EACH_TRANSITIVE_PROPERTY_METHOD(
           ASSERT_TRANSITIVE_PROPERTY_METHOD) false); // NOLINT(clang-tidy)
 #undef ASSERT_TRANSITIVE_PROPERTY_METHOD
       }
     }
 
-    std::string linkedTargetsContent;
-
-    std::string interfacePropertyName;
-    bool isInterfaceProperty = false;
+    if (isInterfaceProperty) {
+      return target->EvaluateInterfaceProperty(propertyName, context,
+                                               dagCheckerParent);
+    }
 
-#define POPULATE_INTERFACE_PROPERTY_NAME(prop)                                \
-  if (propertyName == #prop) {                                                \
-    interfacePropertyName = "INTERFACE_" #prop;                               \
-  } else if (propertyName == "INTERFACE_" #prop) {                            \
-    interfacePropertyName = "INTERFACE_" #prop;                               \
-    isInterfaceProperty = true;                                               \
-  } else
+    cmGeneratorExpressionDAGChecker dagChecker(
+      context->Backtrace, target, propertyName, content, dagCheckerParent);
 
-    CM_FOR_EACH_TRANSITIVE_PROPERTY_NAME(POPULATE_INTERFACE_PROPERTY_NAME)
-    // Note that the above macro terminates with an else
-    /* else */ if (cmHasLiteralPrefix(propertyName, "COMPILE_DEFINITIONS_")) {
-      cmPolicies::PolicyStatus polSt =
-        context->LG->GetPolicyStatus(cmPolicies::CMP0043);
-      if (polSt == cmPolicies::WARN || polSt == cmPolicies::OLD) {
-        interfacePropertyName = "INTERFACE_COMPILE_DEFINITIONS";
-      }
+    switch (dagChecker.Check()) {
+      case cmGeneratorExpressionDAGChecker::SELF_REFERENCE:
+        dagChecker.ReportError(context, content->GetOriginalExpression());
+        return std::string();
+      case cmGeneratorExpressionDAGChecker::CYCLIC_REFERENCE:
+        // No error. We just skip cyclic references.
+        return std::string();
+      case cmGeneratorExpressionDAGChecker::ALREADY_SEEN:
+        // We handle transitive properties above.  For non-transitive
+        // properties we accept repeats anyway.
+      case cmGeneratorExpressionDAGChecker::DAG:
+        break;
     }
-#undef POPULATE_INTERFACE_PROPERTY_NAME
-    cmGeneratorTarget const* headTarget =
-      context->HeadTarget && isInterfaceProperty ? context->HeadTarget
-                                                 : target;
 
-    if (isInterfaceProperty) {
-      if (cmLinkInterfaceLibraries const* iface =
-            target->GetLinkInterfaceLibraries(context->Config, headTarget,
-                                              true)) {
-        linkedTargetsContent =
-          getLinkedTargetsContent(iface->Libraries, target, headTarget,
-                                  context, &dagChecker, interfacePropertyName);
-      }
-    } else if (!interfacePropertyName.empty()) {
-      if (cmLinkImplementationLibraries const* impl =
-            target->GetLinkImplementationLibraries(context->Config)) {
-        linkedTargetsContent =
-          getLinkedTargetsContent(impl->Libraries, target, target, context,
-                                  &dagChecker, interfacePropertyName);
-      }
+    std::string result;
+    bool haveProp = false;
+    if (const char* p = target->GetProperty(propertyName)) {
+      result = p;
+      haveProp = true;
+    } else if (evaluatingLinkLibraries) {
+      return std::string();
     }
 
-    if (!haveProp) {
-      if (target->IsImported() ||
-          target->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
-        return linkedTargetsContent;
-      }
+    if (!haveProp && !target->IsImported() &&
+        target->GetType() != cmStateEnums::INTERFACE_LIBRARY) {
       if (target->IsLinkInterfaceDependentBoolProperty(propertyName,
                                                        context->Config)) {
         context->HadContextSensitiveCondition = true;
@@ -1345,8 +1309,6 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
                                                              context->Config);
         return propContent ? propContent : "";
       }
-
-      return linkedTargetsContent;
     }
 
     if (!target->IsImported() && dagCheckerParent &&
@@ -1368,15 +1330,17 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
         return propContent ? propContent : "";
       }
     }
+
     if (!interfacePropertyName.empty()) {
-      std::string result = this->EvaluateDependentExpression(
-        prop, context->LG, context, headTarget, target, &dagChecker);
+      result = this->EvaluateDependentExpression(result, context->LG, context,
+                                                 target, target, &dagChecker);
+      std::string linkedTargetsContent = getLinkedTargetsContent(
+        target, interfacePropertyName, context, &dagChecker);
       if (!linkedTargetsContent.empty()) {
         result += (result.empty() ? "" : ";") + linkedTargetsContent;
       }
-      return result;
     }
-    return prop;
+    return result;
   }
 } targetPropertyNode;
 
diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx
index 6e6c917..7c41045 100644
--- a/Source/cmGeneratorTarget.cxx
+++ b/Source/cmGeneratorTarget.cxx
@@ -22,7 +22,9 @@
 #include "cmCustomCommandGenerator.h"
 #include "cmCustomCommandLines.h"
 #include "cmGeneratorExpression.h"
+#include "cmGeneratorExpressionContext.h"
 #include "cmGeneratorExpressionDAGChecker.h"
+#include "cmGeneratorExpressionNode.h"
 #include "cmGlobalGenerator.h"
 #include "cmLocalGenerator.h"
 #include "cmMakefile.h"
@@ -1141,6 +1143,126 @@ bool cmGeneratorTarget::GetPropertyAsBool(const std::string& prop) const
   return this->Target->GetPropertyAsBool(prop);
 }
 
+bool cmGeneratorTarget::MaybeHaveInterfaceProperty(
+  std::string const& prop, cmGeneratorExpressionContext* context) const
+{
+  std::string const key = prop + '@' + context->Config;
+  auto i = this->MaybeInterfacePropertyExists.find(key);
+  if (i == this->MaybeInterfacePropertyExists.end()) {
+    // Insert an entry now in case there is a cycle.
+    i = this->MaybeInterfacePropertyExists.emplace(key, false).first;
+    bool& maybeInterfaceProp = i->second;
+
+    // If this target itself has a non-empty property value, we are done.
+    const char* p = this->GetProperty(prop);
+    maybeInterfaceProp = p && *p;
+
+    // Otherwise, recurse to interface dependencies.
+    if (!maybeInterfaceProp) {
+      cmGeneratorTarget const* headTarget =
+        context->HeadTarget ? context->HeadTarget : this;
+      if (cmLinkInterfaceLibraries const* iface =
+            this->GetLinkInterfaceLibraries(context->Config, headTarget,
+                                            true)) {
+        if (iface->HadHeadSensitiveCondition) {
+          // With a different head target we may get to a library with
+          // this interface property.
+          maybeInterfaceProp = true;
+        } else {
+          // The transitive interface libraries do not depend on the
+          // head target, so we can follow them.
+          for (cmLinkItem const& lib : iface->Libraries) {
+            if (lib.Target &&
+                lib.Target->MaybeHaveInterfaceProperty(prop, context)) {
+              maybeInterfaceProp = true;
+              break;
+            }
+          }
+        }
+      }
+    }
+  }
+  return i->second;
+}
+
+std::string cmGeneratorTarget::EvaluateInterfaceProperty(
+  std::string const& prop, cmGeneratorExpressionContext* context,
+  cmGeneratorExpressionDAGChecker* dagCheckerParent) const
+{
+  std::string result;
+
+  // If the property does not appear transitively at all, we are done.
+  if (!this->MaybeHaveInterfaceProperty(prop, context)) {
+    return result;
+  }
+
+  // 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);
+  switch (dagChecker.Check()) {
+    case cmGeneratorExpressionDAGChecker::SELF_REFERENCE:
+      dagChecker.ReportError(
+        context, "$<TARGET_PROPERTY:" + this->GetName() + "," + prop + ">");
+      return result;
+    case cmGeneratorExpressionDAGChecker::CYCLIC_REFERENCE:
+      // No error. We just skip cyclic references.
+      return result;
+    case cmGeneratorExpressionDAGChecker::ALREADY_SEEN:
+      // No error. We have already seen this transitive property.
+      return result;
+    case cmGeneratorExpressionDAGChecker::DAG:
+      break;
+  }
+
+  cmGeneratorTarget const* headTarget =
+    context->HeadTarget ? context->HeadTarget : this;
+
+  if (const char* p = this->GetProperty(prop)) {
+    result = cmGeneratorExpressionNode::EvaluateDependentExpression(
+      p, context->LG, context, headTarget, this, &dagChecker);
+  }
+
+  if (cmLinkInterfaceLibraries const* iface =
+        this->GetLinkInterfaceLibraries(context->Config, headTarget, true)) {
+    for (cmLinkItem const& lib : iface->Libraries) {
+      // Broken code can have a target in its own link interface.
+      // Don't follow such link interface entries so as not to create a
+      // self-referencing loop.
+      if (lib.Target && lib.Target != this) {
+        // Pretend $<TARGET_PROPERTY:lib.Target,prop> appeared in the
+        // above property and hand-evaluate it as if it were compiled.
+        // Create a context as cmCompiledGeneratorExpression::Evaluate does.
+        cmGeneratorExpressionContext libContext(
+          context->LG, context->Config, context->Quiet, headTarget, this,
+          context->EvaluateForBuildsystem, context->Backtrace,
+          context->Language);
+        std::string libResult = cmGeneratorExpression::StripEmptyListElements(
+          lib.Target->EvaluateInterfaceProperty(prop, &libContext,
+                                                &dagChecker));
+        if (!libResult.empty()) {
+          if (result.empty()) {
+            result = std::move(libResult);
+          } else {
+            result.reserve(result.size() + 1 + libResult.size());
+            result += ";";
+            result += libResult;
+          }
+        }
+        context->HadContextSensitiveCondition =
+          context->HadContextSensitiveCondition ||
+          libContext.HadContextSensitiveCondition;
+        context->HadHeadSensitiveCondition =
+          context->HadHeadSensitiveCondition ||
+          libContext.HadHeadSensitiveCondition;
+      }
+    }
+  }
+
+  return result;
+}
+
 namespace {
 void AddInterfaceEntries(cmGeneratorTarget const* headTarget,
                          std::string const& config, std::string const& prop,
@@ -1152,23 +1274,17 @@ void AddInterfaceEntries(cmGeneratorTarget const* headTarget,
         headTarget->GetLinkImplementationLibraries(config)) {
     for (cmLinkImplItem const& lib : impl->Libraries) {
       if (lib.Target) {
-        std::string uniqueName =
-          headTarget->GetGlobalGenerator()->IndexGeneratorTargetUniquely(
-            lib.Target);
-        std::string genex =
-          "$<TARGET_PROPERTY:" + std::move(uniqueName) + "," + prop + ">";
-        cmGeneratorExpression ge(lib.Backtrace);
-        std::unique_ptr<cmCompiledGeneratorExpression> cge = ge.Parse(genex);
-        cge->SetEvaluateForBuildsystem(true);
-
         EvaluatedTargetPropertyEntry ee(lib, lib.Backtrace);
+        // Pretend $<TARGET_PROPERTY:lib.Target,prop> appeared in our
+        // caller's property and hand-evaluate it as if it were compiled.
+        // Create a context as cmCompiledGeneratorExpression::Evaluate does.
+        cmGeneratorExpressionContext context(
+          headTarget->GetLocalGenerator(), config, false, headTarget,
+          headTarget, true, lib.Backtrace, lang);
         cmSystemTools::ExpandListArgument(
-          cge->Evaluate(headTarget->GetLocalGenerator(), config, false,
-                        headTarget, dagChecker, lang),
+          lib.Target->EvaluateInterfaceProperty(prop, &context, dagChecker),
           ee.Values);
-        if (cge->GetHadContextSensitiveCondition()) {
-          ee.ContextDependent = true;
-        }
+        ee.ContextDependent = context.HadContextSensitiveCondition;
         entries.emplace_back(std::move(ee));
       }
     }
diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h
index e86535d..3874738 100644
--- a/Source/cmGeneratorTarget.h
+++ b/Source/cmGeneratorTarget.h
@@ -14,6 +14,7 @@
 #include <set>
 #include <stddef.h>
 #include <string>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -25,6 +26,9 @@ class cmMakefile;
 class cmSourceFile;
 class cmTarget;
 
+struct cmGeneratorExpressionContext;
+struct cmGeneratorExpressionDAGChecker;
+
 class cmGeneratorTarget
 {
 public:
@@ -674,6 +678,10 @@ public:
 
   class TargetPropertyEntry;
 
+  std::string EvaluateInterfaceProperty(
+    std::string const& prop, cmGeneratorExpressionContext* context,
+    cmGeneratorExpressionDAGChecker* dagCheckerParent) const;
+
   bool HaveInstallTreeRPATH(const std::string& config) const;
 
   bool GetBuildRPATH(const std::string& config, std::string& rpath) const;
@@ -849,6 +857,10 @@ private:
   mutable std::vector<AllConfigSource> AllConfigSources;
   void ComputeAllConfigSources() const;
 
+  mutable std::unordered_map<std::string, bool> MaybeInterfacePropertyExists;
+  bool MaybeHaveInterfaceProperty(std::string const& prop,
+                                  cmGeneratorExpressionContext* context) const;
+
   std::vector<TargetPropertyEntry*> IncludeDirectoriesEntries;
   std::vector<TargetPropertyEntry*> CompileOptionsEntries;
   std::vector<TargetPropertyEntry*> CompileFeaturesEntries;
diff --git a/Source/cmLinkItem.h b/Source/cmLinkItem.h
index 5b635b5..6450c62 100644
--- a/Source/cmLinkItem.h
+++ b/Source/cmLinkItem.h
@@ -58,6 +58,9 @@ struct cmLinkInterfaceLibraries
 {
   // Libraries listed in the interface.
   std::vector<cmLinkItem> Libraries;
+
+  // Whether the list depends on a genex referencing the head target.
+  bool HadHeadSensitiveCondition = false;
 };
 
 struct cmLinkInterface : public cmLinkInterfaceLibraries
@@ -84,7 +87,6 @@ struct cmOptionalLinkInterface : public cmLinkInterface
   bool LibrariesDone = false;
   bool AllDone = false;
   bool Exists = false;
-  bool HadHeadSensitiveCondition = false;
   const char* ExplicitLibraries = nullptr;
 };