Merge topic 'genex-cleanup'

895efd4e7a cmGeneratorExpression: Consolidate recognition of transitive properties
5f7d8192da cmGeneratorExpression: Inline evaluation helper at only call site
91a25de520 cmGeneratorExpression: Add comments on implementation details
0a61116f52 cmGeneratorTarget: Remove EvaluateInterfaceProperty argument default

Acked-by: Kitware Robot <[email protected]>
Acked-by: buildbot <[email protected]>
Merge-request: !9425

Source/cmGeneratorExpression.cxx

@@ -70,13 +70,6 @@ const std::string& cmCompiledGeneratorExpression::Evaluate(
     currentTarget ? currentTarget : headTarget, this->EvaluateForBuildsystem,
     this->Backtrace, language);
 
-  return this->EvaluateWithContext(context, dagChecker);
-}
-
-const std::string& cmCompiledGeneratorExpression::EvaluateWithContext(
-  cmGeneratorExpressionContext& context,
-  cmGeneratorExpressionDAGChecker* dagChecker) const
-{
   if (!this->NeedsEvaluation) {
     return this->Input;
   }

Source/cmGeneratorExpression.h

@@ -17,7 +17,6 @@
 class cmake;
 class cmCompiledGeneratorExpression;
 class cmGeneratorTarget;
-struct cmGeneratorExpressionContext;
 struct cmGeneratorExpressionDAGChecker;
 struct cmGeneratorExpressionEvaluator;
 
@@ -151,10 +150,6 @@ public:
                               std::map<std::string, std::string>& mapping);
 
 private:
-  const std::string& EvaluateWithContext(
-    cmGeneratorExpressionContext& context,
-    cmGeneratorExpressionDAGChecker* dagChecker) const;
-
   cmCompiledGeneratorExpression(cmake& cmakeInstance,
                                 cmListFileBacktrace backtrace,
                                 std::string input);

Source/cmGeneratorExpressionDAGChecker.cxx

@@ -37,16 +37,19 @@ cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker(
   , Content(content)
   , Backtrace(std::move(backtrace))
 {
-  const auto* top = this->Top;
-  this->CheckResult = this->CheckGraph();
+  if (parent) {
+    this->TopIsTransitiveProperty = parent->TopIsTransitiveProperty;
+  } else {
+#define TEST_TRANSITIVE_PROPERTY_METHOD(METHOD) this->METHOD() ||
+    this->TopIsTransitiveProperty = (CM_FOR_EACH_TRANSITIVE_PROPERTY_METHOD(
+      TEST_TRANSITIVE_PROPERTY_METHOD) false); // NOLINT(*)
+#undef TEST_TRANSITIVE_PROPERTY_METHOD
+  }
 
-#define TEST_TRANSITIVE_PROPERTY_METHOD(METHOD) top->METHOD() ||
+  this->CheckResult = this->CheckGraph();
 
-  if (this->CheckResult == DAG &&
-      (CM_FOR_EACH_TRANSITIVE_PROPERTY_METHOD(
-        TEST_TRANSITIVE_PROPERTY_METHOD) false)) // NOLINT(*)
-#undef TEST_TRANSITIVE_PROPERTY_METHOD
-  {
+  if (this->CheckResult == DAG && this->EvaluatingTransitiveProperty()) {
+    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;
@@ -139,14 +142,22 @@ bool cmGeneratorExpressionDAGChecker::GetTransitivePropertiesOnlyCMP0131()
   return this->Top->CMP0131;
 }
 
+bool cmGeneratorExpressionDAGChecker::EvaluatingTransitiveProperty() const
+{
+  return this->TopIsTransitiveProperty;
+}
+
 bool cmGeneratorExpressionDAGChecker::EvaluatingGenexExpression() const
 {
+  // Corresponds to GenexEvaluator::EvaluateExpression.
   return cmHasLiteralPrefix(this->Property, "TARGET_GENEX_EVAL:") ||
     cmHasLiteralPrefix(this->Property, "GENEX_EVAL:");
 }
 
 bool cmGeneratorExpressionDAGChecker::EvaluatingPICExpression() const
 {
+  // Corresponds to checkInterfacePropertyCompatibility's special case
+  // that evaluates the value of POSITION_INDEPENDENT_CODE as a genex.
   return this->Top->Property == "INTERFACE_POSITION_INDEPENDENT_CODE";
 }
 

Source/cmGeneratorExpressionDAGChecker.h

@@ -66,6 +66,7 @@ struct cmGeneratorExpressionDAGChecker
   void ReportError(cmGeneratorExpressionContext* context,
                    const std::string& expr);
 
+  bool EvaluatingTransitiveProperty() const;
   bool EvaluatingGenexExpression() const;
   bool EvaluatingPICExpression() const;
   bool EvaluatingCompileExpression() const;
@@ -109,4 +110,5 @@ private:
   Result CheckResult;
   bool TransitivePropertiesOnly = false;
   bool CMP0131 = false;
+  bool TopIsTransitiveProperty = false;
 };

Source/cmGeneratorExpressionNode.cxx

@@ -547,6 +547,7 @@ static const struct TargetGenexEvalNode : public GenexEvaluator
       return expression;
     }
 
+    // Replace the surrounding context with the named target.
     cmGeneratorExpressionContext targetContext(
       context->LG, context->Config, context->Quiet, target, target,
       context->EvaluateForBuildsystem, context->Backtrace, context->Language);
@@ -2714,8 +2715,9 @@ static std::string getLinkedTargetsContent(
           target->GetLocalGenerator(), context->Config, context->Quiet, target,
           target, context->EvaluateForBuildsystem, lib.Backtrace,
           context->Language);
-        std::string libResult =
-          lib.Target->EvaluateInterfaceProperty(prop, &libContext, dagChecker);
+        std::string libResult = lib.Target->EvaluateInterfaceProperty(
+          prop, &libContext, dagChecker,
+          cmGeneratorTarget::LinkInterfaceFor::Usage);
         if (!libResult.empty()) {
           if (result.empty()) {
             result = std::move(libResult);
@@ -2896,6 +2898,9 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
     bool evaluatingLinkLibraries = false;
 
     if (dagCheckerParent) {
+      // This $<TARGET_PROPERTY:...> node has been reached while evaluating
+      // another target property value.  Check that the outermost evaluation
+      // expects such nested evaluations.
       if (dagCheckerParent->EvaluatingGenexExpression() ||
           dagCheckerParent->EvaluatingPICExpression() ||
           dagCheckerParent->EvaluatingLinkerLauncher()) {
@@ -2911,17 +2916,16 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
           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
+        assert(dagCheckerParent
+                 ->EvaluatingTransitiveProperty()); // NOLINT(clang-tidy)
       }
     }
 
     if (isInterfaceProperty) {
       return cmGeneratorExpression::StripEmptyListElements(
-        target->EvaluateInterfaceProperty(propertyName, context,
-                                          dagCheckerParent));
+        target->EvaluateInterfaceProperty(
+          propertyName, context, dagCheckerParent,
+          cmGeneratorTarget::LinkInterfaceFor::Usage));
     }
 
     cmGeneratorExpressionDAGChecker dagChecker(

Source/cmGeneratorTarget.cxx

@@ -6376,6 +6376,7 @@ PropertyType checkInterfacePropertyCompatibility(cmGeneratorTarget const* tgt,
   std::string interfaceProperty = "INTERFACE_" + p;
   std::unique_ptr<cmGeneratorExpressionInterpreter> genexInterpreter;
   if (p == "POSITION_INDEPENDENT_CODE") {
+    // Corresponds to EvaluatingPICExpression.
     genexInterpreter = cm::make_unique<cmGeneratorExpressionInterpreter>(
       tgt->GetLocalGenerator(), config, tgt);
   }

Source/cmGeneratorTarget.h

@@ -883,7 +883,7 @@ public:
   std::string EvaluateInterfaceProperty(
     std::string const& prop, cmGeneratorExpressionContext* context,
     cmGeneratorExpressionDAGChecker* dagCheckerParent,
-    LinkInterfaceFor interfaceFor = LinkInterfaceFor::Usage) const;
+    LinkInterfaceFor interfaceFor) const;
 
   bool HaveInstallTreeRPATH(const std::string& config) const;