Commit bea390e9 authored by Brad King's avatar Brad King
Browse files

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
parent fc7e4d1e
...@@ -279,12 +279,12 @@ cmComputeLinkDepends::Compute() ...@@ -279,12 +279,12 @@ cmComputeLinkDepends::Compute()
return this->FinalLinkEntries; return this->FinalLinkEntries;
} }
std::map<std::string, int>::iterator cmComputeLinkDepends::AllocateLinkEntry( std::map<cmLinkItem, int>::iterator cmComputeLinkDepends::AllocateLinkEntry(
std::string const& item) 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())); 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->LinkEntryIndex.insert(index_entry).first;
this->EntryList.emplace_back(); this->EntryList.emplace_back();
this->InferredDependSets.push_back(nullptr); this->InferredDependSets.push_back(nullptr);
...@@ -295,15 +295,14 @@ std::map<std::string, int>::iterator cmComputeLinkDepends::AllocateLinkEntry( ...@@ -295,15 +295,14 @@ std::map<std::string, int>::iterator cmComputeLinkDepends::AllocateLinkEntry(
int cmComputeLinkDepends::AddLinkEntry(cmLinkItem const& item) int cmComputeLinkDepends::AddLinkEntry(cmLinkItem const& item)
{ {
// Check if the item entry has already been added. // Check if the item entry has already been added.
std::map<std::string, int>::iterator lei = std::map<cmLinkItem, int>::iterator lei = this->LinkEntryIndex.find(item);
this->LinkEntryIndex.find(item.AsStr());
if (lei != this->LinkEntryIndex.end()) { if (lei != this->LinkEntryIndex.end()) {
// Yes. We do not need to follow the item's dependencies again. // Yes. We do not need to follow the item's dependencies again.
return lei->second; return lei->second;
} }
// Allocate a spot for the item entry. // Allocate a spot for the item entry.
lei = this->AllocateLinkEntry(item.AsStr()); lei = this->AllocateLinkEntry(item);
// Initialize the item entry. // Initialize the item entry.
int index = lei->second; int index = lei->second;
...@@ -397,11 +396,11 @@ void cmComputeLinkDepends::QueueSharedDependencies( ...@@ -397,11 +396,11 @@ void cmComputeLinkDepends::QueueSharedDependencies(
void cmComputeLinkDepends::HandleSharedDependency(SharedDepEntry const& dep) void cmComputeLinkDepends::HandleSharedDependency(SharedDepEntry const& dep)
{ {
// Check if the target already has an entry. // Check if the target already has an entry.
std::map<std::string, int>::iterator lei = std::map<cmLinkItem, int>::iterator lei =
this->LinkEntryIndex.find(dep.Item.AsStr()); this->LinkEntryIndex.find(dep.Item);
if (lei == this->LinkEntryIndex.end()) { if (lei == this->LinkEntryIndex.end()) {
// Allocate a spot for the item entry. // Allocate a spot for the item entry.
lei = this->AllocateLinkEntry(dep.Item.AsStr()); lei = this->AllocateLinkEntry(dep.Item);
// Initialize the item entry. // Initialize the item entry.
LinkEntry& entry = this->EntryList[lei->second]; LinkEntry& entry = this->EntryList[lei->second];
......
...@@ -72,8 +72,8 @@ private: ...@@ -72,8 +72,8 @@ private:
std::string Config; std::string Config;
EntryVector FinalLinkEntries; EntryVector FinalLinkEntries;
std::map<std::string, int>::iterator AllocateLinkEntry( std::map<cmLinkItem, int>::iterator AllocateLinkEntry(
std::string const& item); cmLinkItem const& item);
int AddLinkEntry(cmLinkItem const& item); int AddLinkEntry(cmLinkItem const& item);
void AddVarLinkEntries(int depender_index, const char* value); void AddVarLinkEntries(int depender_index, const char* value);
void AddDirectLinkEntries(); void AddDirectLinkEntries();
...@@ -83,7 +83,7 @@ private: ...@@ -83,7 +83,7 @@ private:
// One entry for each unique item. // One entry for each unique item.
std::vector<LinkEntry> EntryList; std::vector<LinkEntry> EntryList;
std::map<std::string, int> LinkEntryIndex; std::map<cmLinkItem, int> LinkEntryIndex;
// BFS of initial dependencies. // BFS of initial dependencies.
struct BFSEntry struct BFSEntry
......
...@@ -195,7 +195,7 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) ...@@ -195,7 +195,7 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
// dependencies in all targets, because the generated build-systems can't // dependencies in all targets, because the generated build-systems can't
// deal with config-specific dependencies. // deal with config-specific dependencies.
{ {
std::set<std::string> emitted; std::set<cmLinkItem> emitted;
std::vector<std::string> configs; std::vector<std::string> configs;
depender->Makefile->GetConfigurations(configs); depender->Makefile->GetConfigurations(configs);
...@@ -206,30 +206,34 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) ...@@ -206,30 +206,34 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
std::vector<cmSourceFile const*> objectFiles; std::vector<cmSourceFile const*> objectFiles;
depender->GetExternalObjects(objectFiles, it); depender->GetExternalObjects(objectFiles, it);
for (cmSourceFile const* o : objectFiles) { for (cmSourceFile const* o : objectFiles) {
std::string objLib = o->GetObjectLibrary(); std::string const& objLib = o->GetObjectLibrary();
if (!objLib.empty() && emitted.insert(objLib).second) { if (!objLib.empty()) {
if (depender->GetType() != cmStateEnums::EXECUTABLE && cmLinkItem const& objItem = depender->ResolveLinkItem(objLib);
depender->GetType() != cmStateEnums::STATIC_LIBRARY && if (emitted.insert(objItem).second) {
depender->GetType() != cmStateEnums::SHARED_LIBRARY && if (depender->GetType() != cmStateEnums::EXECUTABLE &&
depender->GetType() != cmStateEnums::MODULE_LIBRARY && depender->GetType() != cmStateEnums::STATIC_LIBRARY &&
depender->GetType() != cmStateEnums::OBJECT_LIBRARY) { depender->GetType() != cmStateEnums::SHARED_LIBRARY &&
this->GlobalGenerator->GetCMakeInstance()->IssueMessage( depender->GetType() != cmStateEnums::MODULE_LIBRARY &&
cmake::FATAL_ERROR, depender->GetType() != cmStateEnums::OBJECT_LIBRARY) {
"Only executables and libraries may reference target objects.", this->GlobalGenerator->GetCMakeInstance()->IssueMessage(
depender->GetBacktrace()); cmake::FATAL_ERROR,
return; "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); cmLinkImplementation const* impl = depender->GetLinkImplementation(it);
// A target should not depend on itself. // A target should not depend on itself.
emitted.insert(depender->GetName()); emitted.insert(cmLinkItem(depender));
for (cmLinkImplItem const& lib : impl->Libraries) { for (cmLinkImplItem const& lib : impl->Libraries) {
// Don't emit the same library twice for this target. // 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->AddTargetDepend(depender_index, lib, true);
this->AddInterfaceDepends(depender_index, lib, it, emitted); this->AddInterfaceDepends(depender_index, lib, it, emitted);
} }
...@@ -240,12 +244,12 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) ...@@ -240,12 +244,12 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
// Loop over all utility dependencies. // Loop over all utility dependencies.
{ {
std::set<cmLinkItem> const& tutils = depender->GetUtilityItems(); std::set<cmLinkItem> const& tutils = depender->GetUtilityItems();
std::set<std::string> emitted; std::set<cmLinkItem> emitted;
// A target should not depend on itself. // A target should not depend on itself.
emitted.insert(depender->GetName()); emitted.insert(cmLinkItem(depender));
for (cmLinkItem const& litem : tutils) { for (cmLinkItem const& litem : tutils) {
// Don't emit the same utility twice for this target. // 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); this->AddTargetDepend(depender_index, litem, false);
} }
} }
...@@ -254,14 +258,14 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) ...@@ -254,14 +258,14 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
void cmComputeTargetDepends::AddInterfaceDepends( void cmComputeTargetDepends::AddInterfaceDepends(
int depender_index, const cmGeneratorTarget* dependee, 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]; cmGeneratorTarget const* depender = this->Targets[depender_index];
if (cmLinkInterface const* iface = if (cmLinkInterface const* iface =
dependee->GetLinkInterface(config, depender)) { dependee->GetLinkInterface(config, depender)) {
for (cmLinkItem const& lib : iface->Libraries) { for (cmLinkItem const& lib : iface->Libraries) {
// Don't emit the same library twice for this target. // 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->AddTargetDepend(depender_index, lib, true);
this->AddInterfaceDepends(depender_index, lib, config, emitted); this->AddInterfaceDepends(depender_index, lib, config, emitted);
} }
...@@ -271,7 +275,7 @@ void cmComputeTargetDepends::AddInterfaceDepends( ...@@ -271,7 +275,7 @@ void cmComputeTargetDepends::AddInterfaceDepends(
void cmComputeTargetDepends::AddInterfaceDepends( void cmComputeTargetDepends::AddInterfaceDepends(
int depender_index, cmLinkItem const& dependee_name, 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* depender = this->Targets[depender_index];
cmGeneratorTarget const* dependee = dependee_name.Target; cmGeneratorTarget const* dependee = dependee_name.Target;
...@@ -285,7 +289,7 @@ void cmComputeTargetDepends::AddInterfaceDepends( ...@@ -285,7 +289,7 @@ void cmComputeTargetDepends::AddInterfaceDepends(
if (dependee) { if (dependee) {
// A target should not depend on itself. // A target should not depend on itself.
emitted.insert(depender->GetName()); emitted.insert(cmLinkItem(depender));
this->AddInterfaceDepends(depender_index, dependee, config, emitted); this->AddInterfaceDepends(depender_index, dependee, config, emitted);
} }
} }
......
...@@ -51,11 +51,11 @@ private: ...@@ -51,11 +51,11 @@ private:
bool ComputeFinalDepends(cmComputeComponentGraph const& ccg); bool ComputeFinalDepends(cmComputeComponentGraph const& ccg);
void AddInterfaceDepends(int depender_index, cmLinkItem const& dependee_name, void AddInterfaceDepends(int depender_index, cmLinkItem const& dependee_name,
const std::string& config, const std::string& config,
std::set<std::string>& emitted); std::set<cmLinkItem>& emitted);
void AddInterfaceDepends(int depender_index, void AddInterfaceDepends(int depender_index,
cmGeneratorTarget const* dependee, cmGeneratorTarget const* dependee,
const std::string& config, const std::string& config,
std::set<std::string>& emitted); std::set<cmLinkItem>& emitted);
cmGlobalGenerator* GlobalGenerator; cmGlobalGenerator* GlobalGenerator;
bool DebugMode; bool DebugMode;
bool NoCycles; bool NoCycles;
......
...@@ -4575,14 +4575,14 @@ void cmGeneratorTarget::ComputeLinkInterface( ...@@ -4575,14 +4575,14 @@ void cmGeneratorTarget::ComputeLinkInterface(
this->GetType() == cmStateEnums::INTERFACE_LIBRARY) { this->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
// Shared libraries may have runtime implementation dependencies // Shared libraries may have runtime implementation dependencies
// on other shared libraries that are not in the interface. // 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) { for (cmLinkItem const& lib : iface.Libraries) {
emitted.insert(lib.AsStr()); emitted.insert(lib);
} }
if (this->GetType() != cmStateEnums::INTERFACE_LIBRARY) { if (this->GetType() != cmStateEnums::INTERFACE_LIBRARY) {
cmLinkImplementation const* impl = this->GetLinkImplementation(config); cmLinkImplementation const* impl = this->GetLinkImplementation(config);
for (cmLinkImplItem const& lib : impl->Libraries) { for (cmLinkImplItem const& lib : impl->Libraries) {
if (emitted.insert(lib.AsStr()).second) { if (emitted.insert(lib).second) {
if (lib.Target) { if (lib.Target) {
// This is a runtime dependency on another shared library. // This is a runtime dependency on another shared library.
if (lib.Target->GetType() == cmStateEnums::SHARED_LIBRARY) { if (lib.Target->GetType() == cmStateEnums::SHARED_LIBRARY) {
......
...@@ -396,6 +396,7 @@ if(BUILD_TESTING) ...@@ -396,6 +396,7 @@ if(BUILD_TESTING)
ADD_TEST_MACRO(CompatibleInterface CompatibleInterface) ADD_TEST_MACRO(CompatibleInterface CompatibleInterface)
ADD_TEST_MACRO(AliasTarget AliasTarget) ADD_TEST_MACRO(AliasTarget AliasTarget)
ADD_TEST_MACRO(StagingPrefix StagingPrefix) ADD_TEST_MACRO(StagingPrefix StagingPrefix)
ADD_TEST_MACRO(ImportedSameName ImportedSameName)
ADD_TEST_MACRO(InterfaceLibrary InterfaceLibrary) ADD_TEST_MACRO(InterfaceLibrary InterfaceLibrary)
if (CMAKE_BUILD_TYPE MATCHES "[Dd][Ee][Bb][Uu][Gg]") if (CMAKE_BUILD_TYPE MATCHES "[Dd][Ee][Bb][Uu][Gg]")
set(ConfigSources_BUILD_OPTIONS -DCMAKE_BUILD_TYPE=Debug) set(ConfigSources_BUILD_OPTIONS -DCMAKE_BUILD_TYPE=Debug)
......
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)
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)
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)
extern void a(void);
extern void b(void);
int main(void)
{
a();
b();
return 0;
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment