cmLinkItem: Convert to a "sum type" over a string and target pointer

Avoid exposing the item name implicitly as std::string.  When the item
is a target, avoid storing a second copy of its name.

Most link item construction is paired with calls to `FindTargetToLink`
to get the possible target pointer.  Rename these methods to
`ResolveLinkItem` and refactor them to construct the entire item.

9 files changed, 144 insertions, 66 deletions

diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt
index 0457984535..628cc6fa06 100644
--- a/Source/CMakeLists.txt
+++ b/Source/CMakeLists.txt
@@ -265,6 +265,7 @@ set(SRCS
   cmInstallDirectoryGenerator.h
   cmInstallDirectoryGenerator.cxx
   cmLinkedTree.h
+  cmLinkItem.cxx
   cmLinkItem.h
   cmLinkLineComputer.cxx
   cmLinkLineComputer.h
diff --git a/Source/cmComputeLinkDepends.cxx b/Source/cmComputeLinkDepends.cxx
index b1f386094b..3659a1e57e 100644
--- a/Source/cmComputeLinkDepends.cxx
+++ b/Source/cmComputeLinkDepends.cxx
@@ -295,22 +295,24 @@ 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);
+  std::map<std::string, int>::iterator lei =
+    this->LinkEntryIndex.find(item.AsStr());
   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);
+  lei = this->AllocateLinkEntry(item.AsStr());
 
   // Initialize the item entry.
   int index = lei->second;
   LinkEntry& entry = this->EntryList[index];
-  entry.Item = item;
+  entry.Item = item.AsStr();
   entry.Target = item.Target;
