From f35be599612b788125d08a7c3e61d0fad3805bdd Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 6 Sep 2018 18:22:31 -0400 Subject: Fix transitive usage requirements through same-name imported targets If two imported targets in different directories have the same name we should still be able to propagate transitive usage requirements from both. Fix the DAG checker to work with target pointers instead of target names since the pointers will not be duplicated even if the names are. Fixes: #18345 --- Source/cmExportBuildAndroidMKGenerator.cxx | 2 +- Source/cmExportTryCompileFileGenerator.cxx | 3 +- Source/cmGeneratorExpression.cxx | 3 +- Source/cmGeneratorExpressionDAGChecker.cxx | 13 +++++---- Source/cmGeneratorExpressionDAGChecker.h | 13 +++++---- Source/cmGeneratorExpressionNode.cxx | 13 ++++----- Source/cmGeneratorTarget.cxx | 45 +++++++++++++++--------------- Tests/ImportedSameName/A/CMakeLists.txt | 1 + Tests/ImportedSameName/B/CMakeLists.txt | 1 + Tests/ImportedSameName/main.c | 7 +++++ 10 files changed, 54 insertions(+), 47 deletions(-) diff --git a/Source/cmExportBuildAndroidMKGenerator.cxx b/Source/cmExportBuildAndroidMKGenerator.cxx index bb370c4..3d32b84 100644 --- a/Source/cmExportBuildAndroidMKGenerator.cxx +++ b/Source/cmExportBuildAndroidMKGenerator.cxx @@ -108,7 +108,7 @@ void cmExportBuildAndroidMKGenerator::GenerateInterfaceProperties( // build type of the makefile cmGeneratorExpression ge; cmGeneratorExpressionDAGChecker dagChecker( - target->GetName(), "INTERFACE_LINK_LIBRARIES", nullptr, nullptr); + target, "INTERFACE_LINK_LIBRARIES", nullptr, nullptr); std::unique_ptr cge = ge.Parse(property.second); std::string evaluated = cge->Evaluate( diff --git a/Source/cmExportTryCompileFileGenerator.cxx b/Source/cmExportTryCompileFileGenerator.cxx index 87648cb..c169032 100644 --- a/Source/cmExportTryCompileFileGenerator.cxx +++ b/Source/cmExportTryCompileFileGenerator.cxx @@ -65,8 +65,7 @@ std::string cmExportTryCompileFileGenerator::FindTargets( cmGeneratorExpression ge; - cmGeneratorExpressionDAGChecker dagChecker(tgt->GetName(), propName, nullptr, - nullptr); + cmGeneratorExpressionDAGChecker dagChecker(tgt, propName, nullptr, nullptr); std::unique_ptr cge = ge.Parse(prop); diff --git a/Source/cmGeneratorExpression.cxx b/Source/cmGeneratorExpression.cxx index 6d83a3f..658e9a7 100644 --- a/Source/cmGeneratorExpression.cxx +++ b/Source/cmGeneratorExpression.cxx @@ -13,7 +13,6 @@ #include "cmGeneratorExpressionEvaluator.h" #include "cmGeneratorExpressionLexer.h" #include "cmGeneratorExpressionParser.h" -#include "cmGeneratorTarget.h" #include "cmSystemTools.h" cmGeneratorExpression::cmGeneratorExpression( @@ -395,7 +394,7 @@ const std::string& cmGeneratorExpressionInterpreter::Evaluate( // Specify COMPILE_OPTIONS to DAGchecker, same semantic as COMPILE_FLAGS cmGeneratorExpressionDAGChecker dagChecker( - this->HeadTarget->GetName(), + this->HeadTarget, property == "COMPILE_FLAGS" ? "COMPILE_OPTIONS" : property, nullptr, nullptr); diff --git a/Source/cmGeneratorExpressionDAGChecker.cxx b/Source/cmGeneratorExpressionDAGChecker.cxx index face282..8d57441 100644 --- a/Source/cmGeneratorExpressionDAGChecker.cxx +++ b/Source/cmGeneratorExpressionDAGChecker.cxx @@ -14,7 +14,7 @@ #include cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( - const cmListFileBacktrace& backtrace, const std::string& target, + const cmListFileBacktrace& backtrace, cmGeneratorTarget const* target, const std::string& property, const GeneratorExpressionContent* content, cmGeneratorExpressionDAGChecker* parent) : Parent(parent) @@ -28,7 +28,7 @@ cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( } cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( - const std::string& target, const std::string& property, + cmGeneratorTarget const* target, const std::string& property, const GeneratorExpressionContent* content, cmGeneratorExpressionDAGChecker* parent) : Parent(parent) @@ -58,8 +58,8 @@ void cmGeneratorExpressionDAGChecker::Initialize() TEST_TRANSITIVE_PROPERTY_METHOD) false)) // NOLINT(clang-tidy) #undef TEST_TRANSITIVE_PROPERTY_METHOD { - std::map>::const_iterator it = - top->Seen.find(this->Target); + std::map>::const_iterator + it = top->Seen.find(this->Target); if (it != top->Seen.end()) { const std::set& propSet = it->second; if (propSet.find(this->Property) != propSet.end()) { @@ -166,7 +166,8 @@ bool cmGeneratorExpressionDAGChecker::EvaluatingGenexExpression() return top->Property == "TARGET_GENEX_EVAL" || top->Property == "GENEX_EVAL"; } -bool cmGeneratorExpressionDAGChecker::EvaluatingLinkLibraries(const char* tgt) +bool cmGeneratorExpressionDAGChecker::EvaluatingLinkLibraries( + cmGeneratorTarget const* tgt) { const cmGeneratorExpressionDAGChecker* top = this; const cmGeneratorExpressionDAGChecker* parent = this->Parent; @@ -189,7 +190,7 @@ bool cmGeneratorExpressionDAGChecker::EvaluatingLinkLibraries(const char* tgt) strcmp(prop, "INTERFACE_LINK_LIBRARIES") == 0; } -std::string cmGeneratorExpressionDAGChecker::TopTarget() const +cmGeneratorTarget const* cmGeneratorExpressionDAGChecker::TopTarget() const { const cmGeneratorExpressionDAGChecker* top = this; const cmGeneratorExpressionDAGChecker* parent = this->Parent; diff --git a/Source/cmGeneratorExpressionDAGChecker.h b/Source/cmGeneratorExpressionDAGChecker.h index cd23904..8b1697b 100644 --- a/Source/cmGeneratorExpressionDAGChecker.h +++ b/Source/cmGeneratorExpressionDAGChecker.h @@ -13,6 +13,7 @@ struct GeneratorExpressionContent; struct cmGeneratorExpressionContext; +class cmGeneratorTarget; #define CM_SELECT_BOTH(F, A1, A2) F(A1, A2) #define CM_SELECT_FIRST(F, A1, A2) F(A1) @@ -41,11 +42,11 @@ struct cmGeneratorExpressionContext; struct cmGeneratorExpressionDAGChecker { cmGeneratorExpressionDAGChecker(const cmListFileBacktrace& backtrace, - const std::string& target, + cmGeneratorTarget const* target, const std::string& property, const GeneratorExpressionContent* content, cmGeneratorExpressionDAGChecker* parent); - cmGeneratorExpressionDAGChecker(const std::string& target, + cmGeneratorExpressionDAGChecker(cmGeneratorTarget const* target, const std::string& property, const GeneratorExpressionContent* content, cmGeneratorExpressionDAGChecker* parent); @@ -64,7 +65,7 @@ struct cmGeneratorExpressionDAGChecker const std::string& expr); bool EvaluatingGenexExpression(); - bool EvaluatingLinkLibraries(const char* tgt = nullptr); + bool EvaluatingLinkLibraries(cmGeneratorTarget const* tgt = nullptr); #define DECLARE_TRANSITIVE_PROPERTY_METHOD(METHOD) bool METHOD() const; @@ -75,7 +76,7 @@ struct cmGeneratorExpressionDAGChecker bool GetTransitivePropertiesOnly(); void SetTransitivePropertiesOnly() { this->TransitivePropertiesOnly = true; } - std::string TopTarget() const; + cmGeneratorTarget const* TopTarget() const; private: Result CheckGraph() const; @@ -83,9 +84,9 @@ private: private: const cmGeneratorExpressionDAGChecker* const Parent; - const std::string Target; + cmGeneratorTarget const* Target; const std::string Property; - std::map> Seen; + std::map> Seen; const GeneratorExpressionContent* const Content; const cmListFileBacktrace Backtrace; Result CheckResult; diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index 9aa5212..1e51f09 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -378,8 +378,8 @@ protected: { if (context->HeadTarget) { cmGeneratorExpressionDAGChecker dagChecker( - context->Backtrace, context->HeadTarget->GetName(), genexOperator, - content, dagCheckerParent); + context->Backtrace, context->HeadTarget, genexOperator, content, + dagCheckerParent); switch (dagChecker.Check()) { case cmGeneratorExpressionDAGChecker::SELF_REFERENCE: case cmGeneratorExpressionDAGChecker::CYCLIC_REFERENCE: { @@ -1196,9 +1196,8 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode return target->GetLinkerLanguage(context->Config); } - cmGeneratorExpressionDAGChecker dagChecker(context->Backtrace, - target->GetName(), propertyName, - content, dagCheckerParent); + cmGeneratorExpressionDAGChecker dagChecker( + context->Backtrace, target, propertyName, content, dagCheckerParent); switch (dagChecker.Check()) { case cmGeneratorExpressionDAGChecker::SELF_REFERENCE: @@ -1911,9 +1910,9 @@ struct TargetFilesystemArtifact : public cmGeneratorExpressionNode return std::string(); } if (dagChecker && - (dagChecker->EvaluatingLinkLibraries(name.c_str()) || + (dagChecker->EvaluatingLinkLibraries(target) || (dagChecker->EvaluatingSources() && - name == dagChecker->TopTarget()))) { + target == dagChecker->TopTarget()))) { ::reportError(context, content->GetOriginalExpression(), "Expressions which require the linker language may not " "be used while evaluating link libraries"); diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 080ff1c..e8e7b90 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -773,7 +773,7 @@ bool cmGeneratorTarget::IsSystemIncludeDirectory( if (iter == this->SystemIncludesCache.end()) { cmGeneratorExpressionDAGChecker dagChecker( - this->GetName(), "SYSTEM_INCLUDE_DIRECTORIES", nullptr, nullptr); + this, "SYSTEM_INCLUDE_DIRECTORIES", nullptr, nullptr); bool excludeImported = this->GetPropertyAsBool("NO_SYSTEM_FROM_IMPORTED"); @@ -964,8 +964,8 @@ void cmGeneratorTarget::GetSourceFiles(std::vector& files, this->DebugSourcesDone = true; } - cmGeneratorExpressionDAGChecker dagChecker(this->GetName(), "SOURCES", - nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "SOURCES", nullptr, + nullptr); std::unordered_set uniqueSrcs; bool contextDependentDirectSources = @@ -2078,8 +2078,8 @@ void cmGeneratorTarget::GetAutoUicOptions(std::vector& result, } cmGeneratorExpression ge; - cmGeneratorExpressionDAGChecker dagChecker( - this->GetName(), "AUTOUIC_OPTIONS", nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "AUTOUIC_OPTIONS", nullptr, + nullptr); cmSystemTools::ExpandListArgument( ge.Parse(prop)->Evaluate(this->LocalGenerator, config, false, this, &dagChecker), @@ -2588,8 +2588,8 @@ std::vector cmGeneratorTarget::GetIncludeDirectories( std::vector includes; std::unordered_set uniqueIncludes; - cmGeneratorExpressionDAGChecker dagChecker( - this->GetName(), "INCLUDE_DIRECTORIES", nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "INCLUDE_DIRECTORIES", + nullptr, nullptr); std::vector debugProperties; const char* debugProp = @@ -2710,8 +2710,8 @@ void cmGeneratorTarget::GetCompileOptions(std::vector& result, { std::unordered_set uniqueOptions; - cmGeneratorExpressionDAGChecker dagChecker( - this->GetName(), "COMPILE_OPTIONS", nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "COMPILE_OPTIONS", nullptr, + nullptr); std::vector debugProperties; const char* debugProp = @@ -2763,8 +2763,8 @@ void cmGeneratorTarget::GetCompileFeatures(std::vector& result, { std::unordered_set uniqueFeatures; - cmGeneratorExpressionDAGChecker dagChecker( - this->GetName(), "COMPILE_FEATURES", nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "COMPILE_FEATURES", nullptr, + nullptr); std::vector debugProperties; const char* debugProp = @@ -2814,8 +2814,8 @@ void cmGeneratorTarget::GetCompileDefinitions( { std::unordered_set uniqueOptions; - cmGeneratorExpressionDAGChecker dagChecker( - this->GetName(), "COMPILE_DEFINITIONS", nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "COMPILE_DEFINITIONS", + nullptr, nullptr); std::vector debugProperties; const char* debugProp = @@ -2895,8 +2895,8 @@ void cmGeneratorTarget::GetLinkOptions(std::vector& result, { std::unordered_set uniqueOptions; - cmGeneratorExpressionDAGChecker dagChecker(this->GetName(), "LINK_OPTIONS", - nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "LINK_OPTIONS", nullptr, + nullptr); std::vector debugProperties; const char* debugProp = @@ -3043,8 +3043,8 @@ void cmGeneratorTarget::GetStaticLibraryLinkOptions( std::vector entries; std::unordered_set uniqueOptions; - cmGeneratorExpressionDAGChecker dagChecker( - this->GetName(), "STATIC_LIBRARY_OPTIONS", nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "STATIC_LIBRARY_OPTIONS", + nullptr, nullptr); if (const char* linkOptions = this->GetProperty("STATIC_LIBRARY_OPTIONS")) { std::vector options; @@ -3083,8 +3083,8 @@ void cmGeneratorTarget::GetLinkDepends(std::vector& result, { std::vector linkDependsEntries; std::unordered_set uniqueOptions; - cmGeneratorExpressionDAGChecker dagChecker(this->GetName(), "LINK_DEPENDS", - nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "LINK_DEPENDS", nullptr, + nullptr); if (const char* linkDepends = this->GetProperty("LINK_DEPENDS")) { std::vector depends; @@ -4509,8 +4509,7 @@ void cmGeneratorTarget::ExpandLinkItems( std::vector& items, bool& hadHeadSensitiveCondition) const { cmGeneratorExpression ge; - cmGeneratorExpressionDAGChecker dagChecker(this->GetName(), prop, nullptr, - nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, prop, nullptr, nullptr); // The $ expression may be in a link interface to specify private // link dependencies that are otherwise excluded from usage requirements. if (usage_requirements_only) { @@ -5561,8 +5560,8 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries( end = entryRange.end(); le != end; ++le, ++btIt) { std::vector llibs; - cmGeneratorExpressionDAGChecker dagChecker( - this->GetName(), "LINK_LIBRARIES", nullptr, nullptr); + cmGeneratorExpressionDAGChecker dagChecker(this, "LINK_LIBRARIES", nullptr, + nullptr); cmGeneratorExpression ge(*btIt); std::unique_ptr const cge = ge.Parse(*le); std::string const& evaluated = diff --git a/Tests/ImportedSameName/A/CMakeLists.txt b/Tests/ImportedSameName/A/CMakeLists.txt index 9417a2c..0a31b40 100644 --- a/Tests/ImportedSameName/A/CMakeLists.txt +++ b/Tests/ImportedSameName/A/CMakeLists.txt @@ -1,4 +1,5 @@ add_library(a STATIC a.c) +target_compile_definitions(a INTERFACE DEF_A) add_library(sameName INTERFACE IMPORTED) target_link_libraries(sameName INTERFACE a) diff --git a/Tests/ImportedSameName/B/CMakeLists.txt b/Tests/ImportedSameName/B/CMakeLists.txt index 6947fa9..d930326 100644 --- a/Tests/ImportedSameName/B/CMakeLists.txt +++ b/Tests/ImportedSameName/B/CMakeLists.txt @@ -1,4 +1,5 @@ add_library(b STATIC b.c) +target_compile_definitions(b INTERFACE DEF_B) add_library(sameName INTERFACE IMPORTED) target_link_libraries(sameName INTERFACE b) diff --git a/Tests/ImportedSameName/main.c b/Tests/ImportedSameName/main.c index 33196b7..a0cb27f 100644 --- a/Tests/ImportedSameName/main.c +++ b/Tests/ImportedSameName/main.c @@ -1,3 +1,10 @@ +#ifndef DEF_A +# error "DEF_A not defined" +#endif +#ifndef DEF_B +# error "DEF_B not defined" +#endif + extern void a(void); extern void b(void); -- cgit v0.12