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

12 files changed, 82 insertions, 41 deletions

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<std::string, int>::iterator cmComputeLinkDepends::AllocateLinkEntry(
-  std::string const& item)
+std::map<cmLinkItem, int>::iterator cmComputeLinkDepends::AllocateLinkEntry(
+  cmLinkItem const& item)
 {
-  std::map<std::string, int>::value_type index_entry(
+  std::map<cmLinkItem, int>::value_type index_entry(
     item, static_cast<int>(this->EntryList.size()));
-  std::map<std::string, int>::iterator lei =
+  std::map<cmLinkItem, int>::iterator lei =
     this->LinkEntryIndex.insert(index_entry).first;
   this->EntryList.emplace_back();
   this->InferredDependSets.push_back(nullptr);
@@ -295,15 +295,14 @@ std::map<std::string, int>::iterator cmComputeLinkDepends::AllocateLinkEntry(
 int cmComputeLinkDepends::AddLinkEntry(cmLinkItem const& item)
 {
   // Check if the item entry has already been added.
-  std::map<std::string, int>::iterator lei =
-    this->LinkEntryIndex.find(item.AsStr());
+  std::map<cmLinkItem, int>::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<std::string, int>::iterator lei =
-    this->LinkEntryIndex.find(dep.Item.AsStr());
+  std::map<cmLinkItem, int>::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<std::string, int>::iterator AllocateLinkEntry(
-    std::string const& item);
+  std::map<cmLinkItem, int>::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<LinkEntry> EntryList;
-  std::map<std::string, int> LinkEntryIndex;
+  std::map<cmLinkItem, int> 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<std::string> emitted;
+    std::set<cmLinkItem> emitted;
 
     std::vector<std::string> configs;
     depender->Makefile->GetConfigurations(configs);
@@ -206,30 +206,34 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
       std::vector<cmSourceFile const*> 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<cmGeneratorTarget*>(depender)->Target->AddUtility(
+              objLib);
           }
-          const_cast<cmGeneratorTarget*>(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<cmLinkItem> const& tutils = depender->GetUtilityItems();
-    std::set<std::string> emitted;
+    std::set<cmLinkItem> 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<std::string>& emitted)
+  const std::string& config, std::set<cmLinkItem>& 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<std::string>& emitted)
+  const std::string& config, std::set<cmLinkItem>& 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<std::string>& emitted);
+                           std::set<cmLinkItem>& emitted);
   void AddInterfaceDepends(int depender_index,
                            cmGeneratorTarget const* dependee,
                            const std::string& config,
-                           std::set<std::string>& emitted);
+                           std::set<cmLinkItem>& 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<std::string> emitted;
+      std::set<cmLinkItem> 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;
+}