-  entry.IsFlag = (!entry.Target && item[0] == '-' && item[1] != 'l' &&
-                  item.substr(0, 10) != "-framework");
+  entry.IsFlag =
+    (!entry.Target && entry.Item[0] == '-' && entry.Item[1] != 'l' &&
+     entry.Item.substr(0, 10) != "-framework");
 
   // If the item has dependencies queue it to follow them.
   if (entry.Target) {
@@ -396,14 +398,14 @@ 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);
+    this->LinkEntryIndex.find(dep.Item.AsStr());
   if (lei == this->LinkEntryIndex.end()) {
     // Allocate a spot for the item entry.
-    lei = this->AllocateLinkEntry(dep.Item);
+    lei = this->AllocateLinkEntry(dep.Item.AsStr());
 
     // Initialize the item entry.
     LinkEntry& entry = this->EntryList[lei->second];
-    entry.Item = dep.Item;
+    entry.Item = dep.Item.AsStr();
     entry.Target = dep.Item.Target;
 
     // This item was added specifically because it is a dependent
@@ -473,9 +475,9 @@ void cmComputeLinkDepends::AddVarLinkEntries(int depender_index,
 
       // If the library is meant for this link type then use it.
       if (llt == GENERAL_LibraryType || llt == this->LinkType) {
-        actual_libs.emplace_back(d, this->FindTargetToLink(depender_index, d));
+        actual_libs.emplace_back(this->ResolveLinkItem(depender_index, d));
       } else if (this->OldLinkDirMode) {
-        cmLinkItem item(d, this->FindTargetToLink(depender_index, d));
+        cmLinkItem item = this->ResolveLinkItem(depender_index, d);
         this->CheckWrongConfigItem(item);
       }
 
@@ -512,7 +514,7 @@ void cmComputeLinkDepends::AddLinkEntries(int depender_index,
     // Skip entries that will resolve to the target getting linked or
     // are empty.
     cmLinkItem const& item = l;
-    if (item == this->Target->GetName() || item.empty()) {
+    if (item.AsStr() == this->Target->GetName() || item.AsStr().empty()) {
       continue;
     }
 
@@ -553,8 +555,8 @@ void cmComputeLinkDepends::AddLinkEntries(int depender_index,
   }
 }
 
-cmGeneratorTarget const* cmComputeLinkDepends::FindTargetToLink(
-  int depender_index, const std::string& name)
+cmLinkItem cmComputeLinkDepends::ResolveLinkItem(int depender_index,
+                                                 const std::string& name)
 {
   // Look for a target in the scope of the depender.
   cmGeneratorTarget const* from = this->Target;
@@ -564,7 +566,7 @@ cmGeneratorTarget const* cmComputeLinkDepends::FindTargetToLink(
       from = depender;
     }
   }
-  return from->FindTargetToLink(name);
+  return from->ResolveLinkItem(name);
 }
 
 void cmComputeLinkDepends::InferDependencies()
diff --git a/Source/cmComputeLinkDepends.h b/Source/cmComputeLinkDepends.h
index dd0e029435..fb3a02f31a 100644
--- a/Source/cmComputeLinkDepends.h
+++ b/Source/cmComputeLinkDepends.h
@@ -79,8 +79,7 @@ private:
   void AddDirectLinkEntries();
   template <typename T>
   void AddLinkEntries(int depender_index, std::vector<T> const& libs);
-  cmGeneratorTarget const* FindTargetToLink(int depender_index,
-                                            const std::string& name);
+  cmLinkItem ResolveLinkItem(int depender_index, const std::string& name);
 
   // One entry for each unique item.
   std::vector<LinkEntry> EntryList;
diff --git a/Source/cmComputeTargetDepends.cxx b/Source/cmComputeTargetDepends.cxx
index efdd3a5f86..edda2bd4eb 100644
--- a/Source/cmComputeTargetDepends.cxx
+++ b/Source/cmComputeTargetDepends.cxx
@@ -229,7 +229,7 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
       emitted.insert(depender->GetName());
       for (cmLinkImplItem const& lib : impl->Libraries) {
         // Don't emit the same library twice for this target.
-        if (emitted.insert(lib).second) {
+        if (emitted.insert(lib.AsStr()).second) {
           this->AddTargetDepend(depender_index, lib, true);
           this->AddInterfaceDepends(depender_index, lib, it, emitted);
         }
@@ -245,7 +245,7 @@ void cmComputeTargetDepends::CollectTargetDepends(int depender_index)
     emitted.insert(depender->GetName());
     for (cmLinkItem const& litem : tutils) {
       // Don't emit the same utility twice for this target.
-      if (emitted.insert(litem).second) {
+      if (emitted.insert(litem.AsStr()).second) {
         this->AddTargetDepend(depender_index, litem, false);
       }
     }
@@ -261,7 +261,7 @@ void cmComputeTargetDepends::AddInterfaceDepends(
         dependee->GetLinkInterface(config, depender)) {
     for (cmLinkItem const& lib : iface->Libraries) {
       // Don't emit the same library twice for this target.
-      if (emitted.insert(lib).second) {
+      if (emitted.insert(lib.AsStr()).second) {
         this->AddTargetDepend(depender_index, lib, true);
         this->AddInterfaceDepends(depender_index, lib, config, emitted);
       }
@@ -324,7 +324,7 @@ void cmComputeTargetDepends::AddTargetDepend(int depender_index,
         << depender->GetName() << "\" does not exist.";
 
       cmListFileBacktrace const* backtrace =
-        depender->GetUtilityBacktrace(dependee_name);
+        depender->GetUtilityBacktrace(dependee_name.AsStr());
       if (backtrace) {
         cm->IssueMessage(messageType, e.str(), *backtrace);
       } else {
diff --git a/Source/cmExportFileGenerator.cxx b/Source/cmExportFileGenerator.cxx
index 75d3374287..1c5040abe3 100644
--- a/Source/cmExportFileGenerator.cxx
+++ b/Source/cmExportFileGenerator.cxx
@@ -831,6 +831,16 @@ void cmExportFileGenerator::SetImportDetailProperties(
   }
 }
 
+static std::string const& asString(std::string const& l)
+{
+  return l;
+}
+
+static std::string const& asString(cmLinkItem const& l)
+{
+  return l.AsStr();
+}
+
 template <typename T>
 void cmExportFileGenerator::SetImportLinkProperty(
   std::string const& suffix, cmGeneratorTarget* target,
@@ -850,7 +860,7 @@ void cmExportFileGenerator::SetImportLinkProperty(
     link_entries += sep;
     sep = ";";
 
-    std::string temp = l;
+    std::string temp = asString(l);
     this->AddTargetNamespace(temp, target, missingTargets);
     link_entries += temp;
   }
diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx
index 80a99d526b..377c008e5b 100644
--- a/Source/cmGeneratorTarget.cxx
+++ b/Source/cmGeneratorTarget.cxx
@@ -671,9 +671,12 @@ std::set<cmLinkItem> const& cmGeneratorTarget::GetUtilityItems() const
     this->UtilityItemsDone = true;
     std::set<std::string> const& utilities = this->GetUtilities();
     for (std::string const& i : utilities) {
-      cmGeneratorTarget* gt =
-        this->LocalGenerator->FindGeneratorTargetToUse(i);
-      this->UtilityItems.insert(cmLinkItem(i, gt));
+      if (cmGeneratorTarget* gt =
+            this->LocalGenerator->FindGeneratorTargetToUse(i)) {
+        this->UtilityItems.insert(cmLinkItem(gt));
+      } else {
+        this->UtilityItems.insert(cmLinkItem(i));
+      }
     }
   }
   return this->UtilityItems;
@@ -816,7 +819,8 @@ static void AddInterfaceEntries(
         thisTarget->GetLinkImplementationLibraries(config)) {
     for (cmLinkImplItem const& lib : impl->Libraries) {
       if (lib.Target) {
-        std::string genex = "$<TARGET_PROPERTY:" + lib + "," + prop + ">";
+        std::string genex =
+          "$<TARGET_PROPERTY:" + lib.AsStr() + "," + prop + ">";
         cmGeneratorExpression ge(lib.Backtrace);
         std::unique_ptr<cmCompiledGeneratorExpression> cge = ge.Parse(genex);
         cge->SetEvaluateForBuildsystem(true);
@@ -836,7 +840,7 @@ static void AddObjectEntries(
     for (cmLinkImplItem const& lib : impl->Libraries) {
       if (lib.Target &&
           lib.Target->GetType() == cmStateEnums::OBJECT_LIBRARY) {
-        std::string genex = "$<TARGET_OBJECTS:" + lib + ">";
+        std::string genex = "$<TARGET_OBJECTS:" + lib.AsStr() + ">";
         cmGeneratorExpression ge(lib.Backtrace);
         std::unique_ptr<cmCompiledGeneratorExpression> cge = ge.Parse(genex);
         cge->SetEvaluateForBuildsystem(true);
@@ -860,7 +864,7 @@ static bool processSources(
 
   for (cmGeneratorTarget::TargetPropertyEntry* entry : entries) {
     cmLinkImplItem const& item = entry->LinkImplItem;
-    std::string const& targetName = item;
+    std::string const& targetName = item.AsStr();
     std::vector<std::string> entrySources;
     cmSystemTools::ExpandListArgument(
       entry->ge->Evaluate(tgt->GetLocalGenerator(), config, false, tgt, tgt,
@@ -1756,7 +1760,7 @@ public:
   void Visit(cmLinkItem const& item)
   {
     if (!item.Target) {
-      if (item.find("::") != std::string::npos) {
+      if (item.AsStr().find("::") != std::string::npos) {
         bool noMessage = false;
         cmake::MessageType messageType = cmake::FATAL_ERROR;
         std::ostringstream e;
@@ -1777,7 +1781,7 @@ public:
 
         if (!noMessage) {
           e << "Target \"" << this->Target->GetName()
-            << "\" links to target \"" << item
+            << "\" links to target \"" << item.AsStr()
             << "\" but the target was not found.  Perhaps a find_package() "
                "call is missing for an IMPORTED target, or an ALIAS target is "
                "missing?";
@@ -2477,7 +2481,7 @@ static void processIncludeDirectories(
 {
   for (cmGeneratorTarget::TargetPropertyEntry* entry : entries) {
     cmLinkImplItem const& item = entry->LinkImplItem;
-    std::string const& targetName = item;
+    std::string const& targetName = item.AsStr();
     bool const fromImported = item.Target && item.Target->IsImported();
     bool const checkCMP0027 = item.FromGenex;
     std::vector<std::string> entryIncludes;
@@ -2615,7 +2619,7 @@ std::vector<std::string> cmGeneratorTarget::GetIncludeDirectories(
     cmLinkImplementationLibraries const* impl =
       this->GetLinkImplementationLibraries(config);
     for (cmLinkImplItem const& lib : impl->Libraries) {
-      std::string libDir = cmSystemTools::CollapseFullPath(lib);
+      std::string libDir = cmSystemTools::CollapseFullPath(lib.AsStr());
 
       static cmsys::RegularExpression frameworkCheck(
         "(.*\\.framework)(/Versions/[^/]+)?/[^/]+$");
@@ -4495,7 +4499,7 @@ void cmGeneratorTarget::LookupLinkItems(std::vector<std::string> const& names,
     if (name == this->GetName() || name.empty()) {
       continue;
     }
-    items.emplace_back(name, this->FindTargetToLink(name));
+    items.push_back(this->ResolveLinkItem(name));
   }
 }
 
@@ -4573,12 +4577,12 @@ void cmGeneratorTarget::ComputeLinkInterface(
       // on other shared libraries that are not in the interface.
       std::unordered_set<std::string> emitted;
       for (cmLinkItem const& lib : iface.Libraries) {
-        emitted.insert(lib);
+        emitted.insert(lib.AsStr());
       }
       if (this->GetType() != cmStateEnums::INTERFACE_LIBRARY) {
         cmLinkImplementation const* impl = this->GetLinkImplementation(config);
         for (cmLinkImplItem const& lib : impl->Libraries) {
-          if (emitted.insert(lib).second) {
+          if (emitted.insert(lib.AsStr()).second) {
             if (lib.Target) {
               // This is a runtime dependency on another shared library.
               if (lib.Target->GetType() == cmStateEnums::SHARED_LIBRARY) {
@@ -5603,7 +5607,7 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries(
       }
 
       // The entry is meant for this configuration.
-      impl.Libraries.emplace_back(name, this->FindTargetToLink(name), *btIt,
+      impl.Libraries.emplace_back(this->ResolveLinkItem(name), *btIt,
                                   evaluated != *le);
     }
 
@@ -5631,14 +5635,12 @@ void cmGeneratorTarget::ComputeLinkImplementationLibraries(
         continue;
       }
       // Support OLD behavior for CMP0003.
-      impl.WrongConfigLibraries.emplace_back(name,
-                                             this->FindTargetToLink(name));
+      impl.WrongConfigLibraries.push_back(this->ResolveLinkItem(name));
     }
   }
 }
 
-cmGeneratorTarget* cmGeneratorTarget::FindTargetToLink(
-  std::string const& name) const
+cmLinkItem cmGeneratorTarget::ResolveLinkItem(std::string const& name) const
 {
   cmGeneratorTarget* tgt =
     this->LocalGenerator->FindGeneratorTargetToUse(name);
@@ -5651,7 +5653,11 @@ cmGeneratorTarget* cmGeneratorTarget::FindTargetToLink(
     tgt = nullptr;
   }
 
-  return tgt;
+  if (tgt) {
+    return cmLinkItem(tgt);
+  }
+
+  return cmLinkItem(name);
 }
 
 std::string cmGeneratorTarget::GetPDBDirectory(const std::string& config) const
diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h
index 1030d91983..a847e21c1a 100644
--- a/Source/cmGeneratorTarget.h
+++ b/Source/cmGeneratorTarget.h
@@ -357,7 +357,7 @@ public:
                                           cmOptionalLinkImplementation& impl,
                                           const cmGeneratorTarget* head) const;
 
-  cmGeneratorTarget* FindTargetToLink(std::string const& name) const;
+  cmLinkItem ResolveLinkItem(std::string const& name) const;
 
   // Compute the set of languages compiled by the target.  This is
   // computed every time it is called because the languages can change
diff --git a/Source/cmLinkItem.cxx b/Source/cmLinkItem.cxx
new file mode 100644
index 0000000000..69b6821c16
--- /dev/null
+++ b/Source/cmLinkItem.cxx
@@ -0,0 +1,72 @@
+/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
+   file Copyright.txt or https://cmake.org/licensing for details.  */
+#include "cmLinkItem.h"
+
+#include "cmGeneratorTarget.h"
+
+#include <utility> // IWYU pragma: keep
+
+cmLinkItem::cmLinkItem()
+  : String()
+  , Target(nullptr)
+{
+}
+
+cmLinkItem::cmLinkItem(std::string const& n)
+  : String(n)
+  , Target(nullptr)
+{
+}
+
+cmLinkItem::cmLinkItem(cmGeneratorTarget const* t)
+  : String()
+  , Target(t)
+{
+}
+
+std::string const& cmLinkItem::AsStr() const
+{
+  return this->Target ? this->Target->GetName() : this->String;
+}
+
+bool operator<(cmLinkItem const& l, cmLinkItem const& r)
+{
+  // Order among targets.
+  if (l.Target && r.Target) {
+    return l.Target < r.Target;
+  }
+  // Order targets before strings.
+  if (l.Target) {
+    return true;
+  }
+  if (r.Target) {
+    return false;
+  }
+  // Order among strings.
+  return l.String < r.String;
+}
+
+bool operator==(cmLinkItem const& l, cmLinkItem const& r)
+{
+  return l.Target == r.Target && l.String == r.String;
+}
+
+std::ostream& operator<<(std::ostream& os, cmLinkItem const& item)
+{
+  return os << item.AsStr();
+}
+
+cmLinkImplItem::cmLinkImplItem()
+  : cmLinkItem()
+  , Backtrace()
+  , FromGenex(false)
+{
+}
+
+cmLinkImplItem::cmLinkImplItem(cmLinkItem item, cmListFileBacktrace const& bt,
+                               bool fromGenex)
+  : cmLinkItem(std::move(item))
+  , Backtrace(bt)
+  , FromGenex(fromGenex)
+{
+}
diff --git a/Source/cmLinkItem.h b/Source/cmLinkItem.h
index e8c94871b8..74fd298e45 100644
--- a/Source/cmLinkItem.h
+++ b/Source/cmLinkItem.h
@@ -7,6 +7,7 @@
 
 #include <algorithm>
 #include <map>
+#include <ostream>
 #include <string>
 #include <vector>
 
@@ -17,40 +18,27 @@
 class cmGeneratorTarget;
 
 // Basic information about each link item.
-class cmLinkItem : public std::string
+class cmLinkItem
 {
-  typedef std::string std_string;
+  std::string String;
 
 public:
-  cmLinkItem()
-    : std_string()
-    , Target(nullptr)
-  {
-  }
-  cmLinkItem(const std_string& n, cmGeneratorTarget const* t)
-    : std_string(n)
-    , Target(t)
-  {
-  }
+  cmLinkItem();
+  explicit cmLinkItem(std::string const& s);
+  explicit cmLinkItem(cmGeneratorTarget const* t);
+  std::string const& AsStr() const;
   cmGeneratorTarget const* Target;
+  friend bool operator<(cmLinkItem const& l, cmLinkItem const& r);
+  friend bool operator==(cmLinkItem const& l, cmLinkItem const& r);
+  friend std::ostream& operator<<(std::ostream& os, cmLinkItem const& item);
 };
 
 class cmLinkImplItem : public cmLinkItem
 {
 public:
-  cmLinkImplItem()
-    : cmLinkItem()
-    , Backtrace()
-    , FromGenex(false)
-  {
-  }
-  cmLinkImplItem(std::string const& n, cmGeneratorTarget const* t,
-                 cmListFileBacktrace const& bt, bool fromGenex)
-    : cmLinkItem(n, t)
-    , Backtrace(bt)
-    , FromGenex(fromGenex)
-  {
-  }
+  cmLinkImplItem();
+  cmLinkImplItem(cmLinkItem item, cmListFileBacktrace const& bt,
+                 bool fromGenex);
   cmListFileBacktrace Backtrace;
   bool FromGenex;
 };