From aaa5dbbf64392dad518668ac1d046d624dac41f3 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:22 +0200 Subject: [PATCH 01/19] cmTarget: Use static storage for computed properties Avoid having to populate a mutable container to return a value. --- Source/cmTarget.cxx | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 651bcc8cd9..ca2c37d8e0 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1100,8 +1100,9 @@ const char* cmTarget::GetProperty(const std::string& prop, // available configuration. // if (this->IsImported()) { - this->Properties.SetProperty( - propLOCATION, this->ImportedGetFullPath("", false).c_str()); + static std::string loc; + loc = this->ImportedGetFullPath("", false); + return loc.c_str(); } else { // For a non-imported target this is deprecated because it // cannot take into account the per-configuration name of the @@ -1112,7 +1113,9 @@ const char* cmTarget::GetProperty(const std::string& prop, gg->CreateGenerationObjects(); } cmGeneratorTarget* gt = gg->FindGeneratorTarget(this->GetName()); - this->Properties.SetProperty(propLOCATION, gt->GetLocationForBuild()); + static std::string loc; + loc = gt->GetLocationForBuild(); + return loc.c_str(); } } @@ -1125,16 +1128,18 @@ const char* cmTarget::GetProperty(const std::string& prop, const char* configName = prop.c_str() + 9; if (this->IsImported()) { - this->Properties.SetProperty( - prop, this->ImportedGetFullPath(configName, false).c_str()); + static std::string loc; + loc = this->ImportedGetFullPath(configName, false); + return loc.c_str(); } else { cmGlobalGenerator* gg = this->Makefile->GetGlobalGenerator(); if (!gg->GetConfigureDoneCMP0026()) { gg->CreateGenerationObjects(); } cmGeneratorTarget* gt = gg->FindGeneratorTarget(this->GetName()); - this->Properties.SetProperty( - prop, gt->GetFullPath(configName, false).c_str()); + static std::string loc; + loc = gt->GetFullPath(configName, false); + return loc.c_str(); } } // Support "_LOCATION". @@ -1146,16 +1151,18 @@ const char* cmTarget::GetProperty(const std::string& prop, return CM_NULLPTR; } if (this->IsImported()) { - this->Properties.SetProperty( - prop, this->ImportedGetFullPath(configName, false).c_str()); + static std::string loc; + loc = this->ImportedGetFullPath(configName, false); + return loc.c_str(); } else { cmGlobalGenerator* gg = this->Makefile->GetGlobalGenerator(); if (!gg->GetConfigureDoneCMP0026()) { gg->CreateGenerationObjects(); } cmGeneratorTarget* gt = gg->FindGeneratorTarget(this->GetName()); - this->Properties.SetProperty( - prop, gt->GetFullPath(configName, false).c_str()); + static std::string loc; + loc = gt->GetFullPath(configName, false); + return loc.c_str(); } } } @@ -1330,7 +1337,9 @@ const char* cmTarget::GetProperty(const std::string& prop, } } } - this->Properties.SetProperty("SOURCES", ss.str().c_str()); + static std::string srcs; + srcs = ss.str(); + return srcs.c_str(); } } From 60b3f216c18aa317656433bcc0b724e6abd8d02f Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:22 +0200 Subject: [PATCH 02/19] cmTarget: Remove mutable marker from properties --- Source/cmGeneratorTarget.cxx | 2 +- Source/cmTarget.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 7eb0ebff64..d622ad99bd 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -3950,7 +3950,7 @@ void cmGeneratorTarget::ComputeVersionedName(std::string& vName, std::vector cmGeneratorTarget::GetPropertyKeys() const { - cmPropertyMap propsObject = this->Target->GetProperties(); + cmPropertyMap const& propsObject = this->Target->GetProperties(); std::vector props; props.reserve(propsObject.size()); for (cmPropertyMap::const_iterator it = propsObject.begin(); diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 3d88688e9c..1c81081e05 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -208,7 +208,7 @@ public: } // Get the properties - cmPropertyMap& GetProperties() const { return this->Properties; } + cmPropertyMap const& GetProperties() const { return this->Properties; } bool GetMappedConfig(std::string const& desired_config, const char** loc, const char** imp, std::string& suffix) const; @@ -282,7 +282,7 @@ private: bool implib) const; private: - mutable cmPropertyMap Properties; + cmPropertyMap Properties; std::set SystemIncludeDirectories; std::set LinkDirectoriesEmmitted; std::set Utilities; From 1fb6f672bd2bd041edb772b0627278383a00fb62 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:23 +0200 Subject: [PATCH 03/19] cmTarget: Move SOURCES property computation before the rest The SOURCES have to be computed. --- Source/cmTarget.cxx | 170 ++++++++++++++++++++++---------------------- 1 file changed, 85 insertions(+), 85 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index ca2c37d8e0..129dd63e31 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1167,6 +1167,91 @@ const char* cmTarget::GetProperty(const std::string& prop, } } } + if (prop == "SOURCES") { + if (this->Internal->SourceEntries.empty()) { + return CM_NULLPTR; + } + + std::ostringstream ss; + const char* sep = ""; + for (std::vector::const_iterator i = + this->Internal->SourceEntries.begin(); + i != this->Internal->SourceEntries.end(); ++i) { + std::string const& entry = *i; + + std::vector files; + cmSystemTools::ExpandListArgument(entry, files); + for (std::vector::const_iterator li = files.begin(); + li != files.end(); ++li) { + if (cmHasLiteralPrefix(*li, "$size() - 1] == '>') { + std::string objLibName = li->substr(17, li->size() - 18); + + if (cmGeneratorExpression::Find(objLibName) != std::string::npos) { + ss << sep; + sep = ";"; + ss << *li; + continue; + } + + bool addContent = false; + bool noMessage = true; + std::ostringstream e; + cmake::MessageType messageType = cmake::AUTHOR_WARNING; + switch (context->GetPolicyStatus(cmPolicies::CMP0051)) { + case cmPolicies::WARN: + e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; + noMessage = false; + case cmPolicies::OLD: + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::NEW: + addContent = true; + } + if (!noMessage) { + e << "Target \"" << this->Name + << "\" contains " + "$ generator expression in its sources " + "list. " + "This content was not previously part of the SOURCES " + "property " + "when that property was read at configure time. Code " + "reading " + "that property needs to be adapted to ignore the generator " + "expression using the string(GENEX_STRIP) command."; + context->IssueMessage(messageType, e.str()); + } + if (addContent) { + ss << sep; + sep = ";"; + ss << *li; + } + } else if (cmGeneratorExpression::Find(*li) == std::string::npos) { + ss << sep; + sep = ";"; + ss << *li; + } else { + cmSourceFile* sf = this->Makefile->GetOrCreateSource(*li); + // Construct what is known about this source file location. + cmSourceFileLocation const& location = sf->GetLocation(); + std::string sname = location.GetDirectory(); + if (!sname.empty()) { + sname += "/"; + } + sname += location.GetName(); + + ss << sep; + sep = ";"; + // Append this list entry. + ss << sname; + } + } + } + static std::string srcs; + srcs = ss.str(); + return srcs.c_str(); + } static UNORDERED_SET specialProps; #define MAKE_STATIC_PROP(PROP) static const std::string prop##PROP = #PROP MAKE_STATIC_PROP(LINK_LIBRARIES); @@ -1256,91 +1341,6 @@ const char* cmTarget::GetProperty(const std::string& prop, if (prop == propSOURCE_DIR) { return this->GetMakefile()->GetCurrentSourceDirectory(); } - if (prop == propSOURCES) { - if (this->Internal->SourceEntries.empty()) { - return CM_NULLPTR; - } - - std::ostringstream ss; - const char* sep = ""; - for (std::vector::const_iterator i = - this->Internal->SourceEntries.begin(); - i != this->Internal->SourceEntries.end(); ++i) { - std::string const& entry = *i; - - std::vector files; - cmSystemTools::ExpandListArgument(entry, files); - for (std::vector::const_iterator li = files.begin(); - li != files.end(); ++li) { - if (cmHasLiteralPrefix(*li, "$size() - 1] == '>') { - std::string objLibName = li->substr(17, li->size() - 18); - - if (cmGeneratorExpression::Find(objLibName) != std::string::npos) { - ss << sep; - sep = ";"; - ss << *li; - continue; - } - - bool addContent = false; - bool noMessage = true; - std::ostringstream e; - cmake::MessageType messageType = cmake::AUTHOR_WARNING; - switch (context->GetPolicyStatus(cmPolicies::CMP0051)) { - case cmPolicies::WARN: - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; - noMessage = false; - case cmPolicies::OLD: - break; - case cmPolicies::REQUIRED_ALWAYS: - case cmPolicies::REQUIRED_IF_USED: - case cmPolicies::NEW: - addContent = true; - } - if (!noMessage) { - e << "Target \"" << this->Name - << "\" contains " - "$ generator expression in its sources " - "list. " - "This content was not previously part of the SOURCES " - "property " - "when that property was read at configure time. Code " - "reading " - "that property needs to be adapted to ignore the generator " - "expression using the string(GENEX_STRIP) command."; - context->IssueMessage(messageType, e.str()); - } - if (addContent) { - ss << sep; - sep = ";"; - ss << *li; - } - } else if (cmGeneratorExpression::Find(*li) == std::string::npos) { - ss << sep; - sep = ";"; - ss << *li; - } else { - cmSourceFile* sf = this->Makefile->GetOrCreateSource(*li); - // Construct what is known about this source file location. - cmSourceFileLocation const& location = sf->GetLocation(); - std::string sname = location.GetDirectory(); - if (!sname.empty()) { - sname += "/"; - } - sname += location.GetName(); - - ss << sep; - sep = ";"; - // Append this list entry. - ss << sname; - } - } - } - static std::string srcs; - srcs = ss.str(); - return srcs.c_str(); - } } const char* retVal = this->Properties.GetPropertyValue(prop); From 705fcf522be16eee03b1757274b23ada6547e6bd Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:23 +0200 Subject: [PATCH 04/19] cmTarget: Move IMPORTED check to callers This way the policy can be checked without depending on cmTarget. --- Source/cmTarget.cxx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 129dd63e31..62e2d1ccec 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1032,9 +1032,6 @@ void cmTarget::CheckProperty(const std::string& prop, bool cmTarget::HandleLocationPropertyPolicy(cmMakefile* context) const { - if (this->IsImported()) { - return true; - } std::ostringstream e; const char* modal = CM_NULLPTR; cmake::MessageType messageType = cmake::AUTHOR_WARNING; @@ -1090,7 +1087,9 @@ const char* cmTarget::GetProperty(const std::string& prop, this->GetType() == cmState::UNKNOWN_LIBRARY) { static const std::string propLOCATION = "LOCATION"; if (prop == propLOCATION) { - if (!this->HandleLocationPropertyPolicy(context)) { + + if (!this->IsImported() && + !this->HandleLocationPropertyPolicy(context)) { return CM_NULLPTR; } @@ -1122,7 +1121,8 @@ const char* cmTarget::GetProperty(const std::string& prop, // Support "LOCATION_". else if (cmHasLiteralPrefix(prop, "LOCATION_")) { - if (!this->HandleLocationPropertyPolicy(context)) { + if (!this->IsImported() && + !this->HandleLocationPropertyPolicy(context)) { return CM_NULLPTR; } const char* configName = prop.c_str() + 9; @@ -1147,7 +1147,8 @@ const char* cmTarget::GetProperty(const std::string& prop, !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { std::string configName(prop.c_str(), prop.size() - 9); if (configName != "IMPORTED") { - if (!this->HandleLocationPropertyPolicy(context)) { + if (!this->IsImported() && + !this->HandleLocationPropertyPolicy(context)) { return CM_NULLPTR; } if (this->IsImported()) { From a55cac4ba475ff61da4146272246512a52247323 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:23 +0200 Subject: [PATCH 05/19] cmTarget: Split property computation into separate class Everything related to property computation will be moved here and eventually shared with cmGeneratorTarget. --- Source/cmTarget.cxx | 124 +++++++++++++++++++++++++++----------------- Source/cmTarget.h | 8 ++- 2 files changed, 78 insertions(+), 54 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 62e2d1ccec..eb19df5502 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1030,7 +1030,19 @@ void cmTarget::CheckProperty(const std::string& prop, } } -bool cmTarget::HandleLocationPropertyPolicy(cmMakefile* context) const +class cmTargetPropertyComputer +{ +public: + static const char* GetProperty(cmTarget const* tgt, const std::string& prop, + cmMakefile* context); + +private: + static bool HandleLocationPropertyPolicy(std::string const& tgtName, + cmMakefile* context); +}; + +bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( + std::string const& tgtName, cmMakefile* context) { std::ostringstream e; const char* modal = CM_NULLPTR; @@ -1050,7 +1062,7 @@ bool cmTarget::HandleLocationPropertyPolicy(cmMakefile* context) const if (modal) { e << "The LOCATION property " << modal << " not be read from target \"" - << this->GetName() + << tgtName << "\". Use the target name directly with " "add_custom_command, or use the generator expression $, " "as appropriate.\n"; @@ -1060,36 +1072,21 @@ bool cmTarget::HandleLocationPropertyPolicy(cmMakefile* context) const return messageType != cmake::FATAL_ERROR; } -const char* cmTarget::GetProperty(const std::string& prop) const +const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, + const std::string& prop, + cmMakefile* context) { - return this->GetProperty(prop, this->Makefile); -} - -const char* cmTarget::GetProperty(const std::string& prop, - cmMakefile* context) const -{ - if (this->GetType() == cmState::INTERFACE_LIBRARY && - !whiteListedInterfaceProperty(prop)) { - std::ostringstream e; - e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " - "The property \"" - << prop << "\" is not allowed."; - context->IssueMessage(cmake::FATAL_ERROR, e.str()); - return CM_NULLPTR; - } - // Watch for special "computed" properties that are dependent on // other properties or variables. Always recompute them. - if (this->GetType() == cmState::EXECUTABLE || - this->GetType() == cmState::STATIC_LIBRARY || - this->GetType() == cmState::SHARED_LIBRARY || - this->GetType() == cmState::MODULE_LIBRARY || - this->GetType() == cmState::UNKNOWN_LIBRARY) { + if (tgt->GetType() == cmState::EXECUTABLE || + tgt->GetType() == cmState::STATIC_LIBRARY || + tgt->GetType() == cmState::SHARED_LIBRARY || + tgt->GetType() == cmState::MODULE_LIBRARY || + tgt->GetType() == cmState::UNKNOWN_LIBRARY) { static const std::string propLOCATION = "LOCATION"; if (prop == propLOCATION) { - - if (!this->IsImported() && - !this->HandleLocationPropertyPolicy(context)) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), context)) { return CM_NULLPTR; } @@ -1098,20 +1095,20 @@ const char* cmTarget::GetProperty(const std::string& prop, // For an imported target this is the location of an arbitrary // available configuration. // - if (this->IsImported()) { + if (tgt->IsImported()) { static std::string loc; - loc = this->ImportedGetFullPath("", false); + loc = tgt->ImportedGetFullPath("", false); return loc.c_str(); } else { // For a non-imported target this is deprecated because it // cannot take into account the per-configuration name of the // target because the configuration type may not be known at // CMake time. - cmGlobalGenerator* gg = this->Makefile->GetGlobalGenerator(); + cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); if (!gg->GetConfigureDoneCMP0026()) { gg->CreateGenerationObjects(); } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(this->GetName()); + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); static std::string loc; loc = gt->GetLocationForBuild(); return loc.c_str(); @@ -1121,22 +1118,22 @@ const char* cmTarget::GetProperty(const std::string& prop, // Support "LOCATION_". else if (cmHasLiteralPrefix(prop, "LOCATION_")) { - if (!this->IsImported() && - !this->HandleLocationPropertyPolicy(context)) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), context)) { return CM_NULLPTR; } const char* configName = prop.c_str() + 9; - if (this->IsImported()) { + if (tgt->IsImported()) { static std::string loc; - loc = this->ImportedGetFullPath(configName, false); + loc = tgt->ImportedGetFullPath(configName, false); return loc.c_str(); } else { - cmGlobalGenerator* gg = this->Makefile->GetGlobalGenerator(); + cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); if (!gg->GetConfigureDoneCMP0026()) { gg->CreateGenerationObjects(); } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(this->GetName()); + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); static std::string loc; loc = gt->GetFullPath(configName, false); return loc.c_str(); @@ -1147,20 +1144,20 @@ const char* cmTarget::GetProperty(const std::string& prop, !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { std::string configName(prop.c_str(), prop.size() - 9); if (configName != "IMPORTED") { - if (!this->IsImported() && - !this->HandleLocationPropertyPolicy(context)) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), context)) { return CM_NULLPTR; } - if (this->IsImported()) { + if (tgt->IsImported()) { static std::string loc; - loc = this->ImportedGetFullPath(configName, false); + loc = tgt->ImportedGetFullPath(configName, false); return loc.c_str(); } else { - cmGlobalGenerator* gg = this->Makefile->GetGlobalGenerator(); + cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); if (!gg->GetConfigureDoneCMP0026()) { gg->CreateGenerationObjects(); } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(this->GetName()); + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); static std::string loc; loc = gt->GetFullPath(configName, false); return loc.c_str(); @@ -1169,15 +1166,15 @@ const char* cmTarget::GetProperty(const std::string& prop, } } if (prop == "SOURCES") { - if (this->Internal->SourceEntries.empty()) { + cmStringRange entries = tgt->GetSourceEntries(); + if (entries.empty()) { return CM_NULLPTR; } std::ostringstream ss; const char* sep = ""; - for (std::vector::const_iterator i = - this->Internal->SourceEntries.begin(); - i != this->Internal->SourceEntries.end(); ++i) { + for (std::vector::const_iterator i = entries.begin(); + i != entries.end(); ++i) { std::string const& entry = *i; std::vector files; @@ -1211,7 +1208,7 @@ const char* cmTarget::GetProperty(const std::string& prop, addContent = true; } if (!noMessage) { - e << "Target \"" << this->Name + e << "Target \"" << tgt->GetName() << "\" contains " "$ generator expression in its sources " "list. " @@ -1233,7 +1230,7 @@ const char* cmTarget::GetProperty(const std::string& prop, sep = ";"; ss << *li; } else { - cmSourceFile* sf = this->Makefile->GetOrCreateSource(*li); + cmSourceFile* sf = tgt->GetMakefile()->GetOrCreateSource(*li); // Construct what is known about this source file location. cmSourceFileLocation const& location = sf->GetLocation(); std::string sname = location.GetDirectory(); @@ -1253,6 +1250,35 @@ const char* cmTarget::GetProperty(const std::string& prop, srcs = ss.str(); return srcs.c_str(); } + return CM_NULLPTR; +} + +const char* cmTarget::GetProperty(const std::string& prop) const +{ + return this->GetProperty(prop, this->Makefile); +} + +const char* cmTarget::GetProperty(const std::string& prop, + cmMakefile* context) const +{ + if (this->GetType() == cmState::INTERFACE_LIBRARY && + !whiteListedInterfaceProperty(prop)) { + std::ostringstream e; + e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " + "The property \"" + << prop << "\" is not allowed."; + context->IssueMessage(cmake::FATAL_ERROR, e.str()); + return CM_NULLPTR; + } + + if (const char* result = + cmTargetPropertyComputer::GetProperty(this, prop, context)) { + return result; + } + if (cmSystemTools::GetFatalErrorOccured()) { + return CM_NULLPTR; + } + static UNORDERED_SET specialProps; #define MAKE_STATIC_PROP(PROP) static const std::string prop##PROP = #PROP MAKE_STATIC_PROP(LINK_LIBRARIES); diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 1c81081e05..84f43b3918 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -267,9 +267,10 @@ public: bool operator()(cmTarget const* t1, cmTarget const* t2) const; }; -private: - bool HandleLocationPropertyPolicy(cmMakefile* context) const; + std::string ImportedGetFullPath(const std::string& config, + bool implib) const; +private: const char* GetSuffixVariableInternal(bool implib) const; const char* GetPrefixVariableInternal(bool implib) const; @@ -278,9 +279,6 @@ private: void SetPropertyDefault(const std::string& property, const char* default_value); - std::string ImportedGetFullPath(const std::string& config, - bool implib) const; - private: cmPropertyMap Properties; std::set SystemIncludeDirectories; From 7d57c1a2c926011bc2e4c7a65b802763ec4efbcb Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:23 +0200 Subject: [PATCH 06/19] cmTarget: Extract location computation methods --- Source/cmTarget.cxx | 98 ++++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 54 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index eb19df5502..0913725046 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1039,6 +1039,10 @@ public: private: static bool HandleLocationPropertyPolicy(std::string const& tgtName, cmMakefile* context); + + static const char* ComputeLocationForBuild(cmTarget const* tgt); + static const char* ComputeLocation(cmTarget const* tgt, + std::string const& config); }; bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( @@ -1072,6 +1076,42 @@ bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( return messageType != cmake::FATAL_ERROR; } +const char* cmTargetPropertyComputer::ComputeLocationForBuild( + cmTarget const* tgt) +{ + static std::string loc; + if (tgt->IsImported()) { + loc = tgt->ImportedGetFullPath("", false); + return loc.c_str(); + } + + cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); + if (!gg->GetConfigureDoneCMP0026()) { + gg->CreateGenerationObjects(); + } + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); + loc = gt->GetLocationForBuild(); + return loc.c_str(); +} + +const char* cmTargetPropertyComputer::ComputeLocation( + cmTarget const* tgt, std::string const& config) +{ + static std::string loc; + if (tgt->IsImported()) { + loc = tgt->ImportedGetFullPath(config, false); + return loc.c_str(); + } + + cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); + if (!gg->GetConfigureDoneCMP0026()) { + gg->CreateGenerationObjects(); + } + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); + loc = gt->GetFullPath(config, false); + return loc.c_str(); +} + const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, const std::string& prop, cmMakefile* context) @@ -1089,31 +1129,7 @@ const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, !HandleLocationPropertyPolicy(tgt->GetName(), context)) { return CM_NULLPTR; } - - // Set the LOCATION property of the target. - // - // For an imported target this is the location of an arbitrary - // available configuration. - // - if (tgt->IsImported()) { - static std::string loc; - loc = tgt->ImportedGetFullPath("", false); - return loc.c_str(); - } else { - // For a non-imported target this is deprecated because it - // cannot take into account the per-configuration name of the - // target because the configuration type may not be known at - // CMake time. - cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); - if (!gg->GetConfigureDoneCMP0026()) { - gg->CreateGenerationObjects(); - } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); - static std::string loc; - loc = gt->GetLocationForBuild(); - return loc.c_str(); - } - + return ComputeLocationForBuild(tgt); } // Support "LOCATION_". @@ -1123,22 +1139,9 @@ const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, return CM_NULLPTR; } const char* configName = prop.c_str() + 9; - - if (tgt->IsImported()) { - static std::string loc; - loc = tgt->ImportedGetFullPath(configName, false); - return loc.c_str(); - } else { - cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); - if (!gg->GetConfigureDoneCMP0026()) { - gg->CreateGenerationObjects(); - } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); - static std::string loc; - loc = gt->GetFullPath(configName, false); - return loc.c_str(); - } + return ComputeLocation(tgt, configName); } + // Support "_LOCATION". else if (cmHasLiteralSuffix(prop, "_LOCATION") && !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { @@ -1148,20 +1151,7 @@ const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, !HandleLocationPropertyPolicy(tgt->GetName(), context)) { return CM_NULLPTR; } - if (tgt->IsImported()) { - static std::string loc; - loc = tgt->ImportedGetFullPath(configName, false); - return loc.c_str(); - } else { - cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); - if (!gg->GetConfigureDoneCMP0026()) { - gg->CreateGenerationObjects(); - } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); - static std::string loc; - loc = gt->GetFullPath(configName, false); - return loc.c_str(); - } + return ComputeLocation(tgt, configName); } } } From 8096682e4edb542a48bbf66c085911db5f00be02 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:23 +0200 Subject: [PATCH 07/19] cmTarget: Extract GetSources method --- Source/cmTarget.cxx | 175 +++++++++++++++++++++++--------------------- 1 file changed, 92 insertions(+), 83 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 0913725046..f39cd4b613 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1043,6 +1043,8 @@ private: static const char* ComputeLocationForBuild(cmTarget const* tgt); static const char* ComputeLocation(cmTarget const* tgt, std::string const& config); + + static const char* GetSources(cmTarget const* tgt, cmMakefile* context); }; bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( @@ -1112,6 +1114,95 @@ const char* cmTargetPropertyComputer::ComputeLocation( return loc.c_str(); } +const char* cmTargetPropertyComputer::GetSources(cmTarget const* tgt, + cmMakefile* context) +{ + cmStringRange entries = tgt->GetSourceEntries(); + + if (entries.empty()) { + return CM_NULLPTR; + } + + std::ostringstream ss; + const char* sep = ""; + for (std::vector::const_iterator i = entries.begin(); + i != entries.end(); ++i) { + std::string const& entry = *i; + + std::vector files; + cmSystemTools::ExpandListArgument(entry, files); + for (std::vector::const_iterator li = files.begin(); + li != files.end(); ++li) { + if (cmHasLiteralPrefix(*li, "$size() - 1] == '>') { + std::string objLibName = li->substr(17, li->size() - 18); + + if (cmGeneratorExpression::Find(objLibName) != std::string::npos) { + ss << sep; + sep = ";"; + ss << *li; + continue; + } + + bool addContent = false; + bool noMessage = true; + std::ostringstream e; + cmake::MessageType messageType = cmake::AUTHOR_WARNING; + switch (context->GetPolicyStatus(cmPolicies::CMP0051)) { + case cmPolicies::WARN: + e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; + noMessage = false; + case cmPolicies::OLD: + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::NEW: + addContent = true; + } + if (!noMessage) { + e << "Target \"" << tgt->GetName() + << "\" contains " + "$ generator expression in its sources " + "list. " + "This content was not previously part of the SOURCES " + "property " + "when that property was read at configure time. Code " + "reading " + "that property needs to be adapted to ignore the generator " + "expression using the string(GENEX_STRIP) command."; + context->IssueMessage(messageType, e.str()); + } + if (addContent) { + ss << sep; + sep = ";"; + ss << *li; + } + } else if (cmGeneratorExpression::Find(*li) == std::string::npos) { + ss << sep; + sep = ";"; + ss << *li; + } else { + cmSourceFile* sf = tgt->GetMakefile()->GetOrCreateSource(*li); + // Construct what is known about this source file location. + cmSourceFileLocation const& location = sf->GetLocation(); + std::string sname = location.GetDirectory(); + if (!sname.empty()) { + sname += "/"; + } + sname += location.GetName(); + + ss << sep; + sep = ";"; + // Append this list entry. + ss << sname; + } + } + } + static std::string srcs; + srcs = ss.str(); + return srcs.c_str(); +} + const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, const std::string& prop, cmMakefile* context) @@ -1156,89 +1247,7 @@ const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, } } if (prop == "SOURCES") { - cmStringRange entries = tgt->GetSourceEntries(); - if (entries.empty()) { - return CM_NULLPTR; - } - - std::ostringstream ss; - const char* sep = ""; - for (std::vector::const_iterator i = entries.begin(); - i != entries.end(); ++i) { - std::string const& entry = *i; - - std::vector files; - cmSystemTools::ExpandListArgument(entry, files); - for (std::vector::const_iterator li = files.begin(); - li != files.end(); ++li) { - if (cmHasLiteralPrefix(*li, "$size() - 1] == '>') { - std::string objLibName = li->substr(17, li->size() - 18); - - if (cmGeneratorExpression::Find(objLibName) != std::string::npos) { - ss << sep; - sep = ";"; - ss << *li; - continue; - } - - bool addContent = false; - bool noMessage = true; - std::ostringstream e; - cmake::MessageType messageType = cmake::AUTHOR_WARNING; - switch (context->GetPolicyStatus(cmPolicies::CMP0051)) { - case cmPolicies::WARN: - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; - noMessage = false; - case cmPolicies::OLD: - break; - case cmPolicies::REQUIRED_ALWAYS: - case cmPolicies::REQUIRED_IF_USED: - case cmPolicies::NEW: - addContent = true; - } - if (!noMessage) { - e << "Target \"" << tgt->GetName() - << "\" contains " - "$ generator expression in its sources " - "list. " - "This content was not previously part of the SOURCES " - "property " - "when that property was read at configure time. Code " - "reading " - "that property needs to be adapted to ignore the generator " - "expression using the string(GENEX_STRIP) command."; - context->IssueMessage(messageType, e.str()); - } - if (addContent) { - ss << sep; - sep = ";"; - ss << *li; - } - } else if (cmGeneratorExpression::Find(*li) == std::string::npos) { - ss << sep; - sep = ";"; - ss << *li; - } else { - cmSourceFile* sf = tgt->GetMakefile()->GetOrCreateSource(*li); - // Construct what is known about this source file location. - cmSourceFileLocation const& location = sf->GetLocation(); - std::string sname = location.GetDirectory(); - if (!sname.empty()) { - sname += "/"; - } - sname += location.GetName(); - - ss << sep; - sep = ";"; - // Append this list entry. - ss << sname; - } - } - } - static std::string srcs; - srcs = ss.str(); - return srcs.c_str(); + return GetSources(tgt, context); } return CM_NULLPTR; } From 7863fba1f6f37db2f75834d0d6a3fbbc680e731c Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:24 +0200 Subject: [PATCH 08/19] cmTarget: Extract GetLocation method --- Source/cmTarget.cxx | 92 ++++++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 38 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index f39cd4b613..c9aabb837e 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1043,6 +1043,9 @@ private: static const char* ComputeLocationForBuild(cmTarget const* tgt); static const char* ComputeLocation(cmTarget const* tgt, std::string const& config); + static const char* GetLocation(cmTarget const* tgt, + std::string const& config, + cmMakefile* context); static const char* GetSources(cmTarget const* tgt, cmMakefile* context); }; @@ -1114,6 +1117,52 @@ const char* cmTargetPropertyComputer::ComputeLocation( return loc.c_str(); } +const char* cmTargetPropertyComputer::GetLocation(cmTarget const* tgt, + const std::string& prop, + cmMakefile* context) +{ + // Watch for special "computed" properties that are dependent on + // other properties or variables. Always recompute them. + if (tgt->GetType() == cmState::EXECUTABLE || + tgt->GetType() == cmState::STATIC_LIBRARY || + tgt->GetType() == cmState::SHARED_LIBRARY || + tgt->GetType() == cmState::MODULE_LIBRARY || + tgt->GetType() == cmState::UNKNOWN_LIBRARY) { + static const std::string propLOCATION = "LOCATION"; + if (prop == propLOCATION) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), context)) { + return CM_NULLPTR; + } + return ComputeLocationForBuild(tgt); + } + + // Support "LOCATION_". + else if (cmHasLiteralPrefix(prop, "LOCATION_")) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), context)) { + return CM_NULLPTR; + } + const char* configName = prop.c_str() + 9; + return ComputeLocation(tgt, configName); + } + + // Support "_LOCATION". + else if (cmHasLiteralSuffix(prop, "_LOCATION") && + !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { + std::string configName(prop.c_str(), prop.size() - 9); + if (configName != "IMPORTED") { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), context)) { + return CM_NULLPTR; + } + return ComputeLocation(tgt, configName); + } + } + } + return CM_NULLPTR; +} + const char* cmTargetPropertyComputer::GetSources(cmTarget const* tgt, cmMakefile* context) { @@ -1207,44 +1256,11 @@ const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, const std::string& prop, cmMakefile* context) { - // Watch for special "computed" properties that are dependent on - // other properties or variables. Always recompute them. - if (tgt->GetType() == cmState::EXECUTABLE || - tgt->GetType() == cmState::STATIC_LIBRARY || - tgt->GetType() == cmState::SHARED_LIBRARY || - tgt->GetType() == cmState::MODULE_LIBRARY || - tgt->GetType() == cmState::UNKNOWN_LIBRARY) { - static const std::string propLOCATION = "LOCATION"; - if (prop == propLOCATION) { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), context)) { - return CM_NULLPTR; - } - return ComputeLocationForBuild(tgt); - } - - // Support "LOCATION_". - else if (cmHasLiteralPrefix(prop, "LOCATION_")) { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), context)) { - return CM_NULLPTR; - } - const char* configName = prop.c_str() + 9; - return ComputeLocation(tgt, configName); - } - - // Support "_LOCATION". - else if (cmHasLiteralSuffix(prop, "_LOCATION") && - !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { - std::string configName(prop.c_str(), prop.size() - 9); - if (configName != "IMPORTED") { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), context)) { - return CM_NULLPTR; - } - return ComputeLocation(tgt, configName); - } - } + if (const char* loc = GetLocation(tgt, prop, context)) { + return loc; + } + if (cmSystemTools::GetFatalErrorOccured()) { + return CM_NULLPTR; } if (prop == "SOURCES") { return GetSources(tgt, context); From e32a6bdd990571bafb8537110128d28a593150b7 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:24 +0200 Subject: [PATCH 09/19] cmListFileBacktrace: Add a method to retrieve the Bottom of a snapshot --- Source/cmListFileCache.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index 4dacee0740..fd779c7d3b 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -120,6 +120,8 @@ public: cmListFileBacktrace& operator=(cmListFileBacktrace const& r); ~cmListFileBacktrace(); + cmState::Snapshot GetBottom() const { return this->Bottom; } + // Get a backtrace with the given file scope added to the top. // May not be called until after construction with a valid snapshot. cmListFileBacktrace Push(std::string const& file) const; From 390a7d8647c13570dc6416fceb884dc51c8ef6f9 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:24 +0200 Subject: [PATCH 10/19] cmTargetPropertyComputer: Implement GetProperty without cmMakefile Only a cmMessenger and a backtrace are needed. --- Source/cmTarget.cxx | 56 ++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index c9aabb837e..27ea157a12 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -8,6 +8,7 @@ #include "cmGlobalGenerator.h" #include "cmListFileCache.h" #include "cmMakefile.h" +#include "cmMessenger.h" #include "cmOutputConverter.h" #include "cmProperty.h" #include "cmSourceFile.h" @@ -1034,29 +1035,34 @@ class cmTargetPropertyComputer { public: static const char* GetProperty(cmTarget const* tgt, const std::string& prop, - cmMakefile* context); + cmMessenger* messenger, + cmListFileBacktrace const& context); private: static bool HandleLocationPropertyPolicy(std::string const& tgtName, - cmMakefile* context); + cmMessenger* messenger, + cmListFileBacktrace const& context); static const char* ComputeLocationForBuild(cmTarget const* tgt); static const char* ComputeLocation(cmTarget const* tgt, std::string const& config); static const char* GetLocation(cmTarget const* tgt, std::string const& config, - cmMakefile* context); + cmMessenger* messenger, + cmListFileBacktrace const& context); - static const char* GetSources(cmTarget const* tgt, cmMakefile* context); + static const char* GetSources(cmTarget const* tgt, cmMessenger* messenger, + cmListFileBacktrace const& context); }; bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( - std::string const& tgtName, cmMakefile* context) + std::string const& tgtName, cmMessenger* messenger, + cmListFileBacktrace const& context) { std::ostringstream e; const char* modal = CM_NULLPTR; cmake::MessageType messageType = cmake::AUTHOR_WARNING; - switch (context->GetPolicyStatus(cmPolicies::CMP0026)) { + switch (context.GetBottom().GetPolicy(cmPolicies::CMP0026)) { case cmPolicies::WARN: e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0026) << "\n"; modal = "should"; @@ -1075,7 +1081,7 @@ bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( << "\". Use the target name directly with " "add_custom_command, or use the generator expression $, " "as appropriate.\n"; - context->IssueMessage(messageType, e.str()); + messenger->IssueMessage(messageType, e.str(), context); } return messageType != cmake::FATAL_ERROR; @@ -1117,9 +1123,9 @@ const char* cmTargetPropertyComputer::ComputeLocation( return loc.c_str(); } -const char* cmTargetPropertyComputer::GetLocation(cmTarget const* tgt, - const std::string& prop, - cmMakefile* context) +const char* cmTargetPropertyComputer::GetLocation( + cmTarget const* tgt, const std::string& prop, cmMessenger* messenger, + cmListFileBacktrace const& context) { // Watch for special "computed" properties that are dependent on // other properties or variables. Always recompute them. @@ -1131,7 +1137,7 @@ const char* cmTargetPropertyComputer::GetLocation(cmTarget const* tgt, static const std::string propLOCATION = "LOCATION"; if (prop == propLOCATION) { if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), context)) { + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, context)) { return CM_NULLPTR; } return ComputeLocationForBuild(tgt); @@ -1140,7 +1146,7 @@ const char* cmTargetPropertyComputer::GetLocation(cmTarget const* tgt, // Support "LOCATION_". else if (cmHasLiteralPrefix(prop, "LOCATION_")) { if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), context)) { + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, context)) { return CM_NULLPTR; } const char* configName = prop.c_str() + 9; @@ -1153,7 +1159,8 @@ const char* cmTargetPropertyComputer::GetLocation(cmTarget const* tgt, std::string configName(prop.c_str(), prop.size() - 9); if (configName != "IMPORTED") { if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), context)) { + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, + context)) { return CM_NULLPTR; } return ComputeLocation(tgt, configName); @@ -1163,8 +1170,9 @@ const char* cmTargetPropertyComputer::GetLocation(cmTarget const* tgt, return CM_NULLPTR; } -const char* cmTargetPropertyComputer::GetSources(cmTarget const* tgt, - cmMakefile* context) +const char* cmTargetPropertyComputer::GetSources( + cmTarget const* tgt, cmMessenger* messenger, + cmListFileBacktrace const& context) { cmStringRange entries = tgt->GetSourceEntries(); @@ -1197,7 +1205,7 @@ const char* cmTargetPropertyComputer::GetSources(cmTarget const* tgt, bool noMessage = true; std::ostringstream e; cmake::MessageType messageType = cmake::AUTHOR_WARNING; - switch (context->GetPolicyStatus(cmPolicies::CMP0051)) { + switch (context.GetBottom().GetPolicy(cmPolicies::CMP0051)) { case cmPolicies::WARN: e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; noMessage = false; @@ -1219,7 +1227,7 @@ const char* cmTargetPropertyComputer::GetSources(cmTarget const* tgt, "reading " "that property needs to be adapted to ignore the generator " "expression using the string(GENEX_STRIP) command."; - context->IssueMessage(messageType, e.str()); + messenger->IssueMessage(messageType, e.str(), context); } if (addContent) { ss << sep; @@ -1252,18 +1260,18 @@ const char* cmTargetPropertyComputer::GetSources(cmTarget const* tgt, return srcs.c_str(); } -const char* cmTargetPropertyComputer::GetProperty(cmTarget const* tgt, - const std::string& prop, - cmMakefile* context) +const char* cmTargetPropertyComputer::GetProperty( + cmTarget const* tgt, const std::string& prop, cmMessenger* messenger, + cmListFileBacktrace const& context) { - if (const char* loc = GetLocation(tgt, prop, context)) { + if (const char* loc = GetLocation(tgt, prop, messenger, context)) { return loc; } if (cmSystemTools::GetFatalErrorOccured()) { return CM_NULLPTR; } if (prop == "SOURCES") { - return GetSources(tgt, context); + return GetSources(tgt, messenger, context); } return CM_NULLPTR; } @@ -1286,8 +1294,8 @@ const char* cmTarget::GetProperty(const std::string& prop, return CM_NULLPTR; } - if (const char* result = - cmTargetPropertyComputer::GetProperty(this, prop, context)) { + if (const char* result = cmTargetPropertyComputer::GetProperty( + this, prop, context->GetMessenger(), context->GetBacktrace())) { return result; } if (cmSystemTools::GetFatalErrorOccured()) { From fbf1721c94b40ea86eeb183a1916f2066eb64bdc Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:24 +0200 Subject: [PATCH 11/19] cmTargetPropertyComputer: Extract into new files --- Source/CMakeLists.txt | 2 + Source/cmTarget.cxx | 246 +--------------------------- Source/cmTargetPropertyComputer.cxx | 243 +++++++++++++++++++++++++++ Source/cmTargetPropertyComputer.h | 43 +++++ bootstrap | 1 + 5 files changed, 290 insertions(+), 245 deletions(-) create mode 100644 Source/cmTargetPropertyComputer.cxx create mode 100644 Source/cmTargetPropertyComputer.h diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index 048667af30..c9e77c30d0 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -362,6 +362,8 @@ set(SRCS cmSystemTools.h cmTarget.cxx cmTarget.h + cmTargetPropertyComputer.cxx + cmTargetPropertyComputer.h cmTargetExport.h cmTest.cxx cmTest.h diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 27ea157a12..63a6fe9ba5 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -14,6 +14,7 @@ #include "cmSourceFile.h" #include "cmSourceFileLocation.h" #include "cmSystemTools.h" +#include "cmTargetPropertyComputer.h" #include "cmake.h" #include @@ -1031,251 +1032,6 @@ void cmTarget::CheckProperty(const std::string& prop, } } -class cmTargetPropertyComputer -{ -public: - static const char* GetProperty(cmTarget const* tgt, const std::string& prop, - cmMessenger* messenger, - cmListFileBacktrace const& context); - -private: - static bool HandleLocationPropertyPolicy(std::string const& tgtName, - cmMessenger* messenger, - cmListFileBacktrace const& context); - - static const char* ComputeLocationForBuild(cmTarget const* tgt); - static const char* ComputeLocation(cmTarget const* tgt, - std::string const& config); - static const char* GetLocation(cmTarget const* tgt, - std::string const& config, - cmMessenger* messenger, - cmListFileBacktrace const& context); - - static const char* GetSources(cmTarget const* tgt, cmMessenger* messenger, - cmListFileBacktrace const& context); -}; - -bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( - std::string const& tgtName, cmMessenger* messenger, - cmListFileBacktrace const& context) -{ - std::ostringstream e; - const char* modal = CM_NULLPTR; - cmake::MessageType messageType = cmake::AUTHOR_WARNING; - switch (context.GetBottom().GetPolicy(cmPolicies::CMP0026)) { - case cmPolicies::WARN: - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0026) << "\n"; - modal = "should"; - case cmPolicies::OLD: - break; - case cmPolicies::REQUIRED_ALWAYS: - case cmPolicies::REQUIRED_IF_USED: - case cmPolicies::NEW: - modal = "may"; - messageType = cmake::FATAL_ERROR; - } - - if (modal) { - e << "The LOCATION property " << modal << " not be read from target \"" - << tgtName - << "\". Use the target name directly with " - "add_custom_command, or use the generator expression $, " - "as appropriate.\n"; - messenger->IssueMessage(messageType, e.str(), context); - } - - return messageType != cmake::FATAL_ERROR; -} - -const char* cmTargetPropertyComputer::ComputeLocationForBuild( - cmTarget const* tgt) -{ - static std::string loc; - if (tgt->IsImported()) { - loc = tgt->ImportedGetFullPath("", false); - return loc.c_str(); - } - - cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); - if (!gg->GetConfigureDoneCMP0026()) { - gg->CreateGenerationObjects(); - } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); - loc = gt->GetLocationForBuild(); - return loc.c_str(); -} - -const char* cmTargetPropertyComputer::ComputeLocation( - cmTarget const* tgt, std::string const& config) -{ - static std::string loc; - if (tgt->IsImported()) { - loc = tgt->ImportedGetFullPath(config, false); - return loc.c_str(); - } - - cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); - if (!gg->GetConfigureDoneCMP0026()) { - gg->CreateGenerationObjects(); - } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); - loc = gt->GetFullPath(config, false); - return loc.c_str(); -} - -const char* cmTargetPropertyComputer::GetLocation( - cmTarget const* tgt, const std::string& prop, cmMessenger* messenger, - cmListFileBacktrace const& context) -{ - // Watch for special "computed" properties that are dependent on - // other properties or variables. Always recompute them. - if (tgt->GetType() == cmState::EXECUTABLE || - tgt->GetType() == cmState::STATIC_LIBRARY || - tgt->GetType() == cmState::SHARED_LIBRARY || - tgt->GetType() == cmState::MODULE_LIBRARY || - tgt->GetType() == cmState::UNKNOWN_LIBRARY) { - static const std::string propLOCATION = "LOCATION"; - if (prop == propLOCATION) { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, context)) { - return CM_NULLPTR; - } - return ComputeLocationForBuild(tgt); - } - - // Support "LOCATION_". - else if (cmHasLiteralPrefix(prop, "LOCATION_")) { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, context)) { - return CM_NULLPTR; - } - const char* configName = prop.c_str() + 9; - return ComputeLocation(tgt, configName); - } - - // Support "_LOCATION". - else if (cmHasLiteralSuffix(prop, "_LOCATION") && - !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { - std::string configName(prop.c_str(), prop.size() - 9); - if (configName != "IMPORTED") { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, - context)) { - return CM_NULLPTR; - } - return ComputeLocation(tgt, configName); - } - } - } - return CM_NULLPTR; -} - -const char* cmTargetPropertyComputer::GetSources( - cmTarget const* tgt, cmMessenger* messenger, - cmListFileBacktrace const& context) -{ - cmStringRange entries = tgt->GetSourceEntries(); - - if (entries.empty()) { - return CM_NULLPTR; - } - - std::ostringstream ss; - const char* sep = ""; - for (std::vector::const_iterator i = entries.begin(); - i != entries.end(); ++i) { - std::string const& entry = *i; - - std::vector files; - cmSystemTools::ExpandListArgument(entry, files); - for (std::vector::const_iterator li = files.begin(); - li != files.end(); ++li) { - if (cmHasLiteralPrefix(*li, "$size() - 1] == '>') { - std::string objLibName = li->substr(17, li->size() - 18); - - if (cmGeneratorExpression::Find(objLibName) != std::string::npos) { - ss << sep; - sep = ";"; - ss << *li; - continue; - } - - bool addContent = false; - bool noMessage = true; - std::ostringstream e; - cmake::MessageType messageType = cmake::AUTHOR_WARNING; - switch (context.GetBottom().GetPolicy(cmPolicies::CMP0051)) { - case cmPolicies::WARN: - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; - noMessage = false; - case cmPolicies::OLD: - break; - case cmPolicies::REQUIRED_ALWAYS: - case cmPolicies::REQUIRED_IF_USED: - case cmPolicies::NEW: - addContent = true; - } - if (!noMessage) { - e << "Target \"" << tgt->GetName() - << "\" contains " - "$ generator expression in its sources " - "list. " - "This content was not previously part of the SOURCES " - "property " - "when that property was read at configure time. Code " - "reading " - "that property needs to be adapted to ignore the generator " - "expression using the string(GENEX_STRIP) command."; - messenger->IssueMessage(messageType, e.str(), context); - } - if (addContent) { - ss << sep; - sep = ";"; - ss << *li; - } - } else if (cmGeneratorExpression::Find(*li) == std::string::npos) { - ss << sep; - sep = ";"; - ss << *li; - } else { - cmSourceFile* sf = tgt->GetMakefile()->GetOrCreateSource(*li); - // Construct what is known about this source file location. - cmSourceFileLocation const& location = sf->GetLocation(); - std::string sname = location.GetDirectory(); - if (!sname.empty()) { - sname += "/"; - } - sname += location.GetName(); - - ss << sep; - sep = ";"; - // Append this list entry. - ss << sname; - } - } - } - static std::string srcs; - srcs = ss.str(); - return srcs.c_str(); -} - -const char* cmTargetPropertyComputer::GetProperty( - cmTarget const* tgt, const std::string& prop, cmMessenger* messenger, - cmListFileBacktrace const& context) -{ - if (const char* loc = GetLocation(tgt, prop, messenger, context)) { - return loc; - } - if (cmSystemTools::GetFatalErrorOccured()) { - return CM_NULLPTR; - } - if (prop == "SOURCES") { - return GetSources(tgt, messenger, context); - } - return CM_NULLPTR; -} - const char* cmTarget::GetProperty(const std::string& prop) const { return this->GetProperty(prop, this->Makefile); diff --git a/Source/cmTargetPropertyComputer.cxx b/Source/cmTargetPropertyComputer.cxx new file mode 100644 index 0000000000..f15cb4394c --- /dev/null +++ b/Source/cmTargetPropertyComputer.cxx @@ -0,0 +1,243 @@ +/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying + file Copyright.txt or https://cmake.org/licensing for details. */ + +#include "cmTargetPropertyComputer.h" + +#include "cmGeneratorTarget.h" +#include "cmGlobalGenerator.h" +#include "cmMakefile.h" +#include "cmMessenger.h" +#include "cmSourceFile.h" +#include "cmSourceFileLocation.h" +#include "cmSystemTools.h" +#include "cmTarget.h" + +#if defined(CMake_HAVE_CXX_UNORDERED_SET) +#include +#define UNORDERED_SET std::unordered_set +#elif defined(CMAKE_BUILD_WITH_CMAKE) +#include +#define UNORDERED_SET cmsys::hash_set +#else +#define UNORDERED_SET std::set +#endif + +bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( + std::string const& tgtName, cmMessenger* messenger, + cmListFileBacktrace const& context) +{ + std::ostringstream e; + const char* modal = CM_NULLPTR; + cmake::MessageType messageType = cmake::AUTHOR_WARNING; + switch (context.GetBottom().GetPolicy(cmPolicies::CMP0026)) { + case cmPolicies::WARN: + e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0026) << "\n"; + modal = "should"; + case cmPolicies::OLD: + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::NEW: + modal = "may"; + messageType = cmake::FATAL_ERROR; + } + + if (modal) { + e << "The LOCATION property " << modal << " not be read from target \"" + << tgtName + << "\". Use the target name directly with " + "add_custom_command, or use the generator expression $, " + "as appropriate.\n"; + messenger->IssueMessage(messageType, e.str(), context); + } + + return messageType != cmake::FATAL_ERROR; +} + +const char* cmTargetPropertyComputer::ComputeLocationForBuild( + cmTarget const* tgt) +{ + static std::string loc; + if (tgt->IsImported()) { + loc = tgt->ImportedGetFullPath("", false); + return loc.c_str(); + } + + cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); + if (!gg->GetConfigureDoneCMP0026()) { + gg->CreateGenerationObjects(); + } + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); + loc = gt->GetLocationForBuild(); + return loc.c_str(); +} + +const char* cmTargetPropertyComputer::ComputeLocation( + cmTarget const* tgt, std::string const& config) +{ + static std::string loc; + if (tgt->IsImported()) { + loc = tgt->ImportedGetFullPath(config, false); + return loc.c_str(); + } + + cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); + if (!gg->GetConfigureDoneCMP0026()) { + gg->CreateGenerationObjects(); + } + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); + loc = gt->GetFullPath(config, false); + return loc.c_str(); +} + +const char* cmTargetPropertyComputer::GetProperty( + cmTarget const* tgt, const std::string& prop, cmMessenger* messenger, + cmListFileBacktrace const& context) +{ + if (const char* loc = GetLocation(tgt, prop, messenger, context)) { + return loc; + } + if (cmSystemTools::GetFatalErrorOccured()) { + return CM_NULLPTR; + } + if (prop == "SOURCES") { + return GetSources(tgt, messenger, context); + } + return CM_NULLPTR; +} + +const char* cmTargetPropertyComputer::GetLocation( + cmTarget const* tgt, const std::string& prop, cmMessenger* messenger, + cmListFileBacktrace const& context) +{ + // Watch for special "computed" properties that are dependent on + // other properties or variables. Always recompute them. + if (tgt->GetType() == cmState::EXECUTABLE || + tgt->GetType() == cmState::STATIC_LIBRARY || + tgt->GetType() == cmState::SHARED_LIBRARY || + tgt->GetType() == cmState::MODULE_LIBRARY || + tgt->GetType() == cmState::UNKNOWN_LIBRARY) { + static const std::string propLOCATION = "LOCATION"; + if (prop == propLOCATION) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, context)) { + return CM_NULLPTR; + } + return ComputeLocationForBuild(tgt); + } + + // Support "LOCATION_". + else if (cmHasLiteralPrefix(prop, "LOCATION_")) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, context)) { + return CM_NULLPTR; + } + const char* configName = prop.c_str() + 9; + return ComputeLocation(tgt, configName); + } + + // Support "_LOCATION". + else if (cmHasLiteralSuffix(prop, "_LOCATION") && + !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { + std::string configName(prop.c_str(), prop.size() - 9); + if (configName != "IMPORTED") { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, + context)) { + return CM_NULLPTR; + } + return ComputeLocation(tgt, configName); + } + } + } + return CM_NULLPTR; +} + +const char* cmTargetPropertyComputer::GetSources( + cmTarget const* tgt,cmMessenger* messenger, + cmListFileBacktrace const& context) +{ + cmStringRange entries = tgt->GetSourceEntries(); + if (entries.empty()) { + return CM_NULLPTR; + } + + std::ostringstream ss; + const char* sep = ""; + for (std::vector::const_iterator i = entries.begin(); + i != entries.end(); ++i) { + std::string const& entry = *i; + + std::vector files; + cmSystemTools::ExpandListArgument(entry, files); + for (std::vector::const_iterator li = files.begin(); + li != files.end(); ++li) { + if (cmHasLiteralPrefix(*li, "$size() - 1] == '>') { + std::string objLibName = li->substr(17, li->size() - 18); + + if (cmGeneratorExpression::Find(objLibName) != std::string::npos) { + ss << sep; + sep = ";"; + ss << *li; + continue; + } + + bool addContent = false; + bool noMessage = true; + std::ostringstream e; + cmake::MessageType messageType = cmake::AUTHOR_WARNING; + switch (context.GetBottom().GetPolicy(cmPolicies::CMP0051)) { + case cmPolicies::WARN: + e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; + noMessage = false; + case cmPolicies::OLD: + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::NEW: + addContent = true; + } + if (!noMessage) { + e << "Target \"" << tgt->GetName() + << "\" contains " + "$ generator expression in its sources " + "list. " + "This content was not previously part of the SOURCES " + "property " + "when that property was read at configure time. Code " + "reading " + "that property needs to be adapted to ignore the generator " + "expression using the string(GENEX_STRIP) command."; + messenger->IssueMessage(messageType, e.str(), context); + } + if (addContent) { + ss << sep; + sep = ";"; + ss << *li; + } + } else if (cmGeneratorExpression::Find(*li) == std::string::npos) { + ss << sep; + sep = ";"; + ss << *li; + } else { + cmSourceFile* sf = tgt->GetMakefile()->GetOrCreateSource(*li); + // Construct what is known about this source file location. + cmSourceFileLocation const& location = sf->GetLocation(); + std::string sname = location.GetDirectory(); + if (!sname.empty()) { + sname += "/"; + } + sname += location.GetName(); + + ss << sep; + sep = ";"; + // Append this list entry. + ss << sname; + } + } + } + static std::string srcs; + srcs = ss.str(); + return srcs.c_str(); +} diff --git a/Source/cmTargetPropertyComputer.h b/Source/cmTargetPropertyComputer.h new file mode 100644 index 0000000000..b7c1df4498 --- /dev/null +++ b/Source/cmTargetPropertyComputer.h @@ -0,0 +1,43 @@ +/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying + file Copyright.txt or https://cmake.org/licensing for details. */ +#ifndef cmTargetPropertyComputer_h +#define cmTargetPropertyComputer_h + +#include // IWYU pragma: keep + +#include "cmListFileCache.h" + +#include +#include + +class cmTarget; +class cmMessenger; + +class cmTargetPropertyComputer +{ +public: + static const char* GetProperty(cmTarget const* tgt, const std::string& prop, + cmMessenger* messenger, + cmListFileBacktrace const& context); + + static std::map ComputeFileLocations( + cmTarget const* tgt); + +private: + static bool HandleLocationPropertyPolicy(std::string const& tgtName, + cmMessenger* messenger, + cmListFileBacktrace const& context); + + static const char* ComputeLocationForBuild(cmTarget const* tgt); + static const char* ComputeLocation(cmTarget const* tgt, + std::string const& config); + static const char* GetLocation(cmTarget const* tgt, std::string const& prop, + cmMessenger* messenger, + cmListFileBacktrace const& context); + + static const char* GetSources(cmTarget const* tgt, + cmMessenger* messenger, + cmListFileBacktrace const& context); +}; + +#endif diff --git a/bootstrap b/bootstrap index fb8b1eb6e3..15963c012b 100755 --- a/bootstrap +++ b/bootstrap @@ -310,6 +310,7 @@ CMAKE_CXX_SOURCES="\ cmBootstrapCommands2 \ cmCommandsForBootstrap \ cmTarget \ + cmTargetPropertyComputer \ cmTest \ cmCustomCommand \ cmCustomCommandGenerator \ From 05251e6d80b03ae14d6a89765f14c2eb10979bd4 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:24 +0200 Subject: [PATCH 12/19] cmTargetPropertyComputer: Move whitelist check from cmTarget --- Source/cmTarget.cxx | 34 +++-------------------------- Source/cmTargetPropertyComputer.cxx | 31 +++++++++++++++++++++++++- Source/cmTargetPropertyComputer.h | 7 +++--- 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 63a6fe9ba5..3d16c50a74 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -707,38 +707,10 @@ cmBacktraceRange cmTarget::GetLinkImplementationBacktraces() const return cmMakeRange(this->Internal->LinkImplementationPropertyBacktraces); } -static bool whiteListedInterfaceProperty(const std::string& prop) -{ - if (cmHasLiteralPrefix(prop, "INTERFACE_")) { - return true; - } - static UNORDERED_SET builtIns; - if (builtIns.empty()) { - builtIns.insert("COMPATIBLE_INTERFACE_BOOL"); - builtIns.insert("COMPATIBLE_INTERFACE_NUMBER_MAX"); - builtIns.insert("COMPATIBLE_INTERFACE_NUMBER_MIN"); - builtIns.insert("COMPATIBLE_INTERFACE_STRING"); - builtIns.insert("EXPORT_NAME"); - builtIns.insert("IMPORTED"); - builtIns.insert("NAME"); - builtIns.insert("TYPE"); - } - - if (builtIns.count(prop)) { - return true; - } - - if (cmHasLiteralPrefix(prop, "MAP_IMPORTED_CONFIG_")) { - return true; - } - - return false; -} - void cmTarget::SetProperty(const std::string& prop, const char* value) { if (this->GetType() == cmState::INTERFACE_LIBRARY && - !whiteListedInterfaceProperty(prop)) { + !cmTargetPropertyComputer::WhiteListedInterfaceProperty(prop)) { std::ostringstream e; e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " "The property \"" @@ -822,7 +794,7 @@ void cmTarget::AppendProperty(const std::string& prop, const char* value, bool asString) { if (this->GetType() == cmState::INTERFACE_LIBRARY && - !whiteListedInterfaceProperty(prop)) { + !cmTargetPropertyComputer::WhiteListedInterfaceProperty(prop)) { std::ostringstream e; e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " "The property \"" @@ -1041,7 +1013,7 @@ const char* cmTarget::GetProperty(const std::string& prop, cmMakefile* context) const { if (this->GetType() == cmState::INTERFACE_LIBRARY && - !whiteListedInterfaceProperty(prop)) { + !cmTargetPropertyComputer::WhiteListedInterfaceProperty(prop)) { std::ostringstream e; e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " "The property \"" diff --git a/Source/cmTargetPropertyComputer.cxx b/Source/cmTargetPropertyComputer.cxx index f15cb4394c..18135fa01d 100644 --- a/Source/cmTargetPropertyComputer.cxx +++ b/Source/cmTargetPropertyComputer.cxx @@ -154,7 +154,7 @@ const char* cmTargetPropertyComputer::GetLocation( } const char* cmTargetPropertyComputer::GetSources( - cmTarget const* tgt,cmMessenger* messenger, + cmTarget const* tgt, cmMessenger* messenger, cmListFileBacktrace const& context) { cmStringRange entries = tgt->GetSourceEntries(); @@ -241,3 +241,32 @@ const char* cmTargetPropertyComputer::GetSources( srcs = ss.str(); return srcs.c_str(); } + +bool cmTargetPropertyComputer::WhiteListedInterfaceProperty( + const std::string& prop) +{ + if (cmHasLiteralPrefix(prop, "INTERFACE_")) { + return true; + } + static UNORDERED_SET builtIns; + if (builtIns.empty()) { + builtIns.insert("COMPATIBLE_INTERFACE_BOOL"); + builtIns.insert("COMPATIBLE_INTERFACE_NUMBER_MAX"); + builtIns.insert("COMPATIBLE_INTERFACE_NUMBER_MIN"); + builtIns.insert("COMPATIBLE_INTERFACE_STRING"); + builtIns.insert("EXPORT_NAME"); + builtIns.insert("IMPORTED"); + builtIns.insert("NAME"); + builtIns.insert("TYPE"); + } + + if (builtIns.count(prop)) { + return true; + } + + if (cmHasLiteralPrefix(prop, "MAP_IMPORTED_CONFIG_")) { + return true; + } + + return false; +} diff --git a/Source/cmTargetPropertyComputer.h b/Source/cmTargetPropertyComputer.h index b7c1df4498..1d2e4ed3e4 100644 --- a/Source/cmTargetPropertyComputer.h +++ b/Source/cmTargetPropertyComputer.h @@ -23,6 +23,8 @@ public: static std::map ComputeFileLocations( cmTarget const* tgt); + static bool WhiteListedInterfaceProperty(const std::string& prop); + private: static bool HandleLocationPropertyPolicy(std::string const& tgtName, cmMessenger* messenger, @@ -35,9 +37,8 @@ private: cmMessenger* messenger, cmListFileBacktrace const& context); - static const char* GetSources(cmTarget const* tgt, - cmMessenger* messenger, - cmListFileBacktrace const& context); + static const char* GetSources(cmTarget const* tgt, cmMessenger* messenger, + cmListFileBacktrace const& context); }; #endif From 637e3f3ee131cf30f7337db06314f4a1af22c245 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:24 +0200 Subject: [PATCH 13/19] cmTargetPropertyComputer: Unify whitelist handling from cmTarget --- Source/cmTarget.cxx | 30 +++++++++-------------------- Source/cmTargetPropertyComputer.cxx | 16 +++++++++++++++ Source/cmTargetPropertyComputer.h | 4 ++++ 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 3d16c50a74..52f1d0432d 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -709,13 +709,9 @@ cmBacktraceRange cmTarget::GetLinkImplementationBacktraces() const void cmTarget::SetProperty(const std::string& prop, const char* value) { - if (this->GetType() == cmState::INTERFACE_LIBRARY && - !cmTargetPropertyComputer::WhiteListedInterfaceProperty(prop)) { - std::ostringstream e; - e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " - "The property \"" - << prop << "\" is not allowed."; - this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); + if (!cmTargetPropertyComputer::PassesWhitelist( + this->GetType(), prop, this->Makefile->GetMessenger(), + this->Makefile->GetBacktrace())) { return; } if (prop == "NAME") { @@ -793,13 +789,9 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) void cmTarget::AppendProperty(const std::string& prop, const char* value, bool asString) { - if (this->GetType() == cmState::INTERFACE_LIBRARY && - !cmTargetPropertyComputer::WhiteListedInterfaceProperty(prop)) { - std::ostringstream e; - e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " - "The property \"" - << prop << "\" is not allowed."; - this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); + if (!cmTargetPropertyComputer::PassesWhitelist( + this->GetType(), prop, this->Makefile->GetMessenger(), + this->Makefile->GetBacktrace())) { return; } if (prop == "NAME") { @@ -1012,13 +1004,9 @@ const char* cmTarget::GetProperty(const std::string& prop) const const char* cmTarget::GetProperty(const std::string& prop, cmMakefile* context) const { - if (this->GetType() == cmState::INTERFACE_LIBRARY && - !cmTargetPropertyComputer::WhiteListedInterfaceProperty(prop)) { - std::ostringstream e; - e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " - "The property \"" - << prop << "\" is not allowed."; - context->IssueMessage(cmake::FATAL_ERROR, e.str()); + if (!cmTargetPropertyComputer::PassesWhitelist(this->GetType(), prop, + context->GetMessenger(), + context->GetBacktrace())) { return CM_NULLPTR; } diff --git a/Source/cmTargetPropertyComputer.cxx b/Source/cmTargetPropertyComputer.cxx index 18135fa01d..3e39a8f84c 100644 --- a/Source/cmTargetPropertyComputer.cxx +++ b/Source/cmTargetPropertyComputer.cxx @@ -270,3 +270,19 @@ bool cmTargetPropertyComputer::WhiteListedInterfaceProperty( return false; } + +bool cmTargetPropertyComputer::PassesWhitelist( + cmState::TargetType tgtType, std::string const& prop, cmMessenger* messenger, + cmListFileBacktrace const& context) +{ + if (tgtType == cmState::INTERFACE_LIBRARY && + !WhiteListedInterfaceProperty(prop)) { + std::ostringstream e; + e << "INTERFACE_LIBRARY targets may only have whitelisted properties. " + "The property \"" + << prop << "\" is not allowed."; + messenger->IssueMessage(cmake::FATAL_ERROR, e.str(), context); + return false; + } + return true; +} diff --git a/Source/cmTargetPropertyComputer.h b/Source/cmTargetPropertyComputer.h index 1d2e4ed3e4..941614bdcd 100644 --- a/Source/cmTargetPropertyComputer.h +++ b/Source/cmTargetPropertyComputer.h @@ -25,6 +25,10 @@ public: static bool WhiteListedInterfaceProperty(const std::string& prop); + static bool PassesWhitelist(cmState::TargetType tgtType, + std::string const& prop, cmMessenger* messenger, + cmListFileBacktrace const& context); + private: static bool HandleLocationPropertyPolicy(std::string const& tgtName, cmMessenger* messenger, From a0a720e6a70133e361762101ce69a0b3f1ab244d Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:25 +0200 Subject: [PATCH 14/19] cm{,Generator}Target: Add global generator accessors Provide 'static polymorphism' between the types in this aspect so that they can be used indiscriminately in a C++ template. --- Source/cmGeneratorTarget.cxx | 5 +++++ Source/cmGeneratorTarget.h | 2 ++ Source/cmTarget.cxx | 5 +++++ Source/cmTarget.h | 3 +++ 4 files changed, 15 insertions(+) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index d622ad99bd..fb732a6032 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -320,6 +320,11 @@ cmGeneratorTarget::~cmGeneratorTarget() cmDeleteAll(this->LinkInformation); } +cmGlobalGenerator* cmGeneratorTarget::GetGlobalGenerator() const +{ + return this->GetLocalGenerator()->GetGlobalGenerator(); +} + cmLocalGenerator* cmGeneratorTarget::GetLocalGenerator() const { return this->LocalGenerator; diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 68ffd5c8d8..bfbd790243 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -32,6 +32,8 @@ public: cmLocalGenerator* GetLocalGenerator() const; + cmGlobalGenerator* GetGlobalGenerator() const; + bool IsImported() const; bool IsImportedGloballyVisible() const; const char* GetLocation(const std::string& config) const; diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 52f1d0432d..76a12d4b23 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -272,6 +272,11 @@ cmTarget::cmTarget(std::string const& name, cmState::TargetType type, } } +cmGlobalGenerator* cmTarget::GetGlobalGenerator() const +{ + return this->GetMakefile()->GetGlobalGenerator(); +} + void cmTarget::AddUtility(const std::string& u, cmMakefile* makefile) { if (this->Utilities.insert(u).second && makefile) { diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 84f43b3918..2d259ed322 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -30,6 +30,7 @@ class cmMakefile; class cmSourceFile; +class cmGlobalGenerator; class cmTargetInternals; class cmTargetInternalPointer @@ -76,6 +77,8 @@ public: */ cmState::TargetType GetType() const { return this->TargetTypeValue; } + cmGlobalGenerator* GetGlobalGenerator() const; + ///! Set/Get the name of the target const std::string& GetName() const { return this->Name; } From 848ae2a663ef83e059561d94b33d551154642231 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:25 +0200 Subject: [PATCH 15/19] cmTargetPropertyComputer: Template some methods on the Target Implement the methods for cmTarget in the cmTarget source. --- Source/cmTarget.cxx | 128 +++++++++++++++++++ Source/cmTargetPropertyComputer.cxx | 189 ---------------------------- Source/cmTargetPropertyComputer.h | 84 +++++++++++-- 3 files changed, 201 insertions(+), 200 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 76a12d4b23..9b2043c391 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -35,6 +35,134 @@ #define UNORDERED_SET std::set #endif +template <> +const char* cmTargetPropertyComputer::ComputeLocationForBuild( + cmTarget const* tgt) +{ + static std::string loc; + if (tgt->IsImported()) { + loc = tgt->ImportedGetFullPath("", false); + return loc.c_str(); + } + + cmGlobalGenerator* gg = tgt->GetGlobalGenerator(); + if (!gg->GetConfigureDoneCMP0026()) { + gg->CreateGenerationObjects(); + } + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); + loc = gt->GetLocationForBuild(); + return loc.c_str(); +} + +template <> +const char* cmTargetPropertyComputer::ComputeLocation( + cmTarget const* tgt, const std::string& config) +{ + static std::string loc; + if (tgt->IsImported()) { + loc = tgt->ImportedGetFullPath(config, false); + return loc.c_str(); + } + + cmGlobalGenerator* gg = tgt->GetGlobalGenerator(); + if (!gg->GetConfigureDoneCMP0026()) { + gg->CreateGenerationObjects(); + } + cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); + loc = gt->GetFullPath(config, false); + return loc.c_str(); +} + +template <> +const char* cmTargetPropertyComputer::GetSources( + cmTarget const* tgt, cmMessenger* messenger, + cmListFileBacktrace const& context) +{ + cmStringRange entries = tgt->GetSourceEntries(); + if (entries.empty()) { + return CM_NULLPTR; + } + + std::ostringstream ss; + const char* sep = ""; + for (std::vector::const_iterator i = entries.begin(); + i != entries.end(); ++i) { + std::string const& entry = *i; + + std::vector files; + cmSystemTools::ExpandListArgument(entry, files); + for (std::vector::const_iterator li = files.begin(); + li != files.end(); ++li) { + if (cmHasLiteralPrefix(*li, "$size() - 1] == '>') { + std::string objLibName = li->substr(17, li->size() - 18); + + if (cmGeneratorExpression::Find(objLibName) != std::string::npos) { + ss << sep; + sep = ";"; + ss << *li; + continue; + } + + bool addContent = false; + bool noMessage = true; + std::ostringstream e; + cmake::MessageType messageType = cmake::AUTHOR_WARNING; + switch (context.GetBottom().GetPolicy(cmPolicies::CMP0051)) { + case cmPolicies::WARN: + e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; + noMessage = false; + case cmPolicies::OLD: + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::NEW: + addContent = true; + } + if (!noMessage) { + e << "Target \"" << tgt->GetName() + << "\" contains " + "$ generator expression in its sources " + "list. " + "This content was not previously part of the SOURCES " + "property " + "when that property was read at configure time. Code " + "reading " + "that property needs to be adapted to ignore the generator " + "expression using the string(GENEX_STRIP) command."; + messenger->IssueMessage(messageType, e.str(), context); + } + if (addContent) { + ss << sep; + sep = ";"; + ss << *li; + } + } else if (cmGeneratorExpression::Find(*li) == std::string::npos) { + ss << sep; + sep = ";"; + ss << *li; + } else { + cmSourceFile* sf = tgt->GetMakefile()->GetOrCreateSource(*li); + // Construct what is known about this source file location. + cmSourceFileLocation const& location = sf->GetLocation(); + std::string sname = location.GetDirectory(); + if (!sname.empty()) { + sname += "/"; + } + sname += location.GetName(); + + ss << sep; + sep = ";"; + // Append this list entry. + ss << sname; + } + } + } + static std::string srcs; + srcs = ss.str(); + return srcs.c_str(); +} + class cmTargetInternals { public: diff --git a/Source/cmTargetPropertyComputer.cxx b/Source/cmTargetPropertyComputer.cxx index 3e39a8f84c..21408d10e1 100644 --- a/Source/cmTargetPropertyComputer.cxx +++ b/Source/cmTargetPropertyComputer.cxx @@ -9,7 +9,6 @@ #include "cmMessenger.h" #include "cmSourceFile.h" #include "cmSourceFileLocation.h" -#include "cmSystemTools.h" #include "cmTarget.h" #if defined(CMake_HAVE_CXX_UNORDERED_SET) @@ -54,194 +53,6 @@ bool cmTargetPropertyComputer::HandleLocationPropertyPolicy( return messageType != cmake::FATAL_ERROR; } -const char* cmTargetPropertyComputer::ComputeLocationForBuild( - cmTarget const* tgt) -{ - static std::string loc; - if (tgt->IsImported()) { - loc = tgt->ImportedGetFullPath("", false); - return loc.c_str(); - } - - cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); - if (!gg->GetConfigureDoneCMP0026()) { - gg->CreateGenerationObjects(); - } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); - loc = gt->GetLocationForBuild(); - return loc.c_str(); -} - -const char* cmTargetPropertyComputer::ComputeLocation( - cmTarget const* tgt, std::string const& config) -{ - static std::string loc; - if (tgt->IsImported()) { - loc = tgt->ImportedGetFullPath(config, false); - return loc.c_str(); - } - - cmGlobalGenerator* gg = tgt->GetMakefile()->GetGlobalGenerator(); - if (!gg->GetConfigureDoneCMP0026()) { - gg->CreateGenerationObjects(); - } - cmGeneratorTarget* gt = gg->FindGeneratorTarget(tgt->GetName()); - loc = gt->GetFullPath(config, false); - return loc.c_str(); -} - -const char* cmTargetPropertyComputer::GetProperty( - cmTarget const* tgt, const std::string& prop, cmMessenger* messenger, - cmListFileBacktrace const& context) -{ - if (const char* loc = GetLocation(tgt, prop, messenger, context)) { - return loc; - } - if (cmSystemTools::GetFatalErrorOccured()) { - return CM_NULLPTR; - } - if (prop == "SOURCES") { - return GetSources(tgt, messenger, context); - } - return CM_NULLPTR; -} - -const char* cmTargetPropertyComputer::GetLocation( - cmTarget const* tgt, const std::string& prop, cmMessenger* messenger, - cmListFileBacktrace const& context) -{ - // Watch for special "computed" properties that are dependent on - // other properties or variables. Always recompute them. - if (tgt->GetType() == cmState::EXECUTABLE || - tgt->GetType() == cmState::STATIC_LIBRARY || - tgt->GetType() == cmState::SHARED_LIBRARY || - tgt->GetType() == cmState::MODULE_LIBRARY || - tgt->GetType() == cmState::UNKNOWN_LIBRARY) { - static const std::string propLOCATION = "LOCATION"; - if (prop == propLOCATION) { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, context)) { - return CM_NULLPTR; - } - return ComputeLocationForBuild(tgt); - } - - // Support "LOCATION_". - else if (cmHasLiteralPrefix(prop, "LOCATION_")) { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, context)) { - return CM_NULLPTR; - } - const char* configName = prop.c_str() + 9; - return ComputeLocation(tgt, configName); - } - - // Support "_LOCATION". - else if (cmHasLiteralSuffix(prop, "_LOCATION") && - !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { - std::string configName(prop.c_str(), prop.size() - 9); - if (configName != "IMPORTED") { - if (!tgt->IsImported() && - !HandleLocationPropertyPolicy(tgt->GetName(), messenger, - context)) { - return CM_NULLPTR; - } - return ComputeLocation(tgt, configName); - } - } - } - return CM_NULLPTR; -} - -const char* cmTargetPropertyComputer::GetSources( - cmTarget const* tgt, cmMessenger* messenger, - cmListFileBacktrace const& context) -{ - cmStringRange entries = tgt->GetSourceEntries(); - if (entries.empty()) { - return CM_NULLPTR; - } - - std::ostringstream ss; - const char* sep = ""; - for (std::vector::const_iterator i = entries.begin(); - i != entries.end(); ++i) { - std::string const& entry = *i; - - std::vector files; - cmSystemTools::ExpandListArgument(entry, files); - for (std::vector::const_iterator li = files.begin(); - li != files.end(); ++li) { - if (cmHasLiteralPrefix(*li, "$size() - 1] == '>') { - std::string objLibName = li->substr(17, li->size() - 18); - - if (cmGeneratorExpression::Find(objLibName) != std::string::npos) { - ss << sep; - sep = ";"; - ss << *li; - continue; - } - - bool addContent = false; - bool noMessage = true; - std::ostringstream e; - cmake::MessageType messageType = cmake::AUTHOR_WARNING; - switch (context.GetBottom().GetPolicy(cmPolicies::CMP0051)) { - case cmPolicies::WARN: - e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0051) << "\n"; - noMessage = false; - case cmPolicies::OLD: - break; - case cmPolicies::REQUIRED_ALWAYS: - case cmPolicies::REQUIRED_IF_USED: - case cmPolicies::NEW: - addContent = true; - } - if (!noMessage) { - e << "Target \"" << tgt->GetName() - << "\" contains " - "$ generator expression in its sources " - "list. " - "This content was not previously part of the SOURCES " - "property " - "when that property was read at configure time. Code " - "reading " - "that property needs to be adapted to ignore the generator " - "expression using the string(GENEX_STRIP) command."; - messenger->IssueMessage(messageType, e.str(), context); - } - if (addContent) { - ss << sep; - sep = ";"; - ss << *li; - } - } else if (cmGeneratorExpression::Find(*li) == std::string::npos) { - ss << sep; - sep = ";"; - ss << *li; - } else { - cmSourceFile* sf = tgt->GetMakefile()->GetOrCreateSource(*li); - // Construct what is known about this source file location. - cmSourceFileLocation const& location = sf->GetLocation(); - std::string sname = location.GetDirectory(); - if (!sname.empty()) { - sname += "/"; - } - sname += location.GetName(); - - ss << sep; - sep = ";"; - // Append this list entry. - ss << sname; - } - } - } - static std::string srcs; - srcs = ss.str(); - return srcs.c_str(); -} - bool cmTargetPropertyComputer::WhiteListedInterfaceProperty( const std::string& prop) { diff --git a/Source/cmTargetPropertyComputer.h b/Source/cmTargetPropertyComputer.h index 941614bdcd..ed9a4e272f 100644 --- a/Source/cmTargetPropertyComputer.h +++ b/Source/cmTargetPropertyComputer.h @@ -6,6 +6,7 @@ #include // IWYU pragma: keep #include "cmListFileCache.h" +#include "cmSystemTools.h" #include #include @@ -16,12 +17,22 @@ class cmMessenger; class cmTargetPropertyComputer { public: - static const char* GetProperty(cmTarget const* tgt, const std::string& prop, + template + static const char* GetProperty(Target const* tgt, const std::string& prop, cmMessenger* messenger, - cmListFileBacktrace const& context); - - static std::map ComputeFileLocations( - cmTarget const* tgt); + cmListFileBacktrace const& context) + { + if (const char* loc = GetLocation(tgt, prop, messenger, context)) { + return loc; + } + if (cmSystemTools::GetFatalErrorOccured()) { + return CM_NULLPTR; + } + if (prop == "SOURCES") { + return GetSources(tgt, messenger, context); + } + return CM_NULLPTR; + } static bool WhiteListedInterfaceProperty(const std::string& prop); @@ -34,14 +45,65 @@ private: cmMessenger* messenger, cmListFileBacktrace const& context); - static const char* ComputeLocationForBuild(cmTarget const* tgt); - static const char* ComputeLocation(cmTarget const* tgt, + template + static const char* ComputeLocationForBuild(Target const* tgt); + template + static const char* ComputeLocation(Target const* tgt, std::string const& config); - static const char* GetLocation(cmTarget const* tgt, std::string const& prop, - cmMessenger* messenger, - cmListFileBacktrace const& context); - static const char* GetSources(cmTarget const* tgt, cmMessenger* messenger, + template + static const char* GetLocation(Target const* tgt, std::string const& prop, + cmMessenger* messenger, + cmListFileBacktrace const& context) + + { + // Watch for special "computed" properties that are dependent on + // other properties or variables. Always recompute them. + if (tgt->GetType() == cmState::EXECUTABLE || + tgt->GetType() == cmState::STATIC_LIBRARY || + tgt->GetType() == cmState::SHARED_LIBRARY || + tgt->GetType() == cmState::MODULE_LIBRARY || + tgt->GetType() == cmState::UNKNOWN_LIBRARY) { + static const std::string propLOCATION = "LOCATION"; + if (prop == propLOCATION) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, + context)) { + return CM_NULLPTR; + } + return ComputeLocationForBuild(tgt); + } + + // Support "LOCATION_". + else if (cmHasLiteralPrefix(prop, "LOCATION_")) { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, + context)) { + return CM_NULLPTR; + } + const char* configName = prop.c_str() + 9; + return ComputeLocation(tgt, configName); + } + + // Support "_LOCATION". + else if (cmHasLiteralSuffix(prop, "_LOCATION") && + !cmHasLiteralPrefix(prop, "XCODE_ATTRIBUTE_")) { + std::string configName(prop.c_str(), prop.size() - 9); + if (configName != "IMPORTED") { + if (!tgt->IsImported() && + !HandleLocationPropertyPolicy(tgt->GetName(), messenger, + context)) { + return CM_NULLPTR; + } + return ComputeLocation(tgt, configName); + } + } + } + return CM_NULLPTR; + } + + template + static const char* GetSources(Target const* tgt, cmMessenger* messenger, cmListFileBacktrace const& context); }; From fa9dbc56a15aec71ac2eda7890efd0116797f373 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:26 +0200 Subject: [PATCH 16/19] cmGeneratorTarget: Implement cmTargetPropertyComputer interface Populate a local member for the sources property when the instance is created. Pass the parameter to avoid the policy check when doing so. Ordinarily, the GetSources function should not be called unconditionally (at generate time), but we need to do so here in case the property is read in a generator expression. The intent is to be able to implement cmGeneratorTarget without requiring cmTarget. --- Source/cmGeneratorTarget.cxx | 38 ++++++++++++++++++++++++++++++++++++ Source/cmGeneratorTarget.h | 2 ++ 2 files changed, 40 insertions(+) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index fb732a6032..050fcb8de0 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -18,6 +18,7 @@ #include "cmSystemTools.h" #include "cmTarget.h" #include "cmTargetLinkLibraryType.h" +#include "cmTargetPropertyComputer.h" #include "cm_auto_ptr.hxx" #include "cmake.h" @@ -42,6 +43,28 @@ #define UNORDERED_SET std::set #endif +template <> +const char* cmTargetPropertyComputer::GetSources( + cmGeneratorTarget const* tgt, cmMessenger* /* messenger */, + cmListFileBacktrace const& /* context */) +{ + return tgt->GetSourcesProperty(); +} + +template <> +const char* cmTargetPropertyComputer::ComputeLocationForBuild< + cmGeneratorTarget>(cmGeneratorTarget const* tgt) +{ + return tgt->GetLocation(""); +} + +template <> +const char* cmTargetPropertyComputer::ComputeLocation( + cmGeneratorTarget const* tgt, const std::string& config) +{ + return tgt->GetLocation(config); +} + class cmGeneratorTarget::TargetPropertyEntry { static cmLinkImplItem NoLinkImplItem; @@ -320,6 +343,21 @@ cmGeneratorTarget::~cmGeneratorTarget() cmDeleteAll(this->LinkInformation); } +const char* cmGeneratorTarget::GetSourcesProperty() const +{ + std::vector values; + for (std::vector::const_iterator + it = this->SourceEntries.begin(), + end = this->SourceEntries.end(); + it != end; ++it) { + values.push_back((*it)->ge->GetInput()); + } + static std::string value; + value.clear(); + value = cmJoin(values, ""); + return value.c_str(); +} + cmGlobalGenerator* cmGeneratorTarget::GetGlobalGenerator() const { return this->GetLocalGenerator()->GetGlobalGenerator(); diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index bfbd790243..15e46b811d 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -532,6 +532,8 @@ public: std::string GetFortranModuleDirectory(std::string const& working_dir) const; + const char* GetSourcesProperty() const; + private: void AddSourceCommon(const std::string& src); From c3fb0d95ad114c9f9680e885c4c2263b43c437dc Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:26 +0200 Subject: [PATCH 17/19] cmTarget: Move sanity checks and computed property access to callers The GetProperty method is now just accessing contained data, meaning it can be implemented in cmState. Remove the cmMakefile context from the signature as a result and remove the overload with the same signature. Add a GetComputedProperty to cmTarget so that templates can be properly instantiated. Otherwise the Commands would need to be able to reach the specializations which are currently in cmTarget.cxx. As a side-effect, the CMP0026 warning now gives a backtrace to the target when issued from a generator expression. --- Source/cmGeneratorTarget.cxx | 12 ++++++++++ Source/cmGetPropertyCommand.cxx | 15 ++++++++++-- Source/cmGetTargetPropertyCommand.cxx | 13 ++++++++++- Source/cmTarget.cxx | 23 ++++--------------- Source/cmTarget.h | 4 +++- .../TARGET_PROPERTY-LOCATION-stderr.txt | 4 +++- 6 files changed, 48 insertions(+), 23 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 050fcb8de0..b0ff13ef91 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -397,6 +397,18 @@ std::string cmGeneratorTarget::GetExportName() const const char* cmGeneratorTarget::GetProperty(const std::string& prop) const { + if (!cmTargetPropertyComputer::PassesWhitelist( + this->GetType(), prop, this->Makefile->GetMessenger(), + this->GetBacktrace())) { + return 0; + } + if (const char* result = cmTargetPropertyComputer::GetProperty( + this, prop, this->Makefile->GetMessenger(), this->GetBacktrace())) { + return result; + } + if (cmSystemTools::GetFatalErrorOccured()) { + return CM_NULLPTR; + } return this->Target->GetProperty(prop); } diff --git a/Source/cmGetPropertyCommand.cxx b/Source/cmGetPropertyCommand.cxx index ba03568437..39445dd818 100644 --- a/Source/cmGetPropertyCommand.cxx +++ b/Source/cmGetPropertyCommand.cxx @@ -6,6 +6,7 @@ #include "cmPropertyDefinition.h" #include "cmSourceFile.h" #include "cmState.h" +#include "cmTargetPropertyComputer.h" #include "cmTest.h" #include "cmake.h" @@ -246,8 +247,18 @@ bool cmGetPropertyCommand::HandleTargetMode() } return this->StoreResult(CM_NULLPTR); } - return this->StoreResult( - target->GetProperty(this->PropertyName, this->Makefile)); + const char* prop_cstr = 0; + cmListFileBacktrace bt = this->Makefile->GetBacktrace(); + cmMessenger* messenger = this->Makefile->GetMessenger(); + if (cmTargetPropertyComputer::PassesWhitelist( + target->GetType(), this->PropertyName, messenger, bt)) { + prop_cstr = + target->GetComputedProperty(this->PropertyName, messenger, bt); + if (!prop_cstr) { + prop_cstr = target->GetProperty(this->PropertyName); + } + } + return this->StoreResult(prop_cstr); } std::ostringstream e; e << "could not find TARGET " << this->Name diff --git a/Source/cmGetTargetPropertyCommand.cxx b/Source/cmGetTargetPropertyCommand.cxx index fe09442acf..6a816d80e7 100644 --- a/Source/cmGetTargetPropertyCommand.cxx +++ b/Source/cmGetTargetPropertyCommand.cxx @@ -2,6 +2,8 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmGetTargetPropertyCommand.h" +#include "cmTargetPropertyComputer.h" + // cmSetTargetPropertyCommand bool cmGetTargetPropertyCommand::InitialPass( std::vector const& args, cmExecutionStatus&) @@ -22,7 +24,16 @@ bool cmGetTargetPropertyCommand::InitialPass( prop_exists = true; } } else if (!args[2].empty()) { - const char* prop_cstr = tgt->GetProperty(args[2], this->Makefile); + const char* prop_cstr = 0; + cmListFileBacktrace bt = this->Makefile->GetBacktrace(); + cmMessenger* messenger = this->Makefile->GetMessenger(); + if (cmTargetPropertyComputer::PassesWhitelist(tgt->GetType(), args[2], + messenger, bt)) { + prop_cstr = tgt->GetComputedProperty(args[2], messenger, bt); + if (!prop_cstr) { + prop_cstr = tgt->GetProperty(args[2]); + } + } if (prop_cstr) { prop = prop_cstr; prop_exists = true; diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 9b2043c391..e80768dd99 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1129,28 +1129,15 @@ void cmTarget::CheckProperty(const std::string& prop, } } -const char* cmTarget::GetProperty(const std::string& prop) const +const char* cmTarget::GetComputedProperty( + const std::string& prop, cmMessenger* messenger, + cmListFileBacktrace const& context) const { - return this->GetProperty(prop, this->Makefile); + return cmTargetPropertyComputer::GetProperty(this, prop, messenger, context); } -const char* cmTarget::GetProperty(const std::string& prop, - cmMakefile* context) const +const char* cmTarget::GetProperty(const std::string& prop) const { - if (!cmTargetPropertyComputer::PassesWhitelist(this->GetType(), prop, - context->GetMessenger(), - context->GetBacktrace())) { - return CM_NULLPTR; - } - - if (const char* result = cmTargetPropertyComputer::GetProperty( - this, prop, context->GetMessenger(), context->GetBacktrace())) { - return result; - } - if (cmSystemTools::GetFatalErrorOccured()) { - return CM_NULLPTR; - } - static UNORDERED_SET specialProps; #define MAKE_STATIC_PROP(PROP) static const std::string prop##PROP = #PROP MAKE_STATIC_PROP(LINK_LIBRARIES); diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 2d259ed322..bd00b3d85d 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -200,9 +200,11 @@ public: void AppendProperty(const std::string& prop, const char* value, bool asString = false); const char* GetProperty(const std::string& prop) const; - const char* GetProperty(const std::string& prop, cmMakefile* context) const; bool GetPropertyAsBool(const std::string& prop) const; void CheckProperty(const std::string& prop, cmMakefile* context) const; + const char* GetComputedProperty(const std::string& prop, + cmMessenger* messenger, + cmListFileBacktrace const& context) const; bool IsImported() const { return this->IsImportedTarget; } bool IsImportedGloballyVisible() const diff --git a/Tests/RunCMake/GeneratorExpression/TARGET_PROPERTY-LOCATION-stderr.txt b/Tests/RunCMake/GeneratorExpression/TARGET_PROPERTY-LOCATION-stderr.txt index e4dbb71da0..d4e5b29aed 100644 --- a/Tests/RunCMake/GeneratorExpression/TARGET_PROPERTY-LOCATION-stderr.txt +++ b/Tests/RunCMake/GeneratorExpression/TARGET_PROPERTY-LOCATION-stderr.txt @@ -1,4 +1,4 @@ -CMake Warning \(dev\) in CMakeLists.txt: +CMake Warning \(dev\) at TARGET_PROPERTY-LOCATION.cmake:2 \(add_library\): Policy CMP0026 is not set: Disallow use of the LOCATION target property. Run "cmake --help-policy CMP0026" for policy details. Use the cmake_policy command to set the policy and suppress this warning. @@ -7,4 +7,6 @@ CMake Warning \(dev\) in CMakeLists.txt: name directly with add_custom_command, or use the generator expression \$, as appropriate. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) This warning is for project developers. Use -Wno-dev to suppress it. From 0d57b07ad99cefc5df9e65d6c14aeaddf5d64b88 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:26 +0200 Subject: [PATCH 18/19] cmTarget: Group code for checking written properties together --- Source/cmTarget.cxx | 50 ++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index e80768dd99..f109444324 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -852,7 +852,20 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) e << "NAME property is read-only\n"; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; + } else if (prop == "EXPORT_NAME" && this->IsImported()) { + std::ostringstream e; + e << "EXPORT_NAME property can't be set on imported targets (\"" + << this->Name << "\")\n"; + this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); + return; + } else if (prop == "SOURCES" && this->IsImported()) { + std::ostringstream e; + e << "SOURCES property can't be set on imported targets (\"" << this->Name + << "\")\n"; + this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); + return; } + if (prop == "INCLUDE_DIRECTORIES") { this->Internal->IncludeDirectoriesEntries.clear(); this->Internal->IncludeDirectoriesBacktraces.clear(); @@ -885,11 +898,6 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); this->Internal->CompileDefinitionsBacktraces.push_back(lfbt); } - } else if (prop == "EXPORT_NAME" && this->IsImported()) { - std::ostringstream e; - e << "EXPORT_NAME property can't be set on imported targets (\"" - << this->Name << "\")\n"; - this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); } else if (prop == "LINK_LIBRARIES") { this->Internal->LinkImplementationPropertyEntries.clear(); this->Internal->LinkImplementationPropertyBacktraces.clear(); @@ -899,14 +907,6 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) this->Internal->LinkImplementationPropertyBacktraces.push_back(lfbt); } } else if (prop == "SOURCES") { - if (this->IsImported()) { - std::ostringstream e; - e << "SOURCES property can't be set on imported targets (\"" - << this->Name << "\")\n"; - this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); - return; - } - this->Internal->SourceEntries.clear(); this->Internal->SourceBacktraces.clear(); if (value) { @@ -932,6 +932,18 @@ void cmTarget::AppendProperty(const std::string& prop, const char* value, e << "NAME property is read-only\n"; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; + } else if (prop == "EXPORT_NAME" && this->IsImported()) { + std::ostringstream e; + e << "EXPORT_NAME property can't be set on imported targets (\"" + << this->Name << "\")\n"; + this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); + return; + } else if (prop == "SOURCES" && this->IsImported()) { + std::ostringstream e; + e << "SOURCES property can't be set on imported targets (\"" << this->Name + << "\")\n"; + this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); + return; } if (prop == "INCLUDE_DIRECTORIES") { if (value && *value) { @@ -957,11 +969,6 @@ void cmTarget::AppendProperty(const std::string& prop, const char* value, cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); this->Internal->CompileDefinitionsBacktraces.push_back(lfbt); } - } else if (prop == "EXPORT_NAME" && this->IsImported()) { - std::ostringstream e; - e << "EXPORT_NAME property can't be set on imported targets (\"" - << this->Name << "\")\n"; - this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); } else if (prop == "LINK_LIBRARIES") { if (value && *value) { cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); @@ -969,13 +976,6 @@ void cmTarget::AppendProperty(const std::string& prop, const char* value, this->Internal->LinkImplementationPropertyBacktraces.push_back(lfbt); } } else if (prop == "SOURCES") { - if (this->IsImported()) { - std::ostringstream e; - e << "SOURCES property can't be set on imported targets (\"" - << this->Name << "\")\n"; - this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); - return; - } cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); this->Internal->SourceEntries.push_back(value); this->Internal->SourceBacktraces.push_back(lfbt); From cef59bb8bac6b6d38025cc492235953c5ff7654b Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 13 Oct 2016 00:18:26 +0200 Subject: [PATCH 19/19] cmTarget: Implement GetProperty in terms of cmState::Snapshot --- Source/cmTarget.cxx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index f109444324..3a22309ee9 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1222,10 +1222,16 @@ const char* cmTarget::GetProperty(const std::string& prop) const return this->GetName().c_str(); } if (prop == propBINARY_DIR) { - return this->GetMakefile()->GetCurrentBinaryDirectory(); + return this->GetMakefile() + ->GetStateSnapshot() + .GetDirectory() + .GetCurrentBinary(); } if (prop == propSOURCE_DIR) { - return this->GetMakefile()->GetCurrentSourceDirectory(); + return this->GetMakefile() + ->GetStateSnapshot() + .GetDirectory() + .GetCurrentSource(); } } @@ -1234,7 +1240,8 @@ const char* cmTarget::GetProperty(const std::string& prop) const const bool chain = this->GetMakefile()->GetState()->IsPropertyChained( prop, cmProperty::TARGET); if (chain) { - return this->Makefile->GetProperty(prop, chain); + return this->Makefile->GetStateSnapshot().GetDirectory().GetProperty( + prop, chain); } } return retVal;