From 535fd6ce6d514deebc8c95424df83f73989f55a5 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Jul 2014 16:12:07 -0400 Subject: [PATCH 1/9] cmTarget: Make GetLink*Libraries methods safer to use Split the library lists out of LinkImplementation and LinkInterface into LinkImplementationLibraries and LinkInterfaceLibraries parent classes, respectively. Return these from GetLinkImplementationLibraries and GetLinkInterfaceLibraries, respectively, so that callers cannot access parts of the structures that have not been populated. --- Source/cmGeneratorExpressionEvaluator.cxx | 2 +- Source/cmTarget.cxx | 31 +++++++++--------- Source/cmTarget.h | 38 +++++++++++++---------- 3 files changed, 39 insertions(+), 32 deletions(-) diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index 3b83cd39f..1c15bd3ca 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -1114,7 +1114,7 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode else if (std::find_if(transBegin, transEnd, cmStrCmp(interfacePropertyName)) != transEnd) { - const cmTarget::LinkImplementation *impl + const cmTarget::LinkImplementationLibraries *impl = target->GetLinkImplementationLibraries(context->Config); if(impl) { diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index cdece87ae..b36a600a8 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -4379,7 +4379,7 @@ bool cmTarget::HaveBuildTreeRPATH(const std::string& config) const { return false; } - if(LinkImplementation const* impl = + if(LinkImplementationLibraries const* impl = this->GetLinkImplementationLibraries(config)) { return !impl->Libraries.empty(); @@ -5935,7 +5935,7 @@ cmTarget::LinkInterface const* cmTarget::GetLinkInterface( } //---------------------------------------------------------------------------- -cmTarget::LinkInterface const* +cmTarget::LinkInterfaceLibraries const* cmTarget::GetLinkInterfaceLibraries(const std::string& config, cmTarget const* head, bool usage_requirements_only) const @@ -6017,7 +6017,7 @@ void processILibs(const std::string& config, if (item.Target && emitted.insert(item.Target).second) { tgts.push_back(item.Target); - if(cmTarget::LinkInterface const* iface = + if(cmTarget::LinkInterfaceLibraries const* iface = item.Target->GetLinkInterfaceLibraries(config, headTarget, true)) { for(std::vector::const_iterator @@ -6041,7 +6041,7 @@ cmTarget::GetLinkImplementationClosure(const std::string& config) const this->Internal->CacheLinkImplementationClosureDone[config] = true; std::set emitted; - cmTarget::LinkImplementation const* impl + cmTarget::LinkImplementationLibraries const* impl = this->GetLinkImplementationLibraries(config); for(std::vector::const_iterator @@ -6059,7 +6059,7 @@ void cmTarget::GetTransitivePropertyTargets(const std::string& config, cmTarget const* headTarget, std::vector &tgts) const { - if(cmTarget::LinkInterface const* iface = + if(cmTarget::LinkInterfaceLibraries const* iface = this->GetLinkInterfaceLibraries(config, headTarget, true)) { for(std::vector::const_iterator it = iface->Libraries.begin(); @@ -6177,7 +6177,7 @@ cmTargetInternals::ComputeLinkInterfaceLibraries( // to the link implementation. { // The link implementation is the default link interface. - cmTarget::LinkImplementation const* impl = + cmTarget::LinkImplementationLibraries const* impl = thisTarget->GetLinkImplementationLibrariesInternal(config, headTarget); std::copy(impl->Libraries.begin(), impl->Libraries.end(), std::back_inserter(iface.Libraries)); @@ -6294,7 +6294,7 @@ void cmTargetInternals::ComputeLinkInterface(cmTarget const* thisTarget, || thisTarget->PolicyStatusCMP0022 == cmPolicies::OLD) { // The link implementation is the default link interface. - cmTarget::LinkImplementation const* + cmTarget::LinkImplementationLibraries const* impl = thisTarget->GetLinkImplementationLibrariesInternal(config, headTarget); iface.ImplementationIsInterface = true; @@ -6345,7 +6345,7 @@ void cmTargetInternals::AddInterfaceEntries( cmTarget const* thisTarget, std::string const& config, std::string const& prop, std::vector& entries) { - if(cmTarget::LinkImplementation const* impl = + if(cmTarget::LinkImplementationLibraries const* impl = thisTarget->GetLinkImplementationLibraries(config)) { for (std::vector::const_iterator @@ -6383,7 +6383,7 @@ cmTarget::GetLinkImplementation(const std::string& config) const if(!impl.LibrariesDone) { impl.LibrariesDone = true; - this->ComputeLinkImplementation(config, impl, this); + this->ComputeLinkImplementationLibraries(config, impl, this); } if(!impl.LanguagesDone) { @@ -6394,14 +6394,14 @@ cmTarget::GetLinkImplementation(const std::string& config) const } //---------------------------------------------------------------------------- -cmTarget::LinkImplementation const* +cmTarget::LinkImplementationLibraries const* cmTarget::GetLinkImplementationLibraries(const std::string& config) const { return this->GetLinkImplementationLibrariesInternal(config, this); } //---------------------------------------------------------------------------- -cmTarget::LinkImplementation const* +cmTarget::LinkImplementationLibraries const* cmTarget::GetLinkImplementationLibrariesInternal(const std::string& config, cmTarget const* head) const { @@ -6418,15 +6418,16 @@ cmTarget::GetLinkImplementationLibrariesInternal(const std::string& config, if(!impl.LibrariesDone) { impl.LibrariesDone = true; - this->ComputeLinkImplementation(config, impl, head); + this->ComputeLinkImplementationLibraries(config, impl, head); } return &impl; } //---------------------------------------------------------------------------- -void cmTarget::ComputeLinkImplementation(const std::string& config, - LinkImplementation& impl, - cmTarget const* head) const +void +cmTarget::ComputeLinkImplementationLibraries(const std::string& config, + LinkImplementation& impl, + cmTarget const* head) const { // Collect libraries directly linked in this configuration. for (std::vector::const_iterator diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 400544348..8e21d4fbc 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -262,14 +262,16 @@ public: /** The link interface specifies transitive library dependencies and other information needed by targets that link to this target. */ - struct LinkInterface + struct LinkInterfaceLibraries + { + // Libraries listed in the interface. + std::vector Libraries; + }; + struct LinkInterface: public LinkInterfaceLibraries { // Languages whose runtime libraries must be linked. std::vector Languages; - // Libraries listed in the interface. - std::vector Libraries; - // Shared library dependencies needed for linking on some platforms. std::vector SharedDeps; @@ -290,22 +292,21 @@ public: if the target cannot be linked. */ LinkInterface const* GetLinkInterface(const std::string& config, cmTarget const* headTarget) const; - LinkInterface const* GetLinkInterfaceLibraries(const std::string& config, - cmTarget const* headTarget, - bool usage_requirements_only) const; + LinkInterfaceLibraries const* + GetLinkInterfaceLibraries(const std::string& config, + cmTarget const* headTarget, + bool usage_requirements_only) const; void GetTransitivePropertyTargets(const std::string& config, cmTarget const* headTarget, std::vector &libs) const; + std::vector const& GetLinkImplementationClosure(const std::string& config) const; /** The link implementation specifies the direct library dependencies needed by the object files of the target. */ - struct LinkImplementation + struct LinkImplementationLibraries { - // Languages whose runtime libraries must be linked. - std::vector Languages; - // Libraries linked directly in this configuration. std::vector Libraries; @@ -313,10 +314,15 @@ public: // Needed only for OLD behavior of CMP0003. std::vector WrongConfigLibraries; }; + struct LinkImplementation: public LinkImplementationLibraries + { + // Languages whose runtime libraries must be linked. + std::vector Languages; + }; LinkImplementation const* GetLinkImplementation(const std::string& config) const; - LinkImplementation const* + LinkImplementationLibraries const* GetLinkImplementationLibraries(const std::string& config) const; /** Link information from the transitive closure of the link @@ -778,12 +784,12 @@ private: GetImportLinkInterface(const std::string& config, cmTarget const* head, bool usage_requirements_only) const; - LinkImplementation const* + LinkImplementationLibraries const* GetLinkImplementationLibrariesInternal(const std::string& config, cmTarget const* head) const; - void ComputeLinkImplementation(const std::string& config, - LinkImplementation& impl, - cmTarget const* head) const; + void ComputeLinkImplementationLibraries(const std::string& config, + LinkImplementation& impl, + cmTarget const* head) const; void ComputeLinkImplementationLanguages(const std::string& config, LinkImplementation& impl) const; void ComputeLinkClosure(const std::string& config, LinkClosure& lc) const; From fb3518dc81ac1b776503d4369c6d375a706485d1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Jul 2014 11:39:36 -0400 Subject: [PATCH 2/9] Refactor system include annotation propagation Since commit v3.0.0-rc1~174^2~1 (cmTarget: Fix system include annotation propagation, 2014-01-01) the cmGeneratorTarget::IsSystemIncludeDirectory method needs to collect all targets that might provide INTERFACE_(|SYSTEM)_INCLUDE_DIRECTORIES for the current target. We now have cmTarget::GetLinkImplementationClosure to provide this, so use it. --- Source/cmGeneratorTarget.cxx | 41 ++++++------------------------------ 1 file changed, 7 insertions(+), 34 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 64c5822f8..f9b68d47b 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -445,13 +445,6 @@ bool cmGeneratorTarget::IsSystemIncludeDirectory(const std::string& dir, if (iter == this->SystemIncludesCache.end()) { - cmTarget::LinkImplementation const* impl - = this->Target->GetLinkImplementation(config); - if(!impl) - { - return false; - } - cmGeneratorExpressionDAGChecker dagChecker( this->GetName(), "SYSTEM_INCLUDE_DIRECTORIES", 0, 0); @@ -471,35 +464,15 @@ bool cmGeneratorTarget::IsSystemIncludeDirectory(const std::string& dir, &dagChecker), result); } - std::set uniqueDeps; - for(std::vector::const_iterator - li = impl->Libraries.begin(); li != impl->Libraries.end(); ++li) + std::vector const& deps = + this->Target->GetLinkImplementationClosure(config); + for(std::vector::const_iterator + li = deps.begin(), le = deps.end(); li != le; ++li) { - cmTarget const* tgt = li->Target; - if (!tgt) - { - continue; - } - - if (uniqueDeps.insert(tgt).second) - { - handleSystemIncludesDep(this->Makefile, tgt, config, this->Target, - &dagChecker, result, excludeImported); - - std::vector deps; - tgt->GetTransitivePropertyTargets(config, this->Target, deps); - - for(std::vector::const_iterator di = deps.begin(); - di != deps.end(); ++di) - { - if (uniqueDeps.insert(*di).second) - { - handleSystemIncludesDep(this->Makefile, *di, config, this->Target, - &dagChecker, result, excludeImported); - } - } - } + handleSystemIncludesDep(this->Makefile, *li, config, this->Target, + &dagChecker, result, excludeImported); } + std::set unique; for(std::vector::iterator li = result.begin(); li != result.end(); ++li) From 0a8fbac19a1d12adaa10873cc8fdb3dff164c981 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Jul 2014 11:42:43 -0400 Subject: [PATCH 3/9] cmTarget: Drop GetTransitivePropertyTargets method Inline the implementation at the only remaining call site. --- Source/cmGeneratorExpressionEvaluator.cxx | 16 +++++++++++++--- Source/cmTarget.cxx | 19 ------------------- Source/cmTarget.h | 3 --- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index 1c15bd3ca..19b3e16d6 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -1098,10 +1098,20 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode if (std::find_if(transBegin, transEnd, cmStrCmp(propertyName)) != transEnd) { - std::vector tgts; - target->GetTransitivePropertyTargets(context->Config, - headTarget, tgts); + if(cmTarget::LinkInterfaceLibraries const* iface = + target->GetLinkInterfaceLibraries(context->Config, headTarget, true)) + { + for(std::vector::const_iterator + it = iface->Libraries.begin(); + it != iface->Libraries.end(); ++it) + { + if (it->Target) + { + tgts.push_back(it->Target); + } + } + } if (!tgts.empty()) { linkedTargetsContent = diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index b36a600a8..8185bcc2c 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -6054,25 +6054,6 @@ cmTarget::GetLinkImplementationClosure(const std::string& config) const return tgts; } -//---------------------------------------------------------------------------- -void cmTarget::GetTransitivePropertyTargets(const std::string& config, - cmTarget const* headTarget, - std::vector &tgts) const -{ - if(cmTarget::LinkInterfaceLibraries const* iface = - this->GetLinkInterfaceLibraries(config, headTarget, true)) - { - for(std::vector::const_iterator it = iface->Libraries.begin(); - it != iface->Libraries.end(); ++it) - { - if (it->Target) - { - tgts.push_back(it->Target); - } - } - } -} - //---------------------------------------------------------------------------- void cmTargetInternals::ComputeLinkInterfaceLibraries( diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 8e21d4fbc..12712725c 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -296,9 +296,6 @@ public: GetLinkInterfaceLibraries(const std::string& config, cmTarget const* headTarget, bool usage_requirements_only) const; - void GetTransitivePropertyTargets(const std::string& config, - cmTarget const* headTarget, - std::vector &libs) const; std::vector const& GetLinkImplementationClosure(const std::string& config) const; From 8cb9105431bcc4bd206d92b7cd53cebdb1a783bd Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Jul 2014 11:50:32 -0400 Subject: [PATCH 4/9] Genex: Simplify TARGET_PROPERTY transitive lookup In cmGeneratorExpressionEvaluator, make getLinkedTargetsContent a template so it can traverse over either the Libraries in a cmTarget LinkImplementationLibraries or a cmTarget LinkInterfaceLibraries. This also avoids creating a separate vector. --- Source/cmGeneratorExpressionEvaluator.cxx | 94 ++++++++--------------- 1 file changed, 30 insertions(+), 64 deletions(-) diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index 19b3e16d6..d08c3c9ed 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -799,72 +799,51 @@ static const char* targetPropertyTransitiveWhitelist[] = { #undef TRANSITIVE_PROPERTY_NAME +template std::string getLinkedTargetsContent( - std::vector &targets, + std::vector const &libraries, cmTarget const* target, cmTarget const* headTarget, cmGeneratorExpressionContext *context, cmGeneratorExpressionDAGChecker *dagChecker, const std::string &interfacePropertyName) { - cmGeneratorExpression ge(&context->Backtrace); - + std::string linkedTargetsContent; std::string sep; std::string depString; - for (std::vector::const_iterator - it = targets.begin(); - it != targets.end(); ++it) + for (typename std::vector::const_iterator it = libraries.begin(); + it != libraries.end(); ++it) { - if (*it == target) + // 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 (it->Target && it->Target != target) { - // 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. - continue; + depString += + sep + "$Target->GetName() + "," + interfacePropertyName + ">"; + sep = ";"; } - depString += - sep + "$GetName() + "," + interfacePropertyName + ">"; - sep = ";"; } - cmsys::auto_ptr cge = ge.Parse(depString); - std::string linkedTargetsContent = cge->Evaluate(target->GetMakefile(), - context->Config, - context->Quiet, - headTarget, - target, - dagChecker); - if (cge->GetHadContextSensitiveCondition()) + if(!depString.empty()) { - context->HadContextSensitiveCondition = true; + cmGeneratorExpression ge(&context->Backtrace); + cmsys::auto_ptr cge = ge.Parse(depString); + linkedTargetsContent = cge->Evaluate(target->GetMakefile(), + context->Config, + context->Quiet, + headTarget, + target, + dagChecker); + if (cge->GetHadContextSensitiveCondition()) + { + context->HadContextSensitiveCondition = true; + } } return linkedTargetsContent; } -std::string -getLinkedTargetsContent( - std::vector const &libraries, - cmTarget const* target, - cmTarget const* headTarget, - cmGeneratorExpressionContext *context, - cmGeneratorExpressionDAGChecker *dagChecker, - const std::string &interfacePropertyName) -{ - std::vector tgts; - for (std::vector::const_iterator - it = libraries.begin(); - it != libraries.end(); ++it) - { - if (it->Target) - { - tgts.push_back(it->Target); - } - } - return getLinkedTargetsContent(tgts, target, headTarget, context, - dagChecker, interfacePropertyName); -} - //---------------------------------------------------------------------------- static const struct TargetPropertyNode : public cmGeneratorExpressionNode { @@ -1098,27 +1077,14 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode if (std::find_if(transBegin, transEnd, cmStrCmp(propertyName)) != transEnd) { - std::vector tgts; if(cmTarget::LinkInterfaceLibraries const* iface = target->GetLinkInterfaceLibraries(context->Config, headTarget, true)) { - for(std::vector::const_iterator - it = iface->Libraries.begin(); - it != iface->Libraries.end(); ++it) - { - if (it->Target) - { - tgts.push_back(it->Target); - } - } - } - if (!tgts.empty()) - { linkedTargetsContent = - getLinkedTargetsContent(tgts, target, - headTarget, - context, &dagChecker, - interfacePropertyName); + getLinkedTargetsContent(iface->Libraries, target, + headTarget, + context, &dagChecker, + interfacePropertyName); } } else if (std::find_if(transBegin, transEnd, From 60bafeb68404dc37434644f56e98000b3b0fff81 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Jul 2014 15:18:05 -0400 Subject: [PATCH 5/9] Genex: Avoid repeated search of transitive property whitelist In cmGeneratorExpressionEvaluator, avoid searching through the list of transitive interface property names repeatedly during evaluation of TargetPropertyNode. Simply record the results of the first search for later re-use. --- Source/cmGeneratorExpressionEvaluator.cxx | 59 ++++++++++------------- 1 file changed, 26 insertions(+), 33 deletions(-) diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index d08c3c9ed..ddcc39d68 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -1044,12 +1044,18 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode std::string linkedTargetsContent; std::string interfacePropertyName; + bool isInterfaceProperty = false; #define POPULATE_INTERFACE_PROPERTY_NAME(prop) \ - if (propertyName == #prop || propertyName == "INTERFACE_" #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) @@ -1065,17 +1071,10 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } } #undef POPULATE_INTERFACE_PROPERTY_NAME - cmTarget const* headTarget = context->HeadTarget ? context->HeadTarget : target; - const char * const *transBegin = - cmArrayBegin(targetPropertyTransitiveWhitelist) + 1; - const char * const *transEnd = - cmArrayEnd(targetPropertyTransitiveWhitelist); - - if (std::find_if(transBegin, transEnd, - cmStrCmp(propertyName)) != transEnd) + if(isInterfaceProperty) { if(cmTarget::LinkInterfaceLibraries const* iface = target->GetLinkInterfaceLibraries(context->Config, headTarget, true)) @@ -1087,18 +1086,17 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode interfacePropertyName); } } - else if (std::find_if(transBegin, transEnd, - cmStrCmp(interfacePropertyName)) != transEnd) + else if(!interfacePropertyName.empty()) { const cmTarget::LinkImplementationLibraries *impl = target->GetLinkImplementationLibraries(context->Config); if(impl) { linkedTargetsContent = - getLinkedTargetsContent(impl->Libraries, target, - headTarget, - context, &dagChecker, - interfacePropertyName); + getLinkedTargetsContent(impl->Libraries, target, + headTarget, + context, &dagChecker, + interfacePropertyName); } } @@ -1178,32 +1176,27 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode return propContent ? propContent : ""; } } - for (size_t i = 1; - i < cmArraySize(targetPropertyTransitiveWhitelist); - ++i) + if(!interfacePropertyName.empty()) { - if (targetPropertyTransitiveWhitelist[i] == interfacePropertyName) - { - cmGeneratorExpression ge(&context->Backtrace); - cmsys::auto_ptr cge = ge.Parse(prop); - cge->SetEvaluateForBuildsystem(context->EvaluateForBuildsystem); - std::string result = cge->Evaluate(context->Makefile, + cmGeneratorExpression ge(&context->Backtrace); + cmsys::auto_ptr cge = ge.Parse(prop); + cge->SetEvaluateForBuildsystem(context->EvaluateForBuildsystem); + std::string result = cge->Evaluate(context->Makefile, context->Config, context->Quiet, headTarget, target, &dagChecker); - if (cge->GetHadContextSensitiveCondition()) - { - context->HadContextSensitiveCondition = true; - } - if (!linkedTargetsContent.empty()) - { - result += (result.empty() ? "" : ";") + linkedTargetsContent; - } - return result; + if (cge->GetHadContextSensitiveCondition()) + { + context->HadContextSensitiveCondition = true; } + if (!linkedTargetsContent.empty()) + { + result += (result.empty() ? "" : ";") + linkedTargetsContent; + } + return result; } return prop; } From d5f0743d0f97fb44bcfaafd8680a00631d1c7d40 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Jul 2014 15:22:08 -0400 Subject: [PATCH 6/9] Genex: Refactor empty element strip In cmGeneratorExpressionEvaluator, teach getLinkedTargetsContent to call cmGeneratorExpression::StripEmptyListElements to transform its return value so that callers do not have to do so. --- Source/cmGeneratorExpressionEvaluator.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index ddcc39d68..db1d180ec 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -841,6 +841,8 @@ getLinkedTargetsContent( context->HadContextSensitiveCondition = true; } } + linkedTargetsContent = + cmGeneratorExpression::StripEmptyListElements(linkedTargetsContent); return linkedTargetsContent; } @@ -1100,9 +1102,6 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } } - linkedTargetsContent = - cmGeneratorExpression::StripEmptyListElements(linkedTargetsContent); - if (!prop) { if (target->IsImported() From 9d72df45057afd955d6bbb7ee2ceb62ab8dc777a Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Jul 2014 15:19:44 -0400 Subject: [PATCH 7/9] Genex: Adjust code layout slightly --- Source/cmGeneratorExpressionEvaluator.cxx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index db1d180ec..28879f1c5 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -1039,8 +1039,8 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode CM_FOR_EACH_TRANSITIVE_PROPERTY_METHOD( ASSERT_TRANSITIVE_PROPERTY_METHOD) false); - } #undef ASSERT_TRANSITIVE_PROPERTY_METHOD + } } std::string linkedTargetsContent; @@ -1090,9 +1090,8 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } else if(!interfacePropertyName.empty()) { - const cmTarget::LinkImplementationLibraries *impl - = target->GetLinkImplementationLibraries(context->Config); - if(impl) + if(cmTarget::LinkImplementationLibraries const* impl = + target->GetLinkImplementationLibraries(context->Config)) { linkedTargetsContent = getLinkedTargetsContent(impl->Libraries, target, From 89095514a7f6d7075e8d2fda1b88445b87a3bec8 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 16 Jul 2014 12:58:19 -0400 Subject: [PATCH 8/9] cmTarget: Refactor GetLinkImplementationClosure internals Store the 'Done' flag directly in each map entry instead of using a separate map. --- Source/cmTarget.cxx | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 8185bcc2c..d3a9bebfa 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -164,6 +164,13 @@ public: typedef std::map LinkClosureMapType; LinkClosureMapType LinkClosureMap; + struct LinkImplClosure: public std::vector + { + LinkImplClosure(): Done(false) {} + bool Done; + }; + std::map LinkImplClosureMap; + typedef std::map > SourceFilesMapType; SourceFilesMapType SourceFilesMap; @@ -203,15 +210,12 @@ public: CachedLinkInterfaceSourcesEntries; std::map > CachedLinkInterfaceCompileFeaturesEntries; - std::map > - CachedLinkImplementationClosure; std::map CacheLinkInterfaceIncludeDirectoriesDone; std::map CacheLinkInterfaceCompileDefinitionsDone; std::map CacheLinkInterfaceCompileOptionsDone; std::map CacheLinkInterfaceSourcesDone; std::map CacheLinkInterfaceCompileFeaturesDone; - std::map CacheLinkImplementationClosureDone; }; cmLinkImplItem cmTargetInternals::TargetPropertyEntry::NoLinkImplItem; @@ -6034,11 +6038,11 @@ void processILibs(const std::string& config, std::vector const& cmTarget::GetLinkImplementationClosure(const std::string& config) const { - std::vector& tgts = - this->Internal->CachedLinkImplementationClosure[config]; - if(!this->Internal->CacheLinkImplementationClosureDone[config]) + cmTargetInternals::LinkImplClosure& tgts = + this->Internal->LinkImplClosureMap[config]; + if(!tgts.Done) { - this->Internal->CacheLinkImplementationClosureDone[config] = true; + tgts.Done = true; std::set emitted; cmTarget::LinkImplementationLibraries const* impl From c45dd669abe746de2e9724044591d70397653d98 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 16 Jul 2014 11:16:28 -0400 Subject: [PATCH 9/9] cmTarget: Cache compatible interface property sets Replace isLinkDependentProperty with a CompatibleInterfaces structure that records all the compatible interface properties in a set for each type. This avoids repeatedly traversing the link implementation closure and asking every target for its compatible interface properties. --- Source/cmTarget.cxx | 93 ++++++++++++++++++++++----------------------- Source/cmTarget.h | 10 +++++ 2 files changed, 55 insertions(+), 48 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index d3a9bebfa..29029eb89 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -171,6 +171,13 @@ public: }; std::map LinkImplClosureMap; + struct CompatibleInterfaces: public cmTarget::CompatibleInterfaces + { + CompatibleInterfaces(): Done(false) {} + bool Done; + }; + std::map CompatibleInterfacesMap; + typedef std::map > SourceFilesMapType; SourceFilesMapType SourceFilesMap; @@ -5302,44 +5309,6 @@ const char * cmTarget::GetLinkInterfaceDependentNumberMaxProperty( NumberMaxType, 0); } -//---------------------------------------------------------------------------- -bool isLinkDependentProperty(cmTarget const* tgt, const std::string &p, - const std::string& interfaceProperty, - const std::string& config) -{ - std::vector const& deps = - tgt->GetLinkImplementationClosure(config); - - if(deps.empty()) - { - return false; - } - - for(std::vector::const_iterator li = deps.begin(); - li != deps.end(); ++li) - { - const char *prop = (*li)->GetProperty(interfaceProperty); - if (!prop) - { - continue; - } - - std::vector props; - cmSystemTools::ExpandListArgument(prop, props); - - for(std::vector::iterator pi = props.begin(); - pi != props.end(); ++pi) - { - if (*pi == p) - { - return true; - } - } - } - - return false; -} - //---------------------------------------------------------------------------- bool cmTarget::IsLinkInterfaceDependentBoolProperty(const std::string &p, const std::string& config) const @@ -5349,9 +5318,7 @@ bool cmTarget::IsLinkInterfaceDependentBoolProperty(const std::string &p, { return false; } - return (p == "POSITION_INDEPENDENT_CODE") || - isLinkDependentProperty(this, p, "COMPATIBLE_INTERFACE_BOOL", - config); + return this->GetCompatibleInterfaces(config).PropsBool.count(p) > 0; } //---------------------------------------------------------------------------- @@ -5363,9 +5330,7 @@ bool cmTarget::IsLinkInterfaceDependentStringProperty(const std::string &p, { return false; } - return (p == "AUTOUIC_OPTIONS") || - isLinkDependentProperty(this, p, "COMPATIBLE_INTERFACE_STRING", - config); + return this->GetCompatibleInterfaces(config).PropsString.count(p) > 0; } //---------------------------------------------------------------------------- @@ -5377,8 +5342,7 @@ bool cmTarget::IsLinkInterfaceDependentNumberMinProperty(const std::string &p, { return false; } - return isLinkDependentProperty(this, p, "COMPATIBLE_INTERFACE_NUMBER_MIN", - config); + return this->GetCompatibleInterfaces(config).PropsNumberMin.count(p) > 0; } //---------------------------------------------------------------------------- @@ -5390,8 +5354,7 @@ bool cmTarget::IsLinkInterfaceDependentNumberMaxProperty(const std::string &p, { return false; } - return isLinkDependentProperty(this, p, "COMPATIBLE_INTERFACE_NUMBER_MAX", - config); + return this->GetCompatibleInterfaces(config).PropsNumberMax.count(p) > 0; } //---------------------------------------------------------------------------- @@ -6058,6 +6021,40 @@ cmTarget::GetLinkImplementationClosure(const std::string& config) const return tgts; } +//---------------------------------------------------------------------------- +cmTarget::CompatibleInterfaces const& +cmTarget::GetCompatibleInterfaces(std::string const& config) const +{ + cmTargetInternals::CompatibleInterfaces& compat = + this->Internal->CompatibleInterfacesMap[config]; + if(!compat.Done) + { + compat.Done = true; + compat.PropsBool.insert("POSITION_INDEPENDENT_CODE"); + compat.PropsString.insert("AUTOUIC_OPTIONS"); + std::vector const& deps = + this->GetLinkImplementationClosure(config); + for(std::vector::const_iterator li = deps.begin(); + li != deps.end(); ++li) + { +#define CM_READ_COMPATIBLE_INTERFACE(X, x) \ + if(const char* prop = (*li)->GetProperty("COMPATIBLE_INTERFACE_" #X)) \ + { \ + std::vector props; \ + cmSystemTools::ExpandListArgument(prop, props); \ + std::copy(props.begin(), props.end(), \ + std::inserter(compat.Props##x, compat.Props##x.begin())); \ + } + CM_READ_COMPATIBLE_INTERFACE(BOOL, Bool) + CM_READ_COMPATIBLE_INTERFACE(STRING, String) + CM_READ_COMPATIBLE_INTERFACE(NUMBER_MIN, NumberMin) + CM_READ_COMPATIBLE_INTERFACE(NUMBER_MAX, NumberMax) +#undef CM_READ_COMPATIBLE_INTERFACE + } + } + return compat; +} + //---------------------------------------------------------------------------- void cmTargetInternals::ComputeLinkInterfaceLibraries( diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 12712725c..486315e09 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -300,6 +300,16 @@ public: std::vector const& GetLinkImplementationClosure(const std::string& config) const; + struct CompatibleInterfaces + { + std::set PropsBool; + std::set PropsString; + std::set PropsNumberMax; + std::set PropsNumberMin; + }; + CompatibleInterfaces const& + GetCompatibleInterfaces(std::string const& config) const; + /** The link implementation specifies the direct library dependencies needed by the object files of the target. */ struct LinkImplementationLibraries