Merge topic 'improve-CMP0028-messages'

ea050286e7 CMP0028: Report the target whose link interface has an offending item
a97c92e46e CMP0028: Report backtrace to link item rather than target creation
0dcbf4ab58 cmGeneratorTarget: Record when imported target link iface libs are done

Acked-by: Kitware Robot <kwrobot@kitware.com>
Tested-by: buildbot <buildbot@kitware.com>
Merge-request: !6810

5 files changed, 111 insertions, 32 deletions

diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx
index 1493f24..e370472 100644
--- a/Source/cmGeneratorTarget.cxx
+++ b/Source/cmGeneratorTarget.cxx
@@ -2687,7 +2687,6 @@ public:
     : Config(std::move(config))
     , Languages(languages)
     , HeadTarget(head)
-    , Target(target)
     , SecondPass(secondPass)
   {
     this->Visited.insert(target);
@@ -2696,36 +2695,6 @@ public:
   void Visit(cmLinkItem const& item)
   {
     if (!item.Target) {
-      if (item.AsStr().find("::") != std::string::npos) {
-        bool noMessage = false;
-        MessageType messageType = MessageType::FATAL_ERROR;
-        std::ostringstream e;
-        switch (this->Target->GetLocalGenerator()->GetPolicyStatus(
-          cmPolicies::CMP0028)) {
-          case cmPolicies::WARN: {
-            e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0028) << "\n";
-            messageType = MessageType::AUTHOR_WARNING;
-          } break;
-          case cmPolicies::OLD:
-            noMessage = true;
-            break;
-          case cmPolicies::REQUIRED_IF_USED:
-          case cmPolicies::REQUIRED_ALWAYS:
-          case cmPolicies::NEW:
-            // Issue the fatal message.
-            break;
-        }
-
-        if (!noMessage) {
-          e << "Target \"" << this->Target->GetName()
-            << "\" 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?";
-          this->Target->GetLocalGenerator()->GetCMakeInstance()->IssueMessage(
-            messageType, e.str(), this->Target->GetBacktrace());
-        }
-      }
       return;
     }
     if (!this->Visited.insert(item.Target).second) {
@@ -2758,7 +2727,6 @@ private:
   std::string Config;
   std::unordered_set<std::string>& Languages;
   cmGeneratorTarget const* HeadTarget;
-  const cmGeneratorTarget* Target;
   std::set<cmGeneratorTarget const*> Visited;
   bool SecondPass;
   bool HadLinkLanguageSensitiveCondition = false;
@@ -6277,6 +6245,84 @@ cmComputeLinkInformation* cmGeneratorTarget::GetLinkInformation(
   return i->second.get();
 }
 
+void cmGeneratorTarget::CheckLinkLibraries() const
+{
+  // Check link the implementation for each generated configuration.
+  for (auto const& hmp : this->LinkImplMap) {
+    HeadToLinkImplementationMap const& hm = hmp.second;
+    // There could be several entries used when computing the pre-CMP0022
+    // default link interface.  Check only the entry for our own link impl.
+    auto const hmi = hm.find(this);
+    if (hmi == hm.end() || !hmi->second.LibrariesDone) {
+      continue;
+    }
+    for (cmLinkImplItem const& item : hmi->second.Libraries) {
+      if (!this->VerifyLinkItemColons(LinkItemRole::Implementation, item)) {
+        return;
+      }
+    }
+  }
+
+  // Check link the interface for each generated combination of
+  // configuration and consuming head target.  We should not need to
+  // consider LinkInterfaceUsageRequirementsOnlyMap because its entries
+  // should be a subset of LinkInterfaceMap (with LINK_ONLY left out).
+  for (auto const& hmp : this->LinkInterfaceMap) {
+    for (auto const& hmi : hmp.second) {
+      if (!hmi.second.LibrariesDone) {
+        continue;
+      }
+      for (cmLinkItem const& item : hmi.second.Libraries) {
+        if (!this->VerifyLinkItemColons(LinkItemRole::Interface, item)) {
+          return;
+        }
+      }
+    }
+  }
+}
+
+bool cmGeneratorTarget::VerifyLinkItemColons(LinkItemRole role,
+                                             cmLinkItem const& item) const
+{
+  if (item.Target || item.AsStr().find("::") == std::string::npos) {
+    return true;
+  }
+  MessageType messageType = MessageType::FATAL_ERROR;
+  std::string e;
+  switch (this->GetLocalGenerator()->GetPolicyStatus(cmPolicies::CMP0028)) {
+    case cmPolicies::WARN: {
+      e = cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0028), "\n");
+      messageType = MessageType::AUTHOR_WARNING;
+    } break;
+    case cmPolicies::OLD:
+      return true;
+    case cmPolicies::REQUIRED_IF_USED:
+    case cmPolicies::REQUIRED_ALWAYS:
+    case cmPolicies::NEW:
+      // Issue the fatal message.
+      break;
+  }
+
+  if (role == LinkItemRole::Implementation) {
+    e = cmStrCat(e, "Target \"", this->GetName(), "\" links to");
+  } else {
+    e = cmStrCat(e, "The link interface of target \"", this->GetName(),
+                 "\" contains");
+  }
+  e = cmStrCat(e, ":\n  ", item.AsStr(), "\n",
+               "but the target was not found.  Possible reasons include:\n"
+               "  * There is a typo in the target name.\n"
+               "  * A find_package call is missing for an IMPORTED target.\n"
+               "  * An ALIAS target is missing.\n");
+  cmListFileBacktrace backtrace = item.Backtrace;
+  if (backtrace.Empty()) {
+    backtrace = this->GetBacktrace();
+  }
+  this->GetLocalGenerator()->GetCMakeInstance()->IssueMessage(messageType, e,
+                                                              backtrace);
+  return false;
+}
+
 void cmGeneratorTarget::GetTargetVersion(int& major, int& minor) const
 {
   int patch;
@@ -7284,6 +7330,7 @@ const cmLinkInterface* cmGeneratorTarget::GetImportLinkInterface(
   cmOptionalLinkInterface& iface = hm[headTarget];
   if (!iface.AllDone) {
     iface.AllDone = true;
+    iface.LibrariesDone = true;
     iface.Multiplicity = info->Multiplicity;
     cmExpandList(info->Languages, iface.Languages);
     this->ExpandLinkItems(info->LibrariesProp, cmMakeRange(info->Libraries),
diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h
index 110af43..096e2ea 100644
--- a/Source/cmGeneratorTarget.h
+++ b/Source/cmGeneratorTarget.h
@@ -84,6 +84,10 @@ public:
   cmComputeLinkInformation* GetLinkInformation(
     const std::string& config) const;
 
+  // Perform validation checks on memoized link structures.
+  // Call this after generation is complete.
+  void CheckLinkLibraries() const;
+
   cmStateEnums::TargetType GetType() const;
   const std::string& GetName() const;
   std::string GetExportName() const;
@@ -973,6 +977,13 @@ private:
   cmLinkImplementation const* GetLinkImplementation(const std::string& config,
                                                     bool secondPass) const;
 
+  enum class LinkItemRole
+  {
+    Implementation,
+    Interface,
+  };
+  bool VerifyLinkItemColons(LinkItemRole role, cmLinkItem const& item) const;
+
   // Cache import information from properties for each configuration.
   struct ImportInfo
   {
diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 0a2e7b5..6433681 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -328,6 +328,18 @@ bool cmGlobalGenerator::CheckTargetsForMissingSources() const
   return failed;
 }
 
+void cmGlobalGenerator::CheckTargetLinkLibraries() const
+{
+  for (const auto& generator : this->LocalGenerators) {
+    for (const auto& gt : generator->GetGeneratorTargets()) {
+      gt->CheckLinkLibraries();
+    }
+    for (const auto& gt : generator->GetOwnedImportedGeneratorTargets()) {
+      gt->CheckLinkLibraries();
+    }
+  }
+}
+
 bool cmGlobalGenerator::CheckTargetsForType() const
 {
   if (!this->GetLanguageEnabled("Swift")) {
@@ -1606,6 +1618,9 @@ void cmGlobalGenerator::Generate()
     this->ExtraGenerator->Generate();
   }
 
+  // Perform validation checks on memoized link structures.
+  this->CheckTargetLinkLibraries();
+
   if (!this->CMP0042WarnTargets.empty()) {
     std::ostringstream w;
     w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0042) << "\n";
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 96696aa..2406798 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -684,6 +684,7 @@ private:
 
   virtual void ForceLinkerLanguages();
 
+  void CheckTargetLinkLibraries() const;
   bool CheckTargetsForMissingSources() const;
   bool CheckTargetsForType() const;
   bool CheckTargetsForPchCompilePdb() const;
diff --git a/Source/cmLocalGenerator.h b/Source/cmLocalGenerator.h
index c73cd62..115a54a 100644
--- a/Source/cmLocalGenerator.h
+++ b/Source/cmLocalGenerator.h
@@ -193,6 +193,11 @@ public:
     return this->GeneratorTargets;
   }
 
+  const GeneratorTargetVector& GetOwnedImportedGeneratorTargets() const
+  {
+    return this->OwnedImportedGeneratorTargets;
+  }
+
   void AddGeneratorTarget(std::unique_ptr<cmGeneratorTarget> gt);
   void AddImportedGeneratorTarget(cmGeneratorTarget* gt);
   void AddOwnedImportedGeneratorTarget(std::unique_ptr<cmGeneratorTarget> gt);