From bea390e9bd68e1aa7d86b6aef7384f502c39068c Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 6 Sep 2018 14:21:06 -0400 Subject: Fix dependency propagation through same-name imported targets If two imported targets in different directories have the same name we should still be able to propagate transitive link dependencies from both. Fix the target and link dependency analyzers to de-duplicate targets using target pointers rather than target names since the pointers will not be duplicated even if the names are. Issue: #18345 --- Source/cmComputeLinkDepends.cxx | 19 ++++++------- Source/cmComputeLinkDepends.h | 6 ++-- Source/cmComputeTargetDepends.cxx | 50 ++++++++++++++++++--------------- Source/cmComputeTargetDepends.h | 4 +-- Source/cmGeneratorTarget.cxx | 6 ++-- Tests/CMakeLists.txt | 1 + Tests/ImportedSameName/A/CMakeLists.txt | 7 +++++ Tests/ImportedSameName/A/a.c | 3 ++ Tests/ImportedSameName/B/CMakeLists.txt | 7 +++++ Tests/ImportedSameName/B/b.c | 3 ++ Tests/ImportedSameName/CMakeLists.txt | 8 ++++++ Tests/ImportedSameName/main.c | 9 ++++++ 12 files changed, 82 insertions(+), 41 deletions(-) create mode 100644 Tests/ImportedSameName/A/CMakeLists.txt create mode 100644 Tests/ImportedSameName/A/a.c create mode 100644 Tests/ImportedSameName/B/CMakeLists.txt create mode 100644 Tests/ImportedSameName/B/b.c create mode 100644 Tests/ImportedSameName/CMakeLists.txt create mode 100644 Tests/ImportedSameName/main.c diff --git a/Source/cmComputeLinkDepends.cxx b/Source/cmComputeLinkDepends.cxx index 3659a1e..aa17de6 100644 --- a/Source/cmComputeLinkDepends.cxx +++ b/Source/cmComputeLinkDepends.cxx @@ -279,12 +279,12 @@ cmComputeLinkDepends::Compute() return this->FinalLinkEntries; } -std::map::iterator cmComputeLinkDepends::AllocateLinkEntry( - std::string const& item) +std::map::iterator cmComputeLinkDepends::AllocateLinkEntry( + cmLinkItem const& item) { - std::map::value_type index_entry( + std::map::value_type index_entry( item, static_cast(this->EntryList.size())); - std::map::iterator lei = + std::map::iterator lei = this->LinkEntryIndex.insert(index_entry).first; this->EntryList.emplace_back(); this->InferredDependSets.push_back(nullptr); @@ -295,15 +295,14 @@ std::map::iterator cmComputeLinkDepends::AllocateLinkEntry( int cmComputeLinkDepends::AddLinkEntry(cmLinkItem const& item) { // Check if the item entry has already been added. - std::map::iterator lei = - this->LinkEntryIndex.find(item.AsStr()); + std::map::iterator lei = this->LinkEntryIndex.find(item); if (lei != this->LinkEntryIndex.end()) { // Yes. We do not need to follow the item's dependencies again. return lei->second; } // Allocate a spot for the item entry. - lei = this->AllocateLinkEntry(item.AsStr()); + lei = this->AllocateLinkEntry(item); // Initialize the item entry. int index = lei->second; @@ -397,11 +396,11 @@ void cmComputeLinkDepends::QueueSharedDependencies( void cmComputeLinkDepends::HandleSharedDependency(SharedDepEntry const& dep) { // Check if the target already has an entry. - std::map::iterator lei = - this->LinkEntryIndex.find(dep.Item.AsStr()); + std::map::iterator lei = + this->LinkEntryIndex.find(dep.Item); if (lei == this->LinkEntryIndex.end()) { // Allocate a spot for the item entry. - lei = this->AllocateLinkEntry(dep.Item.AsStr()); + lei = this->AllocateLinkEntry(dep.Item); // Initialize the item entry. LinkEntry& entry = this->EntryList[lei->second]; diff --git a/Source/cmComputeLinkDepends.h b/Source/cmComputeLinkDepends.h index fb3a02f..66fb1e6 100644 --- a/Source/cmComputeLinkDepends.h +++ b/Source/cmComputeLinkDepends.h @@ -72,8 +72,8 @@ private: std::string Config; EntryVector FinalLinkEntries; - std::map::iterator AllocateLinkEntry( - std::string const& item); + std::map::iterator AllocateLinkEntry( + cmLinkItem const& item); int AddLinkEntry(cmLinkItem const& item); void AddVarLinkEntries(int depender_index, const char* value); void AddDirectLinkEntries(); @@ -83,7 +83,7 @@ private: // One entry for each unique item. std::vector EntryList; - std::map LinkEntryIndex; + std::map LinkEntryIndex; // BFS of initial dependencies. struct BFSEntry diff --git a/Source/cmComputeTargetDepends.cxx b/Source/cmComputeTargetDepends.cxx index edda2bd..268e749 100644 --- a/Source/cmComputeTargetDepends.cxx +++ b/Source/cmComputeTargetDepends.cxx @@ -195,7 +195,7 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) // dependencies in all targets, because the generated build-systems can't // deal with config-specific dependencies. { - std::set emitted; + std::set emitted; std::vector configs; depender->Makefile->GetConfigurations(configs); @@ -206,30 +206,34 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) std::vector objectFiles; depender->GetExternalObjects(objectFiles, it); for (cmSourceFile const* o : objectFiles) { - std::string objLib = o->GetObjectLibrary(); - if (!objLib.empty() && emitted.insert(objLib).second) { - if (depender->GetType() != cmStateEnums::EXECUTABLE && - depender->GetType() != cmStateEnums::STATIC_LIBRARY && - depender->GetType() != cmStateEnums::SHARED_LIBRARY && - depender->GetType() != cmStateEnums::MODULE_LIBRARY && - depender->GetType() != cmStateEnums::OBJECT_LIBRARY) { - this->GlobalGenerator->GetCMakeInstance()->IssueMessage( - cmake::FATAL_ERROR, - "Only executables and libraries may reference target objects.", - depender->GetBacktrace()); - return; + std::string const& objLib = o->GetObjectLibrary(); + if (!objLib.empty()) { + cmLinkItem const& objItem = depender->ResolveLinkItem(objLib); + if (emitted.insert(objItem).second) { + if (depender->GetType() != cmStateEnums::EXECUTABLE && + depender->GetType() != cmStateEnums::STATIC_LIBRARY && + depender->GetType() != cmStateEnums::SHARED_LIBRARY && + depender->GetType() != cmStateEnums::MODULE_LIBRARY && + depender->GetType() != cmStateEnums::OBJECT_LIBRARY) { + this->GlobalGenerator->GetCMakeInstance()->IssueMessage( + cmake::FATAL_ERROR, + "Only executables and libraries may reference target objects.", + depender->GetBacktrace()); + return; + } + const_cast(depender)->Target->AddUtility( + objLib); } - const_cast(depender)->Target->AddUtility(objLib); } } cmLinkImplementation const* impl = depender->GetLinkImplementation(it); // A target should not depend on itself. - emitted.insert(depender->GetName()); + emitted.insert(cmLinkItem(depender)); for (cmLinkImplItem const& lib : impl->Libraries) { // Don't emit the same library twice for this target. - if (emitted.insert(lib.AsStr()).second) { + if (emitted.insert(lib).second) { this->AddTargetDepend(depender_index, lib, true); this->AddInterfaceDepends(depender_index, lib, it, emitted); } @@ -240,12 +244,12 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) // Loop over all utility dependencies. { std::set const& tutils = depender->GetUtilityItems(); - std::set emitted; + std::set emitted; // A target should not depend on itself. - emitted.insert(depender->GetName()); + emitted.insert(cmLinkItem(depender)); for (cmLinkItem const& litem : tutils) { // Don't emit the same utility twice for this target. - if (emitted.insert(litem.AsStr()).second) { + if (emitted.insert(litem).second) { this->AddTargetDepend(depender_index, litem, false); } } @@ -254,14 +258,14 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) void cmComputeTargetDepends::AddInterfaceDepends( int depender_index, const cmGeneratorTarget* dependee, - const std::string& config, std::set& emitted) + const std::string& config, std::set& emitted) { cmGeneratorTarget const* depender = this->Targets[depender_index]; if (cmLinkInterface const* iface = dependee->GetLinkInterface(config, depender)) { for (cmLinkItem const& lib : iface->Libraries) { // Don't emit the same library twice for this target. - if (emitted.insert(lib.AsStr()).second) { + if (emitted.insert(lib).second) { this->AddTargetDepend(depender_index, lib, true); this->AddInterfaceDepends(depender_index, lib, config, emitted); } @@ -271,7 +275,7 @@ void cmComputeTargetDepends::AddInterfaceDepends( void cmComputeTargetDepends::AddInterfaceDepends( int depender_index, cmLinkItem const& dependee_name, - const std::string& config, std::set& emitted) + const std::string& config, std::set& emitted) { cmGeneratorTarget const* depender = this->Targets[depender_index]; cmGeneratorTarget const* dependee = dependee_name.Target; @@ -285,7 +289,7 @@ void cmComputeTargetDepends::AddInterfaceDepends( if (dependee) { // A target should not depend on itself. - emitted.insert(depender->GetName()); + emitted.insert(cmLinkItem(depender)); this->AddInterfaceDepends(depender_index, dependee, config, emitted); } } diff --git a/Source/cmComputeTargetDepends.h b/Source/cmComputeTargetDepends.h index e93e376..3046e8a 100644 --- a/Source/cmComputeTargetDepends.h +++ b/Source/cmComputeTargetDepends.h @@ -51,11 +51,11 @@ private: bool ComputeFinalDepends(cmComputeComponentGraph const& ccg); void AddInterfaceDepends(int depender_index, cmLinkItem const& dependee_name, const std::string& config, - std::set& emitted); + std::set& emitted); void AddInterfaceDepends(int depender_index, cmGeneratorTarget const* dependee, const std::string& config, - std::set& emitted); + std::set& emitted); cmGlobalGenerator* GlobalGenerator; bool DebugMode; bool NoCycles; diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 377c008..080ff1c 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -4575,14 +4575,14 @@ void cmGeneratorTarget::ComputeLinkInterface( this->GetType() == cmStateEnums::INTERFACE_LIBRARY) { // Shared libraries may have runtime implementation dependencies // on other shared libraries that are not in the interface. - std::unordered_set emitted; + std::set emitted; for (cmLinkItem const& lib : iface.Libraries) { - emitted.insert(lib.AsStr()); + emitted.insert(lib); } if (this->GetType() != cmStateEnums::INTERFACE_LIBRARY) { cmLinkImplementation const* impl = this->GetLinkImplementation(config); for (cmLinkImplItem const& lib : impl->Libraries) { - if (emitted.insert(lib.AsStr()).second) { + if (emitted.insert(lib).second) { if (lib.Target) { // This is a runtime dependency on another shared library. if (lib.Target->GetType() == cmStateEnums::SHARED_LIBRARY) { diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 971d7ff..a22521b 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -396,6 +396,7 @@ if(BUILD_TESTING) ADD_TEST_MACRO(CompatibleInterface CompatibleInterface) ADD_TEST_MACRO(AliasTarget AliasTarget) ADD_TEST_MACRO(StagingPrefix StagingPrefix) + ADD_TEST_MACRO(ImportedSameName ImportedSameName) ADD_TEST_MACRO(InterfaceLibrary InterfaceLibrary) if (CMAKE_BUILD_TYPE MATCHES "[Dd][Ee][Bb][Uu][Gg]") set(ConfigSources_BUILD_OPTIONS -DCMAKE_BUILD_TYPE=Debug) diff --git a/Tests/ImportedSameName/A/CMakeLists.txt b/Tests/ImportedSameName/A/CMakeLists.txt new file mode 100644 index 0000000..9417a2c --- /dev/null +++ b/Tests/ImportedSameName/A/CMakeLists.txt @@ -0,0 +1,7 @@ +add_library(a STATIC a.c) + +add_library(sameName INTERFACE IMPORTED) +target_link_libraries(sameName INTERFACE a) + +add_library(ifaceA INTERFACE) +target_link_libraries(ifaceA INTERFACE sameName) diff --git a/Tests/ImportedSameName/A/a.c b/Tests/ImportedSameName/A/a.c new file mode 100644 index 0000000..4ef3698 --- /dev/null +++ b/Tests/ImportedSameName/A/a.c @@ -0,0 +1,3 @@ +void a(void) +{ +} diff --git a/Tests/ImportedSameName/B/CMakeLists.txt b/Tests/ImportedSameName/B/CMakeLists.txt new file mode 100644 index 0000000..6947fa9 --- /dev/null +++ b/Tests/ImportedSameName/B/CMakeLists.txt @@ -0,0 +1,7 @@ +add_library(b STATIC b.c) + +add_library(sameName INTERFACE IMPORTED) +target_link_libraries(sameName INTERFACE b) + +add_library(ifaceB INTERFACE) +target_link_libraries(ifaceB INTERFACE sameName) diff --git a/Tests/ImportedSameName/B/b.c b/Tests/ImportedSameName/B/b.c new file mode 100644 index 0000000..c7c7df4 --- /dev/null +++ b/Tests/ImportedSameName/B/b.c @@ -0,0 +1,3 @@ +void b(void) +{ +} diff --git a/Tests/ImportedSameName/CMakeLists.txt b/Tests/ImportedSameName/CMakeLists.txt new file mode 100644 index 0000000..4292c12 --- /dev/null +++ b/Tests/ImportedSameName/CMakeLists.txt @@ -0,0 +1,8 @@ +cmake_minimum_required(VERSION 3.12) +project(ImportedSameName C) + +add_subdirectory(A) +add_subdirectory(B) + +add_executable(ImportedSameName main.c) +target_link_libraries(ImportedSameName PRIVATE ifaceA ifaceB) diff --git a/Tests/ImportedSameName/main.c b/Tests/ImportedSameName/main.c new file mode 100644 index 0000000..33196b7 --- /dev/null +++ b/Tests/ImportedSameName/main.c @@ -0,0 +1,9 @@ +extern void a(void); +extern void b(void); + +int main(void) +{ + a(); + b(); + return 0; +} -- cgit v0.12