Merge topic 'LINK_LIBRARY-genex-with-INTERFACE_LINK_LIBRARIES_DIRECT'

9798482a8c LINK_LIBRARY-genex: correct behavior for INTERFACE_LINK_LIBRARIES_DIRECT

Acked-by: Kitware Robot <[email protected]>
Tested-by: buildbot <[email protected]>
Merge-request: !8992

Source/cmComputeLinkDepends.cxx

@@ -187,15 +187,6 @@ items that we know the linker will reuse automatically (shared libs).
 
 namespace {
 // LINK_LIBRARY helpers
-const auto LL_BEGIN = "<LINK_LIBRARY:"_s;
-const auto LL_END = "</LINK_LIBRARY:"_s;
-
-inline std::string ExtractFeature(std::string const& item)
-{
-  return item.substr(LL_BEGIN.length(),
-                     item.find('>', LL_BEGIN.length()) - LL_BEGIN.length());
-}
-
 bool IsFeatureSupported(cmMakefile* makefile, std::string const& linkLanguage,
                         std::string const& feature)
 {
@@ -434,7 +425,8 @@ private:
 };
 }
 
-const std::string cmComputeLinkDepends::LinkEntry::DEFAULT = "DEFAULT";
+std::string const& cmComputeLinkDepends::LinkEntry::DEFAULT =
+  cmLinkItem::DEFAULT;
 
 cmComputeLinkDepends::cmComputeLinkDepends(const cmGeneratorTarget* target,
                                            const std::string& config,
@@ -632,10 +624,12 @@ std::pair<size_t, bool> cmComputeLinkDepends::AddLinkEntry(
   LinkEntry& entry = this->EntryList[index];
   entry.Item = BT<std::string>(item.AsStr(), item.Backtrace);
   entry.Target = item.Target;
+  entry.Feature = item.Feature;
   if (!entry.Target && entry.Item.Value[0] == '-' &&
       entry.Item.Value[1] != 'l' &&
       entry.Item.Value.substr(0, 10) != "-framework") {
     entry.Kind = LinkEntry::Flag;
+    entry.Feature = LinkEntry::DEFAULT;
   } else if (cmHasPrefix(entry.Item.Value, LG_BEGIN) &&
              cmHasSuffix(entry.Item.Value, '>')) {
     entry.Kind = LinkEntry::Group;
@@ -876,7 +870,6 @@ void cmComputeLinkDepends::AddLinkEntries(size_t depender_index,
 {
   // Track inferred dependency sets implied by this list.
   std::map<size_t, DependSet> dependSets;
-  std::string feature = LinkEntry::DEFAULT;
 
   bool inGroup = false;
   std::pair<size_t, bool> groupIndex{
@@ -893,34 +886,27 @@ void cmComputeLinkDepends::AddLinkEntries(size_t depender_index,
       continue;
     }
 
-    if (cmHasPrefix(item.AsStr(), LL_BEGIN) &&
-        cmHasSuffix(item.AsStr(), '>')) {
-      feature = ExtractFeature(item.AsStr());
-      // emit a warning if an undefined feature is used as part of
-      // an imported target
-      if (depender_index != cmComputeComponentGraph::INVALID_COMPONENT) {
-        const auto& depender = this->EntryList[depender_index];
-        if (depender.Target != nullptr && depender.Target->IsImported() &&
-            !IsFeatureSupported(this->Makefile, this->LinkLanguage, feature)) {
-          this->CMakeInstance->IssueMessage(
-            MessageType::AUTHOR_ERROR,
-            cmStrCat("The 'IMPORTED' target '", depender.Target->GetName(),
-                     "' uses the generator-expression '$<LINK_LIBRARY>' with "
-                     "the feature '",
-                     feature,
-                     "', which is undefined or unsupported.\nDid you miss to "
-                     "define it by setting variables \"CMAKE_",
-                     this->LinkLanguage, "_LINK_LIBRARY_USING_", feature,
-                     "\" and \"CMAKE_", this->LinkLanguage,
-                     "_LINK_LIBRARY_USING_", feature, "_SUPPORTED\"?"),
-            this->Target->GetBacktrace());
-        }
+    // emit a warning if an undefined feature is used as part of
+    // an imported target
+    if (item.Feature != LinkEntry::DEFAULT &&
+        depender_index != cmComputeComponentGraph::INVALID_COMPONENT) {
+      const auto& depender = this->EntryList[depender_index];
+      if (depender.Target != nullptr && depender.Target->IsImported() &&
+          !IsFeatureSupported(this->Makefile, this->LinkLanguage,
+                              item.Feature)) {
+        this->CMakeInstance->IssueMessage(
+          MessageType::AUTHOR_ERROR,
+          cmStrCat("The 'IMPORTED' target '", depender.Target->GetName(),
+                   "' uses the generator-expression '$<LINK_LIBRARY>' with "
+                   "the feature '",
+                   item.Feature,
+                   "', which is undefined or unsupported.\nDid you miss to "
+                   "define it by setting variables \"CMAKE_",
+                   this->LinkLanguage, "_LINK_LIBRARY_USING_", item.Feature,
+                   "\" and \"CMAKE_", this->LinkLanguage,
+                   "_LINK_LIBRARY_USING_", item.Feature, "_SUPPORTED\"?"),
+          this->Target->GetBacktrace());
       }
-      continue;
-    }
-    if (cmHasPrefix(item.AsStr(), LL_END) && cmHasSuffix(item.AsStr(), '>')) {
-      feature = LinkEntry::DEFAULT;
-      continue;
     }
 
     if (cmHasPrefix(item.AsStr(), LG_BEGIN) &&
@@ -981,7 +967,7 @@ void cmComputeLinkDepends::AddLinkEntries(size_t depender_index,
     dependee_index = ale.first;
     LinkEntry& entry = this->EntryList[dependee_index];
     auto const& itemFeature =
-      this->GetCurrentFeature(entry.Item.Value, feature);
+      this->GetCurrentFeature(entry.Item.Value, item.Feature);
     if (inGroup && ale.second && entry.Target != nullptr &&
         (entry.Target->GetType() == cmStateEnums::TargetType::OBJECT_LIBRARY ||
          entry.Target->GetType() ==
@@ -1000,30 +986,27 @@ void cmComputeLinkDepends::AddLinkEntries(size_t depender_index,
           " library '", entry.Item.Value, "'."),
         this->Target->GetBacktrace());
     }
-    if (itemFeature != LinkEntry::DEFAULT) {
-      if (ale.second) {
-        // current item not yet defined
-        if (entry.Target != nullptr &&
-            (entry.Target->GetType() ==
-               cmStateEnums::TargetType::OBJECT_LIBRARY ||
-             entry.Target->GetType() ==
-               cmStateEnums::TargetType::INTERFACE_LIBRARY)) {
-          this->CMakeInstance->IssueMessage(
-            MessageType::AUTHOR_WARNING,
-            cmStrCat("The feature '", feature,
-                     "', specified as part of a generator-expression "
-                     "'$",
-                     LL_BEGIN, feature, ">', will not be applied to the ",
-                     (entry.Target->GetType() ==
-                          cmStateEnums::TargetType::OBJECT_LIBRARY
-                        ? "OBJECT"
-                        : "INTERFACE"),
-                     " library '", entry.Item.Value, "'."),
-            this->Target->GetBacktrace());
-        } else {
-          entry.Feature = itemFeature;
-        }
+    if (ale.second) {
+      // current item not yet defined
+      if (itemFeature != LinkEntry::DEFAULT && entry.Target != nullptr &&
+          (entry.Target->GetType() ==
+             cmStateEnums::TargetType::OBJECT_LIBRARY ||
+           entry.Target->GetType() ==
+             cmStateEnums::TargetType::INTERFACE_LIBRARY)) {
+        this->CMakeInstance->IssueMessage(
+          MessageType::AUTHOR_WARNING,
+          cmStrCat("The feature '", itemFeature,
+                   "', specified as part of a generator-expression "
+                   "'$<LINK_LIBRARY:",
+                   itemFeature, ">', will not be applied to the ",
+                   (entry.Target->GetType() ==
+                        cmStateEnums::TargetType::OBJECT_LIBRARY
+                      ? "OBJECT"
+                      : "INTERFACE"),
+                   " library '", entry.Item.Value, "'."),
+          this->Target->GetBacktrace());
       }
+      entry.Feature = itemFeature;
     }
 
     bool supportedItem = entry.Target == nullptr ||

Source/cmComputeLinkDepends.h

@@ -49,7 +49,7 @@ public:
     {
     }
 
-    static const std::string DEFAULT;
+    static std::string const& DEFAULT;
 
     enum EntryKind
     {

Source/cmGeneratorTarget.cxx

@@ -6901,7 +6901,8 @@ bool cmGeneratorTarget::IsLinkLookupScope(std::string const& n,
 
 cm::optional<cmLinkItem> cmGeneratorTarget::LookupLinkItem(
   std::string const& n, cmListFileBacktrace const& bt,
-  LookupLinkItemScope* scope, LookupSelf lookupSelf) const
+  std::string const& linkFeature, LookupLinkItemScope* scope,
+  LookupSelf lookupSelf) const
 {
   cm::optional<cmLinkItem> maybeItem;
   if (this->IsLinkLookupScope(n, scope->LG)) {
@@ -6913,7 +6914,8 @@ cm::optional<cmLinkItem> cmGeneratorTarget::LookupLinkItem(
       (lookupSelf == LookupSelf::No && name == this->GetName())) {
     return maybeItem;
   }
-  maybeItem = this->ResolveLinkItem(BT<std::string>(name, bt), scope->LG);
+  maybeItem =
+    this->ResolveLinkItem(BT<std::string>(name, bt), scope->LG, linkFeature);
   return maybeItem;
 }
 
@@ -6944,9 +6946,16 @@ void cmGeneratorTarget::ExpandLinkItems(
     cmList libs{ cge->Evaluate(this->LocalGenerator, config, headTarget,
                                &dagChecker, this,
                                headTarget->LinkerLanguage) };
+
+    auto linkFeature = cmLinkItem::DEFAULT;
     for (auto const& lib : libs) {
+      if (auto maybeLinkFeature = ParseLinkFeature(lib)) {
+        linkFeature = std::move(*maybeLinkFeature);
+        continue;
+      }
+
       if (cm::optional<cmLinkItem> maybeItem = this->LookupLinkItem(
-            lib, cge->GetBacktrace(), &scope,
+            lib, cge->GetBacktrace(), linkFeature, &scope,
             field == LinkInterfaceField::Libraries ? LookupSelf::No
                                                    : LookupSelf::Yes)) {
         cmLinkItem item = std::move(*maybeItem);
@@ -7692,9 +7701,16 @@ const cmLinkInterface* cmGeneratorTarget::GetImportLinkInterface(
                           LinkInterfaceField::Libraries, iface);
     cmList deps{ info->SharedDeps };
     LookupLinkItemScope scope{ this->LocalGenerator };
+
+    auto linkFeature = cmLinkItem::DEFAULT;
     for (auto const& dep : deps) {
+      if (auto maybeLinkFeature = ParseLinkFeature(dep)) {
+        linkFeature = std::move(*maybeLinkFeature);
+        continue;
+      }
+
       if (cm::optional<cmLinkItem> maybeItem = this->LookupLinkItem(
-            dep, cmListFileBacktrace(), &scope, LookupSelf::No)) {
+            dep, cmListFileBacktrace(), linkFeature, &scope, LookupSelf::No)) {
         iface.SharedDeps.emplace_back(std::move(*maybeItem));
       }
     }
@@ -8494,7 +8510,13 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries(
       impl.HadLinkLanguageSensitiveCondition = true;
     }
 
+    auto linkFeature = cmLinkItem::DEFAULT;
     for (auto const& lib : llibs) {
+      if (auto maybeLinkFeature = ParseLinkFeature(lib)) {
+        linkFeature = std::move(*maybeLinkFeature);
+        continue;
+      }
+
       if (this->IsLinkLookupScope(lib, lg)) {
         continue;
       }
@@ -8541,8 +8563,8 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries(
       }
 
       // The entry is meant for this configuration.
-      cmLinkItem item =
-        this->ResolveLinkItem(BT<std::string>(name, entry.Backtrace), lg);
+      cmLinkItem item = this->ResolveLinkItem(
+        BT<std::string>(name, entry.Backtrace), lg, linkFeature);
       if (item.Target) {
         auto depsForTarget = synthTargetsForConfig.find(item.Target);
         if (depsForTarget != synthTargetsForConfig.end()) {
@@ -8590,7 +8612,14 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries(
     CMP0003_ComputeLinkType(config, debugConfigs);
   cmTarget::LinkLibraryVectorType const& oldllibs =
     this->Target->GetOriginalLinkLibraries();
+
+  auto linkFeature = cmLinkItem::DEFAULT;
   for (cmTarget::LibraryID const& oldllib : oldllibs) {
+    if (auto maybeLinkFeature = ParseLinkFeature(oldllib.first)) {
+      linkFeature = std::move(*maybeLinkFeature);
+      continue;
+    }
+
     if (oldllib.second != GENERAL_LibraryType && oldllib.second != linkType) {
       std::string name = this->CheckCMP0004(oldllib.first);
       if (name == this->GetName() || name.empty()) {
@@ -8598,7 +8627,7 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries(
       }
       // Support OLD behavior for CMP0003.
       impl.WrongConfigLibraries.push_back(
-        this->ResolveLinkItem(BT<std::string>(name)));
+        this->ResolveLinkItem(BT<std::string>(name), linkFeature));
     }
   }
 }
@@ -8624,19 +8653,20 @@ cmGeneratorTarget::TargetOrString cmGeneratorTarget::ResolveTargetReference(
 }
 
 cmLinkItem cmGeneratorTarget::ResolveLinkItem(
-  BT<std::string> const& name) const
+  BT<std::string> const& name, std::string const& linkFeature) const
 {
-  return this->ResolveLinkItem(name, this->LocalGenerator);
+  return this->ResolveLinkItem(name, this->LocalGenerator, linkFeature);
 }
 
-cmLinkItem cmGeneratorTarget::ResolveLinkItem(BT<std::string> const& name,
-                                              cmLocalGenerator const* lg) const
+cmLinkItem cmGeneratorTarget::ResolveLinkItem(
+  BT<std::string> const& name, cmLocalGenerator const* lg,
+  std::string const& linkFeature) const
 {
   auto bt = name.Backtrace;
   TargetOrString resolved = this->ResolveTargetReference(name.Value, lg);
 
   if (!resolved.Target) {
-    return cmLinkItem(resolved.String, false, bt);
+    return cmLinkItem(resolved.String, false, bt, linkFeature);
   }
 
   // Check deprecation, issue message with `bt` backtrace.
@@ -8657,10 +8687,10 @@ cmLinkItem cmGeneratorTarget::ResolveLinkItem(BT<std::string> const& name,
   // within the project.
   if (resolved.Target->GetType() == cmStateEnums::EXECUTABLE &&
       !resolved.Target->IsExecutableWithExports()) {
-    return cmLinkItem(resolved.Target->GetName(), false, bt);
+    return cmLinkItem(resolved.Target->GetName(), false, bt, linkFeature);
   }
 
-  return cmLinkItem(resolved.Target, false, bt);
+  return cmLinkItem(resolved.Target, false, bt, linkFeature);
 }
 
 bool cmGeneratorTarget::HasPackageReferences() const

Source/cmGeneratorTarget.h

@@ -449,9 +449,12 @@ public:
   TargetOrString ResolveTargetReference(std::string const& name,
                                         cmLocalGenerator const* lg) const;
 
-  cmLinkItem ResolveLinkItem(BT<std::string> const& name) const;
-  cmLinkItem ResolveLinkItem(BT<std::string> const& name,
-                             cmLocalGenerator const* lg) const;
+  cmLinkItem ResolveLinkItem(
+    BT<std::string> const& name,
+    std::string const& linkFeature = cmLinkItem::DEFAULT) const;
+  cmLinkItem ResolveLinkItem(
+    BT<std::string> const& name, cmLocalGenerator const* lg,
+    std::string const& linkFeature = cmLinkItem::DEFAULT) const;
 
   bool HasPackageReferences() const;
   std::vector<std::string> GetPackageReferences() const;
@@ -1182,6 +1185,7 @@ private:
   };
   cm::optional<cmLinkItem> LookupLinkItem(std::string const& n,
                                           cmListFileBacktrace const& bt,
+                                          std::string const& linkFeature,
                                           LookupLinkItemScope* scope,
                                           LookupSelf lookupSelf) const;
 

Source/cmLinkItem.cxx

@@ -4,20 +4,30 @@
 
 #include <utility> // IWYU pragma: keep
 
+#include <cm/optional>
+#include <cm/string_view>
+#include <cmext/string_view>
+
 #include "cmGeneratorTarget.h"
+#include "cmStringAlgorithms.h"
+
+const std::string cmLinkItem::DEFAULT = "DEFAULT";
 
 cmLinkItem::cmLinkItem() = default;
 
-cmLinkItem::cmLinkItem(std::string n, bool c, cmListFileBacktrace bt)
+cmLinkItem::cmLinkItem(std::string n, bool c, cmListFileBacktrace bt,
+                       std::string feature)
   : String(std::move(n))
+  , Feature(std::move(feature))
   , Cross(c)
   , Backtrace(std::move(bt))
 {
 }
 
 cmLinkItem::cmLinkItem(cmGeneratorTarget const* t, bool c,
-                       cmListFileBacktrace bt)
+                       cmListFileBacktrace bt, std::string feature)
   : Target(t)
+  , Feature(std::move(feature))
   , Cross(c)
   , Backtrace(std::move(bt))
 {
@@ -73,3 +83,19 @@ cmLinkImplItem::cmLinkImplItem(cmLinkItem item, bool checkCMP0027)
   , CheckCMP0027(checkCMP0027)
 {
 }
+
+namespace {
+const cm::string_view LL_BEGIN = "<LINK_LIBRARY:"_s;
+const cm::string_view LL_END = "</LINK_LIBRARY:"_s;
+}
+cm::optional<std::string> ParseLinkFeature(std::string const& item)
+{
+  if (cmHasPrefix(item, LL_BEGIN) && cmHasSuffix(item, '>')) {
+    return item.substr(LL_BEGIN.length(),
+                       item.find('>', LL_BEGIN.length()) - LL_BEGIN.length());
+  }
+  if (cmHasPrefix(item, LL_END) && cmHasSuffix(item, '>')) {
+    return cmLinkItem::DEFAULT;
+  }
+  return cm::nullopt;
+}

Source/cmLinkItem.h

@@ -10,6 +10,7 @@
 #include <unordered_map>
 #include <vector>
 
+#include <cm/optional>
 #include <cmext/algorithm>
 
 #include "cmListFileCache.h"
@@ -25,14 +26,20 @@ class cmLinkItem
   std::string String;
 
 public:
+  // default feature: link library without decoration
+  static const std::string DEFAULT;
+
   cmLinkItem();
-  cmLinkItem(std::string s, bool c, cmListFileBacktrace bt);
-  cmLinkItem(cmGeneratorTarget const* t, bool c, cmListFileBacktrace bt);
+  cmLinkItem(std::string s, bool c, cmListFileBacktrace bt,
+             std::string feature = DEFAULT);
+  cmLinkItem(cmGeneratorTarget const* t, bool c, cmListFileBacktrace bt,
+             std::string feature = DEFAULT);
   std::string const& AsStr() const;
   cmGeneratorTarget const* Target = nullptr;
   // The source file representing the external object (used when linking
   // `$<TARGET_OBJECTS>`)
   cmSourceFile const* ObjectSource = nullptr;
+  std::string Feature;
   bool Cross = false;
   cmListFileBacktrace Backtrace;
   friend bool operator<(cmLinkItem const& l, cmLinkItem const& r);
@@ -160,3 +167,6 @@ inline cmTargetLinkLibraryType CMP0003_ComputeLinkType(
   // The current configuration is not a debug configuration.
   return OPTIMIZED_LibraryType;
 }
+
+// Parse LINK_LIBRARY genex markers.
+cm::optional<std::string> ParseLinkFeature(std::string const& item);

Tests/RunCMake/target_link_libraries-LINK_LIBRARY/LINK_LIBRARY-CMP0156-NEW-consuming_LINK_LIBRARIES_DIRECT-check.cmake

@@ -0,0 +1,4 @@
+
+if (actual_stdout MATCHES "(/|-)-LIBFLAG${LINK_SHARED_LIBRARY_PREFIX}base1${LINK_SHARED_LIBRARY_SUFFIX}")
+  set (RunCMake_TEST_FAILED "Found unexpected '--LIBFLAG<base1>'.")
+endif()

Tests/RunCMake/target_link_libraries-LINK_LIBRARY/LINK_LIBRARY-CMP0156-NEW-consuming_LINK_LIBRARIES_DIRECT-result.txt

@@ -0,0 +1 @@
+.*

Tests/RunCMake/target_link_libraries-LINK_LIBRARY/LINK_LIBRARY-CMP0156-OLD-consuming_LINK_LIBRARIES_DIRECT-check.cmake

@@ -0,0 +1,3 @@
+if (actual_stdout MATCHES "(/|-)-LIBFLAG${LINK_SHARED_LIBRARY_PREFIX}base1${LINK_SHARED_LIBRARY_SUFFIX}")
+  set (RunCMake_TEST_FAILED "Found unexpected '--LIBFLAG<base1>'.")
+endif()

Tests/RunCMake/target_link_libraries-LINK_LIBRARY/LINK_LIBRARY-CMP0156-OLD-consuming_LINK_LIBRARIES_DIRECT-result.txt

@@ -0,0 +1 @@
+.*

Tests/RunCMake/target_link_libraries-LINK_LIBRARY/LINK_LIBRARY.cmake

@@ -108,3 +108,10 @@ target_link_libraries(LinkLibrary_override_features4 PRIVATE "$<LINK_LIBRARY:fea
 set_property(TARGET LinkLibrary_override_features4 PROPERTY LINK_LIBRARY_OVERRIDE "feat3,base1,other1")
 set_property(TARGET LinkLibrary_override_features4 PROPERTY LINK_LIBRARY_OVERRIDE_base1 feat2)
 set_property(TARGET LinkLibrary_override_features4 PROPERTY LINK_LIBRARY_OVERRIDE_other1 feat2)
+
+# testing NTERFACE_LINK_LIBRARIES_DIRECT property
+add_library(lib_with_LINK_LIBRARIES_DIRECT SHARED base.c)
+set_property(TARGET lib_with_LINK_LIBRARIES_DIRECT PROPERTY INTERFACE_LINK_LIBRARIES_DIRECT base1)
+
+add_library(LinkLibrary_consuming_LINK_LIBRARIES_DIRECT SHARED lib.c)
+target_link_libraries(LinkLibrary_consuming_LINK_LIBRARIES_DIRECT PRIVATE $<LINK_LIBRARY:feat1,lib_with_LINK_LIBRARIES_DIRECT>)

Tests/RunCMake/target_link_libraries-LINK_LIBRARY/RunCMakeTest.cmake

@@ -62,6 +62,9 @@ if ((RunCMake_GENERATOR MATCHES "Makefiles|Ninja|Xcode"
     # testing target property LINK_LIBRARY_OVERRIDE_<LIBRARY>
     run_cmake_target(LINK_LIBRARY-CMP0156-${policy} override-features3 LinkLibrary_override_features3)
     run_cmake_target(LINK_LIBRARY-CMP0156-${policy} override-features4 LinkLibrary_override_features4)
+
+    # testing target property INTERFACE_LINK_LIBRARIES_DIRECT
+    run_cmake_target(LINK_LIBRARY-CMP0156-${policy} consuming_LINK_LIBRARIES_DIRECT LinkLibrary_consuming_LINK_LIBRARIES_DIRECT)
   endforeach()
 
   run_cmake(imported-target)