From 73337cb383a704664a47fd3fec84c8feaa303995 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 22 Mar 2022 09:31:36 -0400 Subject: LINK_LIBRARIES: Evaluate separately for linking and usage requirements We evaluate `LINK_LIBRARIES` and `INTERFACE_LINK_LIBRARIES` for two purposes: * Constructing the link line. * Collecting usage requirements. We evaluate `INTERFACE_LINK_LIBRARIES` separately for each purpose in order to support the `$` generator expression used to express private link dependencies of a static library. Previously we only evaluated `LINK_LIBRARIES` for linking, and used that result for collecting usage requirements too. Therefore `$` does not work in `LINK_LIBRARIES`. With the introduction of `INTERFACE_LINK_LIBRARIES_DIRECT`, evaluation of `LINK_LIBRARIES` now needs to distinguish these two cases in order to honor link dependencies encountered through `$` without also exposing other usage requirements through private dependencies of a static library. Revise internal infrastructure to distinguish the two cases when evaluating `LINK_LIBRARIES`. Make the information available in code paths for `INTERFACE_LINK_LIBRARIES_DIRECT` and `LINK_ONLY`. Defer actually using the information to later commits. Issue: #22496 --- Source/cmComputeLinkDepends.cxx | 4 +- Source/cmComputeTargetDepends.cxx | 4 +- Source/cmExportBuildAndroidMKGenerator.cxx | 3 +- Source/cmGeneratorExpressionNode.cxx | 3 +- Source/cmGeneratorTarget.cxx | 83 ++++++++++++++++++++---------- Source/cmGeneratorTarget.h | 17 ++++-- Source/cmGlobalXCodeGenerator.cxx | 5 +- Source/cmLinkItemGraphVisitor.cxx | 4 +- Source/cmLinkLineDeviceComputer.cxx | 3 +- Source/cmQtAutoGenInitializer.cxx | 6 ++- 10 files changed, 89 insertions(+), 43 deletions(-) diff --git a/Source/cmComputeLinkDepends.cxx b/Source/cmComputeLinkDepends.cxx index a4dc01b..8cbdcaa 100644 --- a/Source/cmComputeLinkDepends.cxx +++ b/Source/cmComputeLinkDepends.cxx @@ -677,8 +677,8 @@ void cmComputeLinkDepends::AddVarLinkEntries(int depender_index, void cmComputeLinkDepends::AddDirectLinkEntries() { // Add direct link dependencies in this configuration. - cmLinkImplementation const* impl = - this->Target->GetLinkImplementation(this->Config); + cmLinkImplementation const* impl = this->Target->GetLinkImplementation( + this->Config, cmGeneratorTarget::LinkInterfaceFor::Link); this->AddLinkEntries(-1, impl->Libraries); this->AddLinkObjects(impl->Objects); diff --git a/Source/cmComputeTargetDepends.cxx b/Source/cmComputeTargetDepends.cxx index ef89c8b..3ff16a4 100644 --- a/Source/cmComputeTargetDepends.cxx +++ b/Source/cmComputeTargetDepends.cxx @@ -218,8 +218,8 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index) emitted.insert(cmLinkItem(depender, false, cmListFileBacktrace())); emitted.insert(cmLinkItem(depender, true, cmListFileBacktrace())); - if (cmLinkImplementation const* impl = - depender->GetLinkImplementation(it)) { + if (cmLinkImplementation const* impl = depender->GetLinkImplementation( + it, cmGeneratorTarget::LinkInterfaceFor::Link)) { for (cmLinkImplItem const& lib : impl->Libraries) { // Don't emit the same library twice for this target. if (emitted.insert(lib).second) { diff --git a/Source/cmExportBuildAndroidMKGenerator.cxx b/Source/cmExportBuildAndroidMKGenerator.cxx index bcd8c64..1441925 100644 --- a/Source/cmExportBuildAndroidMKGenerator.cxx +++ b/Source/cmExportBuildAndroidMKGenerator.cxx @@ -168,7 +168,8 @@ void cmExportBuildAndroidMKGenerator::GenerateInterfaceProperties( // Tell the NDK build system if prebuilt static libraries use C++. if (target->GetType() == cmStateEnums::STATIC_LIBRARY) { - cmLinkImplementation const* li = target->GetLinkImplementation(config); + cmLinkImplementation const* li = target->GetLinkImplementation( + config, cmGeneratorTarget::LinkInterfaceFor::Link); if (cm::contains(li->Languages, "CXX")) { os << "LOCAL_HAS_CPP := true\n"; } diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index db043ec..3d1cfbf 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -1415,7 +1415,8 @@ static std::string getLinkedTargetsContent( { std::string result; if (cmLinkImplementationLibraries const* impl = - target->GetLinkImplementationLibraries(context->Config)) { + target->GetLinkImplementationLibraries( + context->Config, cmGeneratorTarget::LinkInterfaceFor::Usage)) { for (cmLinkImplItem const& lib : impl->Libraries) { if (lib.Target) { // Pretend $ appeared in our diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index ab3ed12..17a521a 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -752,6 +752,7 @@ void cmGeneratorTarget::ClearSourcesCache() this->Objects.clear(); this->VisitedConfigsForObjects.clear(); this->LinkImplMap.clear(); + this->LinkImplUsageRequirementsOnlyMap.clear(); } void cmGeneratorTarget::ClearLinkInterfaceCache() @@ -1302,7 +1303,8 @@ bool cmGeneratorTarget::IsSystemIncludeDirectory( &dagChecker, result, excludeImported, language); } - cmLinkImplementation const* impl = this->GetLinkImplementation(config); + cmLinkImplementation const* impl = + this->GetLinkImplementation(config, LinkInterfaceFor::Usage); if (impl != nullptr) { auto runtimeEntries = impl->LanguageRuntimeLibraries.find(language); if (runtimeEntries != impl->LanguageRuntimeLibraries.end()) { @@ -1519,7 +1521,8 @@ void AddLangSpecificImplicitIncludeDirectories( const std::string& config, const std::string& propertyName, IncludeDirectoryFallBack mode, EvaluatedTargetPropertyEntries& entries) { - if (const auto* libraries = target->GetLinkImplementationLibraries(config)) { + if (const auto* libraries = target->GetLinkImplementationLibraries( + config, LinkInterfaceFor::Usage)) { cmGeneratorExpressionDAGChecker dag{ target->GetBacktrace(), target, propertyName, nullptr, nullptr }; @@ -1609,7 +1612,7 @@ void AddInterfaceEntries( { if (searchRuntime == IncludeRuntimeInterface::Yes) { if (cmLinkImplementation const* impl = - headTarget->GetLinkImplementation(config)) { + headTarget->GetLinkImplementation(config, interfaceFor)) { entries.HadContextSensitiveCondition = impl->HadContextSensitiveCondition; @@ -1623,7 +1626,7 @@ void AddInterfaceEntries( } } else { if (cmLinkImplementationLibraries const* impl = - headTarget->GetLinkImplementationLibraries(config)) { + headTarget->GetLinkImplementationLibraries(config, interfaceFor)) { entries.HadContextSensitiveCondition = impl->HadContextSensitiveCondition; addInterfaceEntry(headTarget, config, prop, lang, dagChecker, entries, @@ -1638,7 +1641,8 @@ void AddObjectEntries(cmGeneratorTarget const* headTarget, EvaluatedTargetPropertyEntries& entries) { if (cmLinkImplementationLibraries const* impl = - headTarget->GetLinkImplementationLibraries(config)) { + headTarget->GetLinkImplementationLibraries(config, + LinkInterfaceFor::Usage)) { entries.HadContextSensitiveCondition = impl->HadContextSensitiveCondition; for (cmLinkImplItem const& lib : impl->Libraries) { if (lib.Target && @@ -2821,7 +2825,7 @@ bool cmGeneratorTarget::ComputeLinkClosure(const std::string& config, // Get languages built in this target. std::unordered_set languages; cmLinkImplementation const* impl = - this->GetLinkImplementation(config, secondPass); + this->GetLinkImplementation(config, LinkInterfaceFor::Link, secondPass); assert(impl); languages.insert(impl->Languages.cbegin(), impl->Languages.cend()); @@ -3089,7 +3093,7 @@ cmGeneratorTarget::GetLinkImplementationClosure( std::set emitted; cmLinkImplementationLibraries const* impl = - this->GetLinkImplementationLibraries(config); + this->GetLinkImplementationLibraries(config, LinkInterfaceFor::Usage); assert(impl); for (cmLinkImplItem const& lib : impl->Libraries) { @@ -3831,7 +3835,8 @@ std::vector> cmGeneratorTarget::GetIncludeDirectories( if (this->Makefile->IsOn("APPLE")) { if (cmLinkImplementationLibraries const* impl = - this->GetLinkImplementationLibraries(config)) { + this->GetLinkImplementationLibraries(config, + LinkInterfaceFor::Usage)) { for (cmLinkImplItem const& lib : impl->Libraries) { std::string libDir = cmSystemTools::CollapseFullPath( lib.AsStr(), this->Makefile->GetHomeOutputDirectory()); @@ -6845,8 +6850,8 @@ void cmGeneratorTarget::ComputeLinkInterface( emitted.insert(lib); } if (this->GetType() != cmStateEnums::INTERFACE_LIBRARY) { - cmLinkImplementation const* impl = - this->GetLinkImplementation(config, secondPass); + cmLinkImplementation const* impl = this->GetLinkImplementation( + config, LinkInterfaceFor::Link, secondPass); for (cmLinkImplItem const& lib : impl->Libraries) { if (emitted.insert(lib).second) { if (lib.Target) { @@ -6868,15 +6873,16 @@ void cmGeneratorTarget::ComputeLinkInterface( this->GetPolicyStatusCMP0022() == cmPolicies::OLD) { // The link implementation is the default link interface. cmLinkImplementationLibraries const* impl = - this->GetLinkImplementationLibrariesInternal(config, headTarget); + this->GetLinkImplementationLibrariesInternal(config, headTarget, + LinkInterfaceFor::Link); iface.ImplementationIsInterface = true; iface.WrongConfigLibraries = impl->WrongConfigLibraries; } if (this->LinkLanguagePropagatesToDependents()) { // Targets using this archive need its language runtime libraries. - if (cmLinkImplementation const* impl = - this->GetLinkImplementation(config, secondPass)) { + if (cmLinkImplementation const* impl = this->GetLinkImplementation( + config, LinkInterfaceFor::Link, secondPass)) { iface.Languages = impl->Languages; } } @@ -7307,7 +7313,8 @@ void cmGeneratorTarget::ComputeLinkInterfaceLibraries( // The link implementation is the default link interface. if (cmLinkImplementationLibraries const* impl = - this->GetLinkImplementationLibrariesInternal(config, headTarget)) { + this->GetLinkImplementationLibrariesInternal(config, headTarget, + interfaceFor)) { iface.Libraries.insert(iface.Libraries.end(), impl->Libraries.begin(), impl->Libraries.end()); if (this->GetPolicyStatusCMP0022() == cmPolicies::WARN && @@ -7686,27 +7693,30 @@ cmGeneratorTarget::GetHeadToLinkInterfaceUsageRequirementsMap( } const cmLinkImplementation* cmGeneratorTarget::GetLinkImplementation( - const std::string& config) const + const std::string& config, LinkInterfaceFor implFor) const { - return this->GetLinkImplementation(config, false); + return this->GetLinkImplementation(config, implFor, false); } const cmLinkImplementation* cmGeneratorTarget::GetLinkImplementation( - const std::string& config, bool secondPass) const + const std::string& config, LinkInterfaceFor implFor, bool secondPass) const { // There is no link implementation for targets that cannot compile sources. if (!this->CanCompileSources()) { return nullptr; } - cmOptionalLinkImplementation& impl = - this->LinkImplMap[cmSystemTools::UpperCase(config)][this]; + HeadToLinkImplementationMap& hm = + (implFor == LinkInterfaceFor::Usage + ? this->GetHeadToLinkImplementationUsageRequirementsMap(config) + : this->GetHeadToLinkImplementationMap(config)); + cmOptionalLinkImplementation& impl = hm[this]; if (secondPass) { impl = cmOptionalLinkImplementation(); } if (!impl.LibrariesDone) { impl.LibrariesDone = true; - this->ComputeLinkImplementationLibraries(config, impl, this); + this->ComputeLinkImplementationLibraries(config, impl, this, implFor); } if (!impl.LanguagesDone) { impl.LanguagesDone = true; @@ -7716,6 +7726,21 @@ const cmLinkImplementation* cmGeneratorTarget::GetLinkImplementation( return &impl; } +cmGeneratorTarget::HeadToLinkImplementationMap& +cmGeneratorTarget::GetHeadToLinkImplementationMap( + std::string const& config) const +{ + return this->LinkImplMap[cmSystemTools::UpperCase(config)]; +} + +cmGeneratorTarget::HeadToLinkImplementationMap& +cmGeneratorTarget::GetHeadToLinkImplementationUsageRequirementsMap( + std::string const& config) const +{ + return this + ->LinkImplUsageRequirementsOnlyMap[cmSystemTools::UpperCase(config)]; +} + bool cmGeneratorTarget::GetConfigCommonSourceFilesForXcode( std::vector& files) const { @@ -7956,7 +7981,7 @@ bool cmGeneratorTarget::HaveBuildTreeRPATH(const std::string& config) const return true; } if (cmLinkImplementationLibraries const* impl = - this->GetLinkImplementationLibraries(config)) { + this->GetLinkImplementationLibraries(config, LinkInterfaceFor::Link)) { return !impl->Libraries.empty(); } return false; @@ -7964,14 +7989,15 @@ bool cmGeneratorTarget::HaveBuildTreeRPATH(const std::string& config) const cmLinkImplementationLibraries const* cmGeneratorTarget::GetLinkImplementationLibraries( - const std::string& config) const + const std::string& config, LinkInterfaceFor implFor) const { - return this->GetLinkImplementationLibrariesInternal(config, this); + return this->GetLinkImplementationLibrariesInternal(config, this, implFor); } cmLinkImplementationLibraries const* cmGeneratorTarget::GetLinkImplementationLibrariesInternal( - const std::string& config, cmGeneratorTarget const* head) const + const std::string& config, cmGeneratorTarget const* head, + LinkInterfaceFor implFor) const { // There is no link implementation for targets that cannot compile sources. if (!this->CanCompileSources()) { @@ -7980,7 +8006,9 @@ cmGeneratorTarget::GetLinkImplementationLibrariesInternal( // Populate the link implementation libraries for this configuration. HeadToLinkImplementationMap& hm = - this->LinkImplMap[cmSystemTools::UpperCase(config)]; + (implFor == LinkInterfaceFor::Usage + ? this->GetHeadToLinkImplementationUsageRequirementsMap(config) + : this->GetHeadToLinkImplementationMap(config)); // If the link implementation does not depend on the head target // then re-use the one from the head we computed first. @@ -7991,7 +8019,7 @@ cmGeneratorTarget::GetLinkImplementationLibrariesInternal( cmOptionalLinkImplementation& impl = hm[head]; if (!impl.LibrariesDone) { impl.LibrariesDone = true; - this->ComputeLinkImplementationLibraries(config, impl, head); + this->ComputeLinkImplementationLibraries(config, impl, head, implFor); } return &impl; } @@ -8110,8 +8138,9 @@ void ComputeLinkImplTransitive(cmGeneratorTarget const* self, void cmGeneratorTarget::ComputeLinkImplementationLibraries( const std::string& config, cmOptionalLinkImplementation& impl, - cmGeneratorTarget const* head) const + cmGeneratorTarget const* head, LinkInterfaceFor implFor) const { + static_cast(implFor); cmLocalGenerator const* lg = this->LocalGenerator; cmMakefile const* mf = lg->GetMakefile(); cmBTStringRange entryRange = this->Target->GetLinkImplementationEntries(); diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 3e30913..96e48a4 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -399,17 +399,18 @@ public: LinkClosure const* GetLinkClosure(const std::string& config) const; cmLinkImplementation const* GetLinkImplementation( - const std::string& config) const; + const std::string& config, LinkInterfaceFor implFor) const; void ComputeLinkImplementationLanguages( const std::string& config, cmOptionalLinkImplementation& impl) const; cmLinkImplementationLibraries const* GetLinkImplementationLibraries( - const std::string& config) const; + const std::string& config, LinkInterfaceFor implFor) const; void ComputeLinkImplementationLibraries(const std::string& config, cmOptionalLinkImplementation& impl, - const cmGeneratorTarget* head) const; + const cmGeneratorTarget* head, + LinkInterfaceFor implFor) const; struct TargetOrString { @@ -984,6 +985,7 @@ private: const cmGeneratorTarget* head, bool secondPass) const; cmLinkImplementation const* GetLinkImplementation(const std::string& config, + LinkInterfaceFor implFor, bool secondPass) const; enum class LinkItemRole @@ -1108,9 +1110,16 @@ private: }; using LinkImplMapType = std::map; mutable LinkImplMapType LinkImplMap; + mutable LinkImplMapType LinkImplUsageRequirementsOnlyMap; + + HeadToLinkImplementationMap& GetHeadToLinkImplementationMap( + std::string const& config) const; + HeadToLinkImplementationMap& GetHeadToLinkImplementationUsageRequirementsMap( + std::string const& config) const; cmLinkImplementationLibraries const* GetLinkImplementationLibrariesInternal( - const std::string& config, const cmGeneratorTarget* head) const; + const std::string& config, const cmGeneratorTarget* head, + LinkInterfaceFor implFor) const; bool ComputeOutputDir(const std::string& config, cmStateEnums::ArtifactType artifact, std::string& out) const; diff --git a/Source/cmGlobalXCodeGenerator.cxx b/Source/cmGlobalXCodeGenerator.cxx index 1e6624e..f01b36c 100644 --- a/Source/cmGlobalXCodeGenerator.cxx +++ b/Source/cmGlobalXCodeGenerator.cxx @@ -1627,7 +1627,10 @@ void cmGlobalXCodeGenerator::ForceLinkerLanguage(cmGeneratorTarget* gtgt) // If the language is compiled as a source trust Xcode to link with it. for (auto const& Language : - gtgt->GetLinkImplementation("NOCONFIG")->Languages) { + gtgt + ->GetLinkImplementation("NOCONFIG", + cmGeneratorTarget::LinkInterfaceFor::Link) + ->Languages) { if (Language == llang) { return; } diff --git a/Source/cmLinkItemGraphVisitor.cxx b/Source/cmLinkItemGraphVisitor.cxx index d87d183..0ad846b 100644 --- a/Source/cmLinkItemGraphVisitor.cxx +++ b/Source/cmLinkItemGraphVisitor.cxx @@ -98,8 +98,8 @@ void cmLinkItemGraphVisitor::GetDependencies(cmGeneratorTarget const& target, std::string const& config, DependencyMap& dependencies) { - const auto* implementationLibraries = - target.GetLinkImplementationLibraries(config); + const auto* implementationLibraries = target.GetLinkImplementationLibraries( + config, cmGeneratorTarget::LinkInterfaceFor::Link); if (implementationLibraries != nullptr) { for (auto const& lib : implementationLibraries->Libraries) { auto const& name = lib.AsStr(); diff --git a/Source/cmLinkLineDeviceComputer.cxx b/Source/cmLinkLineDeviceComputer.cxx index 71f9f80..43f1b8e 100644 --- a/Source/cmLinkLineDeviceComputer.cxx +++ b/Source/cmLinkLineDeviceComputer.cxx @@ -136,7 +136,8 @@ void cmLinkLineDeviceComputer::ComputeLinkLibraries( linkLib.Value += " "; const cmLinkImplementation* linkImpl = - cli.GetTarget()->GetLinkImplementation(cli.GetConfig()); + cli.GetTarget()->GetLinkImplementation( + cli.GetConfig(), cmGeneratorTarget::LinkInterfaceFor::Link); for (const cmLinkImplItem& iter : linkImpl->Libraries) { if (iter.Target != nullptr && diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx index a01e6ae..a47b3c0 100644 --- a/Source/cmQtAutoGenInitializer.cxx +++ b/Source/cmQtAutoGenInitializer.cxx @@ -114,7 +114,8 @@ bool StaticLibraryCycle(cmGeneratorTarget const* targetOrigin, } // Collect all static_library dependencies from the test target cmLinkImplementationLibraries const* libs = - testTarget->GetLinkImplementationLibraries(config); + testTarget->GetLinkImplementationLibraries( + config, cmGeneratorTarget::LinkInterfaceFor::Link); if (libs) { for (cmLinkItem const& item : libs->Libraries) { cmGeneratorTarget const* depTarget = item.Target; @@ -1266,7 +1267,8 @@ bool cmQtAutoGenInitializer::InitAutogenTarget() std::map commonTargets; for (std::string const& config : this->ConfigsList) { cmLinkImplementationLibraries const* libs = - this->GenTarget->GetLinkImplementationLibraries(config); + this->GenTarget->GetLinkImplementationLibraries( + config, cmGeneratorTarget::LinkInterfaceFor::Link); if (libs) { for (cmLinkItem const& item : libs->Libraries) { cmGeneratorTarget const* libTarget = item.Target; -- cgit v0.12 From 41a6b4a53ba844ef986b0bc4efe8938b97eea810 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 22 Mar 2022 09:49:11 -0400 Subject: INTERFACE_LINK_LIBRARIES_DIRECT: Honor link dependencies through LINK_ONLY In commit f3ad061858 (Add usage requirements to update direct link dependencies, 2022-01-12, v3.23.0-rc1~44^2), we evaluated the transitive closure of `INTERFACE_LINK_LIBRARIES` as a non-linking usage requirement. That left out `INTERFACE_LINK_LIBRARIES_DIRECT` link dependencies that appear behind private dependencies of a static library, guarded by the `$` generator expression. At the time, that decision was intentional, in order to prevent arbitrary usage requirements from leaking out of `PRIVATE` dependencies. Since then, we've revised evaluation of `LINK_LIBRARIES` to distinguish between collecting link dependencies and other usage requirements. Use that information when following `INTERFACE_LINK_LIBRARIES` to collect the matching kind of requirements from `INTERFACE_LINK_LIBRARIES_DIRECT`. Fixes: #22496 --- Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst | 35 +++++++++++++++------- Source/cmGeneratorTarget.cxx | 14 +++++---- Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt | 9 ++++-- .../exe_use_static_A_private.c | 8 ++--- .../exe_use_static_A_public_explicit.c | 23 ++++++++++++++ 5 files changed, 66 insertions(+), 23 deletions(-) create mode 100644 Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_public_explicit.c diff --git a/Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst b/Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst index aa8cc2e..b8b73df 100644 --- a/Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst +++ b/Help/prop_tgt/INTERFACE_LINK_LIBRARIES_DIRECT.rst @@ -56,11 +56,27 @@ on ``X``'s dependents: target_link_libraries(X PUBLIC Y) then ``Y`` is placed in ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES`, - so ``Y``'s usage requirements, including ``INTERFACE_PROPERTY_LINK_DIRECT`` - and ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, are propagated - to ``X``'s dependents. + so ``Y``'s usage requirements, including ``INTERFACE_PROPERTY_LINK_DIRECT``, + ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, and the usage requirements + declared by the direct link dependencies they add, are propagated to + ``X``'s dependents. -* If ``X`` links ``Y`` privately: +* If ``X`` is a static library or object library, and links ``Y`` privately: + + .. code-block:: cmake + + target_link_libraries(X PRIVATE Y) + + then ``$`` is placed in ``X``'s + :prop_tgt:`INTERFACE_LINK_LIBRARIES`. ``Y``'s linking requirements, + including ``INTERFACE_PROPERTY_LINK_DIRECT``, + ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, and the transitive link + dependencies declared by the direct link dependencies they add, are + propagated to ``X``'s dependents. However, ``Y``'s non-linking + usage requirements are blocked by the :genex:`LINK_ONLY` generator + expression, and are not propagated to ``X``'s dependents. + +* If ``X`` is a shared library or executable, and links ``Y`` privately: .. code-block:: cmake @@ -68,13 +84,10 @@ on ``X``'s dependents: then ``Y`` is not placed in ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES`, so ``Y``'s usage requirements, even ``INTERFACE_PROPERTY_LINK_DIRECT`` - and ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, are not propagated - to ``X``'s dependents. - (If ``X`` is a static library or object library, then ``$`` - is placed in ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES`, but the - :genex:`LINK_ONLY` generator expression block ``Y``'s usage requirements.) + and ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``, are not propagated to + ``X``'s dependents. -* In either case, the content of ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES` +* In all cases, the content of ``X``'s :prop_tgt:`INTERFACE_LINK_LIBRARIES` is not affected by ``Y``'s ``INTERFACE_PROPERTY_LINK_DIRECT`` or ``INTERFACE_PROPERTY_LINK_DIRECT_EXCLUDE``. @@ -184,7 +197,7 @@ be an intermediate library: .. code-block:: cmake add_library(app_impl STATIC app_impl.cpp) - target_link_libraries(app_impl PUBLIC Foo) + target_link_libraries(app_impl PRIVATE Foo) add_executable(app main.cpp) target_link_libraries(app PRIVATE app_impl) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 17a521a..6c804b5 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -8035,6 +8035,7 @@ class TransitiveLinkImpl { cmGeneratorTarget const* Self; std::string const& Config; + LinkInterfaceFor ImplFor; cmLinkImplementation& Impl; std::set Emitted; @@ -8045,9 +8046,10 @@ class TransitiveLinkImpl public: TransitiveLinkImpl(cmGeneratorTarget const* self, std::string const& config, - cmLinkImplementation& impl) + LinkInterfaceFor implFor, cmLinkImplementation& impl) : Self(self) , Config(config) + , ImplFor(implFor) , Impl(impl) { } @@ -8064,8 +8066,8 @@ void TransitiveLinkImpl::Follow(cmGeneratorTarget const* target) } // Get this target's usage requirements. - cmLinkInterfaceLibraries const* iface = target->GetLinkInterfaceLibraries( - this->Config, this->Self, LinkInterfaceFor::Usage); + cmLinkInterfaceLibraries const* iface = + target->GetLinkInterfaceLibraries(this->Config, this->Self, this->ImplFor); if (!iface) { return; } @@ -8129,9 +8131,10 @@ void TransitiveLinkImpl::Compute() void ComputeLinkImplTransitive(cmGeneratorTarget const* self, std::string const& config, + LinkInterfaceFor implFor, cmLinkImplementation& impl) { - TransitiveLinkImpl transitiveLinkImpl(self, config, impl); + TransitiveLinkImpl transitiveLinkImpl(self, config, implFor, impl); transitiveLinkImpl.Compute(); } } @@ -8140,7 +8143,6 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries( const std::string& config, cmOptionalLinkImplementation& impl, cmGeneratorTarget const* head, LinkInterfaceFor implFor) const { - static_cast(implFor); cmLocalGenerator const* lg = this->LocalGenerator; cmMakefile const* mf = lg->GetMakefile(); cmBTStringRange entryRange = this->Target->GetLinkImplementationEntries(); @@ -8245,7 +8247,7 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries( // Update the list of direct link dependencies from usage requirements. if (head == this) { - ComputeLinkImplTransitive(this, config, impl); + ComputeLinkImplTransitive(this, config, implFor, impl); } // Get the list of configurations considered to be DEBUG. diff --git a/Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt b/Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt index b06a2fb..dec131d 100644 --- a/Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt +++ b/Tests/InterfaceLinkLibrariesDirect/CMakeLists.txt @@ -52,7 +52,7 @@ set_property(TARGET A APPEND PROPERTY INTERFACE_LINK_LIBRARIES_DIRECT add_library(static_A_public STATIC static_A_public.c) target_link_libraries(static_A_public PUBLIC A) -# Uses A's usage requirements, but does not propagate them. +# Uses A's usage requirements, but propagates only the linking requirements. # Does not use the exe-only usage requirement. Does use the optional one. add_library(static_A_private STATIC static_A_private.c) target_link_libraries(static_A_private PRIVATE A) @@ -63,10 +63,15 @@ add_executable(exe_use_static_A_public exe_use_static_A_public.c) target_link_libraries(exe_use_static_A_public PRIVATE static_A_public) set_property(TARGET exe_use_static_A_public PROPERTY A_LINK_OPTIONAL 1) -# Does not use A's usage requirements. +# Does not use A's usage requirements, but does use its non-optional linking requirements. add_executable(exe_use_static_A_private exe_use_static_A_private.c) target_link_libraries(exe_use_static_A_private PRIVATE static_A_private) +# Uses A's usage requirements, including an optional one, but overrides the link ordering. +add_executable(exe_use_static_A_public_explicit exe_use_static_A_public_explicit.c) +target_link_libraries(exe_use_static_A_public_explicit PRIVATE static_A_public A direct_from_A direct_from_A_for_exe direct_from_A_optional) +set_property(TARGET exe_use_static_A_public_explicit PROPERTY A_LINK_OPTIONAL 1) + #---------------------------------------------------------------------------- # Test how original and injected dependencies get ordered. diff --git a/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_private.c b/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_private.c index 024e96e..12cf309 100644 --- a/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_private.c +++ b/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_private.c @@ -9,15 +9,15 @@ #endif extern void static_A_private(void); -extern void not_direct_from_A(void); -extern void not_direct_from_A_for_exe(void); +extern void direct_from_A(void); +extern void direct_from_A_for_exe(void); extern void not_direct_from_A_optional(void); int main(void) { static_A_private(); - not_direct_from_A(); - not_direct_from_A_for_exe(); + direct_from_A(); + direct_from_A_for_exe(); not_direct_from_A_optional(); return 0; } diff --git a/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_public_explicit.c b/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_public_explicit.c new file mode 100644 index 0000000..f698a24 --- /dev/null +++ b/Tests/InterfaceLinkLibrariesDirect/exe_use_static_A_public_explicit.c @@ -0,0 +1,23 @@ +#ifndef DEF_DIRECT_FROM_A +# error "DEF_DIRECT_FROM_A incorrectly not defined" +#endif +#ifndef DEF_DIRECT_FROM_A_FOR_EXE +# error "DEF_DIRECT_FROM_A_FOR_EXE incorrectly not defined" +#endif +#ifndef DEF_DIRECT_FROM_A_OPTIONAL +# error "DEF_DIRECT_FROM_A_OPTIONAL incorrectly not defined" +#endif + +extern void static_A_public(void); +extern void not_direct_from_A(void); +extern void not_direct_from_A_for_exe(void); +extern void not_direct_from_A_optional(void); + +int main(void) +{ + static_A_public(); + not_direct_from_A(); + not_direct_from_A_for_exe(); + not_direct_from_A_optional(); + return 0; +} -- cgit v0.12 From cf312a2e5423c49cc5da446f5407a6f4a596a3cb Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 22 Mar 2022 11:09:23 -0400 Subject: LINK_LIBRARIES: Add support for LINK_ONLY genex Previously we always used content guarded by `$` in `LINK_LIBRARIES`, even when evaluating for non-linking usage requirements. Add a policy to honor `LINK_ONLY` in `LINK_LIBRARIES` the same way we already do in `INTERFACE_LINK_LIBRARIES`. --- Help/manual/cmake-generator-expressions.7.rst | 15 ++++++----- Help/manual/cmake-policies.7.rst | 1 + Help/policy/CMP0131.rst | 31 ++++++++++++++++++++++ Help/release/dev/link-interface-direct.rst | 4 +++ Source/cmGeneratorTarget.cxx | 14 ++++++++++ Source/cmPolicies.h | 8 ++++-- Tests/InterfaceLinkLibraries/CMakeLists.txt | 9 +++++++ Tests/InterfaceLinkLibraries/foo_link_only.c | 8 ++++++ Tests/InterfaceLinkLibraries/use_foo_link_only.c | 16 +++++++++++ .../RunCMake/TargetPolicies/PolicyList-stderr.txt | 1 + 10 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 Help/policy/CMP0131.rst create mode 100644 Tests/InterfaceLinkLibraries/foo_link_only.c create mode 100644 Tests/InterfaceLinkLibraries/use_foo_link_only.c diff --git a/Help/manual/cmake-generator-expressions.7.rst b/Help/manual/cmake-generator-expressions.7.rst index b79cf3a..530a406 100644 --- a/Help/manual/cmake-generator-expressions.7.rst +++ b/Help/manual/cmake-generator-expressions.7.rst @@ -1106,12 +1106,15 @@ Output-Related Expressions .. versionadded:: 3.1 - Content of ``...`` except when evaluated in a link interface while - propagating :ref:`Target Usage Requirements`, in which case it is the - empty string. - Intended for use only in an :prop_tgt:`INTERFACE_LINK_LIBRARIES` target - property, perhaps via the :command:`target_link_libraries` command, - to specify private link dependencies without other usage requirements. + Content of ``...``, except while collecting :ref:`Target Usage Requirements`, + in which case it is the empty string. This is intended for use in an + :prop_tgt:`INTERFACE_LINK_LIBRARIES` target property, typically populated + via the :command:`target_link_libraries` command, to specify private link + dependencies without other usage requirements. + + .. versionadded:: 3.24 + ``LINK_ONLY`` may also be used in a :prop_tgt:`LINK_LIBRARIES` target + property. See policy :policy:`CMP0131`. .. genex:: $ diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index dfc5de6..9bf2913 100644 --- a/Help/manual/cmake-policies.7.rst +++ b/Help/manual/cmake-policies.7.rst @@ -58,6 +58,7 @@ Policies Introduced by CMake 3.24 .. toctree:: :maxdepth: 1 + CMP0131: LINK_LIBRARIES supports the LINK_ONLY generator expression. CMP0130: while() diagnoses condition evaluation errors. Policies Introduced by CMake 3.23 diff --git a/Help/policy/CMP0131.rst b/Help/policy/CMP0131.rst new file mode 100644 index 0000000..e85b8ab --- /dev/null +++ b/Help/policy/CMP0131.rst @@ -0,0 +1,31 @@ +CMP0131 +------- + +.. versionadded:: 3.24 + +:prop_tgt:`LINK_LIBRARIES` supports the :genex:`$` +generator expression. + +CMake 3.23 and below documented the :genex:`$` generator +expression only for use in :prop_tgt:`INTERFACE_LINK_LIBRARIES`. +When used in :prop_tgt:`LINK_LIBRARIES`, the content guarded inside +:genex:`$` was always used, even when collecting non-linking +usage requirements such as :prop_tgt:`INTERFACE_COMPILE_DEFINITIONS`. + +CMake 3.24 and above prefer to support :genex:`$`, when +used in :prop_tgt:`LINK_LIBRARIES`, by using the guarded content only +for link dependencies and not other usage requirements. This policy +provides compatibility for projects that have not been updated to +account for this change. + +The ``OLD`` behavior for this policy is to use :prop_tgt:`LINK_LIBRARIES` +content guarded by :genex:`$` even for non-linking +usage requirements. The ``NEW`` behavior for this policy is to to use +the guarded content only for link dependencies. + +This policy was introduced in CMake version 3.24. Use the +:command:`cmake_policy` command to set this policy to ``OLD`` or ``NEW`` +explicitly. Unlike many policies, CMake version |release| does *not* +warn when this policy is not set, and simply uses ``OLD`` behavior. + +.. include:: DEPRECATED.txt diff --git a/Help/release/dev/link-interface-direct.rst b/Help/release/dev/link-interface-direct.rst index 2e9a59e..8b858e2 100644 --- a/Help/release/dev/link-interface-direct.rst +++ b/Help/release/dev/link-interface-direct.rst @@ -5,3 +5,7 @@ link-interface-direct :prop_tgt:`INTERFACE_LINK_LIBRARIES_DIRECT_EXCLUDE` target properties were added to express usage requirements affecting a consumer's direct link dependencies. + +* The :prop_tgt:`LINK_LIBRARIES` target property now supports + the :genex:`$` generator expression. + See policy :policy:`CMP0131`. diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 6c804b5..1a13bdb 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -8152,6 +8152,20 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries( // Keep this logic in sync with ExpandLinkItems. cmGeneratorExpressionDAGChecker dagChecker(this, "LINK_LIBRARIES", nullptr, nullptr); + // The $ expression may be used to specify link dependencies + // that are otherwise excluded from usage requirements. + if (implFor == LinkInterfaceFor::Usage) { + switch (this->GetPolicyStatusCMP0131()) { + case cmPolicies::WARN: + case cmPolicies::OLD: + break; + case cmPolicies::REQUIRED_IF_USED: + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::NEW: + dagChecker.SetTransitivePropertiesOnly(); + break; + } + } cmGeneratorExpression ge(entry.Backtrace); std::unique_ptr const cge = ge.Parse(entry.Value); diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 7d4012d..3b9d067 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -390,7 +390,10 @@ class cmMakefile; "Compiler id for MCST LCC compilers is now LCC, not GNU.", 3, 23, 0, \ cmPolicies::WARN) \ SELECT(POLICY, CMP0130, "while() diagnoses condition evaluation errors.", \ - 3, 24, 0, cmPolicies::WARN) + 3, 24, 0, cmPolicies::WARN) \ + SELECT(POLICY, CMP0131, \ + "LINK_LIBRARIES supports the LINK_ONLY generator expression.", 3, \ + 24, 0, cmPolicies::WARN) #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1) #define CM_FOR_EACH_POLICY_ID(POLICY) \ @@ -426,7 +429,8 @@ class cmMakefile; F(CMP0108) \ F(CMP0112) \ F(CMP0113) \ - F(CMP0119) + F(CMP0119) \ + F(CMP0131) /** \class cmPolicies * \brief Handles changes in CMake behavior and policies diff --git a/Tests/InterfaceLinkLibraries/CMakeLists.txt b/Tests/InterfaceLinkLibraries/CMakeLists.txt index 9e14c44..07a747b 100644 --- a/Tests/InterfaceLinkLibraries/CMakeLists.txt +++ b/Tests/InterfaceLinkLibraries/CMakeLists.txt @@ -59,3 +59,12 @@ set_property(TARGET bar_static_private APPEND PROPERTY INTERFACE_LINK_LIBRARIES add_executable(InterfaceLinkLibraries main_vs6_4.cpp) set_property(TARGET InterfaceLinkLibraries APPEND PROPERTY LINK_LIBRARIES bar_static_private) + +add_library(foo_link_only STATIC foo_link_only.c) +target_compile_definitions(foo_link_only PUBLIC FOO_LINK_ONLY) +add_executable(use_foo_link_only_CMP0131_OLD use_foo_link_only.c) +target_link_libraries(use_foo_link_only_CMP0131_OLD PRIVATE "$") +target_compile_definitions(use_foo_link_only_CMP0131_OLD PRIVATE EXPECT_FOO_LINK_ONLY) +cmake_policy(SET CMP0131 NEW) +add_executable(use_foo_link_only_CMP0131_NEW use_foo_link_only.c) +target_link_libraries(use_foo_link_only_CMP0131_NEW PRIVATE "$") diff --git a/Tests/InterfaceLinkLibraries/foo_link_only.c b/Tests/InterfaceLinkLibraries/foo_link_only.c new file mode 100644 index 0000000..9ca1c01 --- /dev/null +++ b/Tests/InterfaceLinkLibraries/foo_link_only.c @@ -0,0 +1,8 @@ +#ifndef FOO_LINK_ONLY +# error "FOO_LINK_ONLY incorrectly not defined" +#endif + +int foo_link_only(void) +{ + return 0; +} diff --git a/Tests/InterfaceLinkLibraries/use_foo_link_only.c b/Tests/InterfaceLinkLibraries/use_foo_link_only.c new file mode 100644 index 0000000..e975c1b --- /dev/null +++ b/Tests/InterfaceLinkLibraries/use_foo_link_only.c @@ -0,0 +1,16 @@ +#ifdef EXPECT_FOO_LINK_ONLY +# ifndef FOO_LINK_ONLY +# error "FOO_LINK_ONLY incorrectly not defined" +# endif +#else +# ifdef FOO_LINK_ONLY +# error "FOO_LINK_ONLY incorrectly defined" +# endif +#endif + +extern int foo_link_only(void); + +int main(void) +{ + return foo_link_only(); +} diff --git a/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt b/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt index 3846d7c..97c3394 100644 --- a/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt +++ b/Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt @@ -35,6 +35,7 @@ \* CMP0112 \* CMP0113 \* CMP0119 + \* CMP0131 Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\) -- cgit v0.12