From b2785a0fbdcfa703f3ad3aaa2949ec7db55a27d9 Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 20 Jul 2019 16:09:03 -0400 Subject: Genex: Move TARGET_PROPERTY linked targets evaluation to end --- Source/cmGeneratorExpressionNode.cxx | 63 ++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index d828dac..1973e99 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -1227,10 +1227,10 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode break; } - std::string prop; + std::string result; bool haveProp = false; if (const char* p = target->GetProperty(propertyName)) { - prop = p; + result = p; haveProp = true; } @@ -1264,8 +1264,6 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } } - std::string linkedTargetsContent; - std::string interfacePropertyName; bool isInterfaceProperty = false; @@ -1287,32 +1285,9 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } } #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); - } - } - 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 +1320,6 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode context->Config); return propContent ? propContent : ""; } - - return linkedTargetsContent; } if (!target->IsImported() && dagCheckerParent && @@ -1368,15 +1341,35 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode return propContent ? propContent : ""; } } + if (!interfacePropertyName.empty()) { - std::string result = this->EvaluateDependentExpression( - prop, context->LG, context, headTarget, target, &dagChecker); + cmGeneratorTarget const* headTarget = + context->HeadTarget && isInterfaceProperty ? context->HeadTarget + : target; + result = this->EvaluateDependentExpression( + result, context->LG, context, headTarget, target, &dagChecker); + std::string linkedTargetsContent; + if (isInterfaceProperty) { + if (cmLinkInterfaceLibraries const* iface = + target->GetLinkInterfaceLibraries(context->Config, headTarget, + true)) { + linkedTargetsContent = getLinkedTargetsContent( + iface->Libraries, target, headTarget, context, &dagChecker, + interfacePropertyName); + } + } else { + if (cmLinkImplementationLibraries const* impl = + target->GetLinkImplementationLibraries(context->Config)) { + linkedTargetsContent = + getLinkedTargetsContent(impl->Libraries, target, target, context, + &dagChecker, interfacePropertyName); + } + } if (!linkedTargetsContent.empty()) { result += (result.empty() ? "" : ";") + linkedTargetsContent; } - return result; } - return prop; + return result; } } targetPropertyNode; -- cgit v0.12 From 7caebeb0e4babc41e5c84b4ce0ea70adcfdea4a1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 20 Jul 2019 17:30:31 -0400 Subject: Genex: Re-order TARGET_PROPERTY logic to de-duplicate checks Check for usage requirement properties early enough to avoid duplicate checks in other conditions. --- Source/cmGeneratorExpressionNode.cxx | 68 ++++++++++++++---------------------- 1 file changed, 26 insertions(+), 42 deletions(-) diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index 1973e99..b027cd4 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -1038,14 +1038,6 @@ 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 std::string getLinkedTargetsContent( std::vector const& libraries, cmGeneratorTarget const* target, @@ -1205,6 +1197,28 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode return target->GetLinkerLanguage(context->Config); } + std::string interfacePropertyName; + bool isInterfaceProperty = false; + +#define POPULATE_INTERFACE_PROPERTY_NAME(prop) \ + if (propertyName == #prop) { \ + interfacePropertyName = "INTERFACE_" #prop; \ + } else if (propertyName == "INTERFACE_" #prop) { \ + interfacePropertyName = "INTERFACE_" #prop; \ + isInterfaceProperty = true; \ + } else + + 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 + cmGeneratorExpressionDAGChecker dagChecker( context->Backtrace, target, propertyName, content, dagCheckerParent); @@ -1216,12 +1230,9 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode // 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(); - } + if (isInterfaceProperty) { + // No error. We're not going to find anything new here. + return std::string(); } case cmGeneratorExpressionDAGChecker::DAG: break; @@ -1239,10 +1250,7 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode 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(*) + if (!interfacePropertyName.empty()) { reportError( context, content->GetOriginalExpression(), "$ expression in link libraries " @@ -1250,42 +1258,18 @@ 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 interfacePropertyName; - bool isInterfaceProperty = false; - -#define POPULATE_INTERFACE_PROPERTY_NAME(prop) \ - if (propertyName == #prop) { \ - interfacePropertyName = "INTERFACE_" #prop; \ - } else if (propertyName == "INTERFACE_" #prop) { \ - interfacePropertyName = "INTERFACE_" #prop; \ - isInterfaceProperty = true; \ - } else - - 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 - if (!haveProp && !target->IsImported() && target->GetType() != cmStateEnums::INTERFACE_LIBRARY) { if (target->IsLinkInterfaceDependentBoolProperty(propertyName, -- cgit v0.12 From 0239bf8ac88bb8ae9d8945be506cee2c9adb08f5 Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 20 Jul 2019 19:29:13 -0400 Subject: Genex: In TARGET_PROPERTY check for usage reqs in link libs earlier --- Source/cmGeneratorExpressionNode.cxx | 51 ++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index b027cd4..b268ea2 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -1219,6 +1219,30 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } #undef POPULATE_INTERFACE_PROPERTY_NAME + bool evaluatingLinkLibraries = false; + + if (dagCheckerParent) { + if (dagCheckerParent->EvaluatingGenexExpression() || + dagCheckerParent->EvaluatingPICExpression()) { + // No check required. + } else if (dagCheckerParent->EvaluatingLinkLibraries()) { + evaluatingLinkLibraries = true; + if (!interfacePropertyName.empty()) { + reportError( + context, content->GetOriginalExpression(), + "$ expression in link libraries " + "evaluation depends on target property which is transitive " + "over the link libraries, creating a recursion."); + 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 + } + } + cmGeneratorExpressionDAGChecker dagChecker( context->Backtrace, target, propertyName, content, dagCheckerParent); @@ -1243,31 +1267,8 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode if (const char* p = target->GetProperty(propertyName)) { result = p; haveProp = true; - } - - if (dagCheckerParent) { - if (dagCheckerParent->EvaluatingGenexExpression() || - dagCheckerParent->EvaluatingPICExpression()) { - // No check required. - } else if (dagCheckerParent->EvaluatingLinkLibraries()) { - if (!interfacePropertyName.empty()) { - reportError( - context, content->GetOriginalExpression(), - "$ expression in link libraries " - "evaluation depends on target property which is transitive " - "over the link libraries, creating a recursion."); - return std::string(); - } - - 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 - } + } else if (evaluatingLinkLibraries) { + return std::string(); } if (!haveProp && !target->IsImported() && -- cgit v0.12 From 11fa818ecda0b50446aef891b06976973005e94b Mon Sep 17 00:00:00 2001 From: Brad King Date: Sun, 21 Jul 2019 07:57:08 -0400 Subject: Genex: Optimize usage requirement TARGET_PROPERTY recursion In large projects the generation process spends a lot of time evaluating usage requirements through transitive interface properties on targets. This can be seen in a contrived example with deep dependencies: set(prev "") foreach(i RANGE 1 500) add_library(a${i} a.c) target_compile_definitions(a${i} PUBLIC A${i}) target_link_libraries(a${i} PUBLIC ${prev}) set(prev a${i}) endforeach() For each usage requirement (such as `INTERFACE_COMPILE_DEFINITIONS` or `INTERFACE_INCLUDE_DIRECTORIES`), the value of the generator expression `$` includes the values of the same property from the transitive closure of link libraries of the target. Previously we computed this by constructing a generator expression string like `$` and recursively evaluating it with the generator expression engine. Avoid the string construction and parsing by creating and using a dedicated evaluation method `cmGeneratorTarget::EvaluateInterfaceProperty` that looks up the properties directly. Issue: #18964, #18965 --- Source/cmGeneratorExpressionNode.cxx | 35 +++++++---------- Source/cmGeneratorTarget.cxx | 75 ++++++++++++++++++++++++++++++++++++ Source/cmGeneratorTarget.h | 7 ++++ 3 files changed, 95 insertions(+), 22 deletions(-) diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index b268ea2..8d02b68 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -1243,6 +1243,11 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } } + if (isInterfaceProperty) { + return target->EvaluateInterfaceProperty(propertyName, context, + dagCheckerParent); + } + cmGeneratorExpressionDAGChecker dagChecker( context->Backtrace, target, propertyName, content, dagCheckerParent); @@ -1254,10 +1259,8 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode // No error. We just skip cyclic references. return std::string(); case cmGeneratorExpressionDAGChecker::ALREADY_SEEN: - if (isInterfaceProperty) { - // No error. We're not going to find anything new here. - return std::string(); - } + // We handle transitive properties above. For non-transitive + // properties we accept repeats anyway. case cmGeneratorExpressionDAGChecker::DAG: break; } @@ -1328,27 +1331,15 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } if (!interfacePropertyName.empty()) { - cmGeneratorTarget const* headTarget = - context->HeadTarget && isInterfaceProperty ? context->HeadTarget - : target; + cmGeneratorTarget const* headTarget = target; result = this->EvaluateDependentExpression( result, context->LG, context, headTarget, target, &dagChecker); std::string linkedTargetsContent; - if (isInterfaceProperty) { - if (cmLinkInterfaceLibraries const* iface = - target->GetLinkInterfaceLibraries(context->Config, headTarget, - true)) { - linkedTargetsContent = getLinkedTargetsContent( - iface->Libraries, target, headTarget, context, &dagChecker, - interfacePropertyName); - } - } else { - if (cmLinkImplementationLibraries const* impl = - target->GetLinkImplementationLibraries(context->Config)) { - linkedTargetsContent = - getLinkedTargetsContent(impl->Libraries, target, target, context, - &dagChecker, interfacePropertyName); - } + if (cmLinkImplementationLibraries const* impl = + target->GetLinkImplementationLibraries(context->Config)) { + linkedTargetsContent = + getLinkedTargetsContent(impl->Libraries, target, target, context, + &dagChecker, interfacePropertyName); } if (!linkedTargetsContent.empty()) { result += (result.empty() ? "" : ";") + linkedTargetsContent; diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 6e6c917..21da2fd 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,79 @@ bool cmGeneratorTarget::GetPropertyAsBool(const std::string& prop) const return this->Target->GetPropertyAsBool(prop); } +std::string cmGeneratorTarget::EvaluateInterfaceProperty( + std::string const& prop, cmGeneratorExpressionContext* context, + cmGeneratorExpressionDAGChecker* dagCheckerParent) const +{ + std::string result; + + // 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); + switch (dagChecker.Check()) { + case cmGeneratorExpressionDAGChecker::SELF_REFERENCE: + dagChecker.ReportError( + context, "$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 $ 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, diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index e86535d..b875c40 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -25,6 +25,9 @@ class cmMakefile; class cmSourceFile; class cmTarget; +struct cmGeneratorExpressionContext; +struct cmGeneratorExpressionDAGChecker; + class cmGeneratorTarget { public: @@ -674,6 +677,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; -- cgit v0.12 From ad2b3a32d1b54716d1e87ae89db8c31d614a4730 Mon Sep 17 00:00:00 2001 From: Brad King Date: Sun, 21 Jul 2019 07:58:29 -0400 Subject: Genex: Optimize build setting TARGET_PROPERTY evaluation For each build setting property (such as `COMPILE_DEFINITIONS` or `INCLUDE_DIRECTORIES`), the value of `$` includes the values of the corresponding `INTERFACE_*` usage requirement property from the transitive closure of link libraries of the target. Previously we computed this by constructing a generator expression string like `$` and recursively evaluating it with the generator expression engine. Avoid the string construction and parsing by using the dedicated evaluation method `cmGeneratorTarget::EvaluateInterfaceProperty`. Issue: #18964, #18965 --- Source/cmGeneratorExpressionNode.cxx | 75 +++++++++++++++++------------------- Source/cmGeneratorTarget.cxx | 22 ++++------- 2 files changed, 43 insertions(+), 54 deletions(-) diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index 8d02b68..49d0c47 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -1038,37 +1038,38 @@ static const struct CompileLanguageAndIdNode : public cmGeneratorExpressionNode } } languageAndIdNode; -template std::string getLinkedTargetsContent( - std::vector 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 + "$"; - 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 $ 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 @@ -1331,16 +1332,10 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } if (!interfacePropertyName.empty()) { - cmGeneratorTarget const* headTarget = target; - result = this->EvaluateDependentExpression( - result, context->LG, context, headTarget, target, &dagChecker); - std::string linkedTargetsContent; - if (cmLinkImplementationLibraries const* impl = - target->GetLinkImplementationLibraries(context->Config)) { - linkedTargetsContent = - getLinkedTargetsContent(impl->Libraries, target, target, context, - &dagChecker, interfacePropertyName); - } + 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; } diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 21da2fd..b22d8b6 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -1227,23 +1227,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 = - "$"; - cmGeneratorExpression ge(lib.Backtrace); - std::unique_ptr cge = ge.Parse(genex); - cge->SetEvaluateForBuildsystem(true); - EvaluatedTargetPropertyEntry ee(lib, lib.Backtrace); + // Pretend $ 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)); } } -- cgit v0.12 From b5460f99315f8e6a6bdc985ebc0ca18dd8a294a8 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 23 Jul 2019 09:52:33 -0400 Subject: cmLinkItem: Expose HadHeadSensitiveCondition in cmLinkInterfaceLibraries Clients may be able to avoid repeating work if they know the transitive link interface libraries do not depend on what is linking them. --- Source/cmLinkItem.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 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; }; -- cgit v0.12 From 1d3841b6003d4f1a45e79f6b9e6d6357514905f1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 23 Jul 2019 10:01:13 -0400 Subject: Genex: Memoize usage requirement TARGET_PROPERTY existence For each usage requirement (such as `INTERFACE_COMPILE_DEFINITIONS` or `INTERFACE_INCLUDE_DIRECTORIES`), the value of the generator expression `$` includes the values of the same property from the transitive closure of link libraries of the target. In cases that a target's transitive closure of dependencies does not depend on the target being linked (the "head" target), we can memoize whether or not a usage requirement property exists at all for that target. When a usage requirement does not exist for a target, we can skip evaluating it for every consuming target. Fixes: #18964, #18965 --- Source/cmGeneratorTarget.cxx | 47 ++++++++++++++++++++++++++++++++++++++++++++ Source/cmGeneratorTarget.h | 5 +++++ 2 files changed, 52 insertions(+) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index b22d8b6..7c41045 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -1143,12 +1143,59 @@ 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 $ as if it were compiled. This is // a subset of TargetPropertyNode::Evaluate without stringify/parse steps // but sufficient for transitive interface properties. diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index b875c40..3874738 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -856,6 +857,10 @@ private: mutable std::vector AllConfigSources; void ComputeAllConfigSources() const; + mutable std::unordered_map MaybeInterfacePropertyExists; + bool MaybeHaveInterfaceProperty(std::string const& prop, + cmGeneratorExpressionContext* context) const; + std::vector IncludeDirectoriesEntries; std::vector CompileOptionsEntries; std::vector CompileFeaturesEntries; -- cgit v0.12