Merge topic 'clean-up-link-configuration'

a1cfc4fe cmMakefile: Simplify programmer error to an assert
4079ba20 cmMakefile: Implement LinkLibraries as an internal property
17ab8e33 cmMakefile: Inline method into only remaining caller
7edfcd0e cmMakefile: Inline method into caller
6c8dc7f1 cmake: Simplify find-package mode library addition
1efca9f4 cmMakefile: Remove obsolete parameter
d9b5f0a3 cmTarget: Remove target name from parameter list
1c70c6cc cmMakefile: Use public API to find a target
2b7baed7 cmMakefile: Inline method into only caller
7ba95492 cmMakefile: Use public API to find a target
6d98b15f cmMakefile: Invert if() condition to remove else
869037ee cmMakefile: Remove ALIAS check
2f6462a6 cmMakefile: Collapse two consecutive if()s into one
148b83a1 cmMakefile: DeMorgan-invert condition
4457a9f1 cmMakefile: Return after error and remove else condition
4d039c5b cmMakefile: Invert handling of error condition
...

Source/cmCPluginAPI.cxx

@@ -162,7 +162,14 @@ void CCONV cmAddLinkDirectoryForTarget(void* arg, const char* tgt,
                                        const char* d)
 {
   cmMakefile* mf = static_cast<cmMakefile*>(arg);
-  mf->AddLinkDirectoryForTarget(tgt, d);
+  cmTarget* t = mf->FindLocalNonAliasTarget(tgt);
+  if (!t) {
+    cmSystemTools::Error(
+      "Attempt to add link directories to non-existent target: ", tgt,
+      " for directory ", d);
+    return;
+  }
+  t->AddLinkDirectory(d);
 }
 
 void CCONV cmAddExecutable(void* arg, const char* exename, int numSrcs,
@@ -330,6 +337,35 @@ void CCONV cmAddCustomCommandToTarget(void* arg, const char* target,
                                cctype, no_comment, no_working_dir);
 }
 
+static void addLinkLibrary(cmMakefile* mf, std::string const& target,
+                           std::string const& lib, cmTargetLinkLibraryType llt)
+{
+  cmTarget* t = mf->FindLocalNonAliasTarget(target);
+  if (!t) {
+    std::ostringstream e;
+    e << "Attempt to add link library \"" << lib << "\" to target \"" << target
+      << "\" which is not built in this directory.";
+    mf->IssueMessage(cmake::FATAL_ERROR, e.str());
+    return;
+  }
+
+  cmTarget* tgt = mf->GetGlobalGenerator()->FindTarget(lib);
+  if (tgt && (tgt->GetType() != cmState::STATIC_LIBRARY) &&
+      (tgt->GetType() != cmState::SHARED_LIBRARY) &&
+      (tgt->GetType() != cmState::INTERFACE_LIBRARY) &&
+      !tgt->IsExecutableWithExports()) {
+    std::ostringstream e;
+    e << "Target \"" << lib << "\" of type "
+      << cmState::GetTargetTypeName(tgt->GetType())
+      << " may not be linked into another target.  "
+      << "One may link only to STATIC or SHARED libraries, or "
+      << "to executables with the ENABLE_EXPORTS property set.";
+    mf->IssueMessage(cmake::FATAL_ERROR, e.str());
+  }
+
+  t->AddLinkLibrary(*mf, lib, llt);
+}
+
 void CCONV cmAddLinkLibraryForTarget(void* arg, const char* tgt,
                                      const char* value, int libtype)
 {
@@ -337,13 +373,13 @@ void CCONV cmAddLinkLibraryForTarget(void* arg, const char* tgt,
 
   switch (libtype) {
     case CM_LIBRARY_GENERAL:
-      mf->AddLinkLibraryForTarget(tgt, value, GENERAL_LibraryType);
+      addLinkLibrary(mf, tgt, value, GENERAL_LibraryType);
       break;
     case CM_LIBRARY_DEBUG:
-      mf->AddLinkLibraryForTarget(tgt, value, DEBUG_LibraryType);
+      addLinkLibrary(mf, tgt, value, DEBUG_LibraryType);
       break;
     case CM_LIBRARY_OPTIMIZED:
-      mf->AddLinkLibraryForTarget(tgt, value, OPTIMIZED_LibraryType);
+      addLinkLibrary(mf, tgt, value, OPTIMIZED_LibraryType);
       break;
   }
 }

Source/cmLinkLibrariesCommand.cxx

@@ -20,7 +20,7 @@ bool cmLinkLibrariesCommand::InitialPass(std::vector<std::string> const& args,
                        "a library");
         return false;
       }
-      this->Makefile->AddLinkLibrary(*i, DEBUG_LibraryType);
+      this->Makefile->AppendProperty("LINK_LIBRARIES", "debug");
     } else if (*i == "optimized") {
       ++i;
       if (i == args.end()) {
@@ -28,10 +28,9 @@ bool cmLinkLibrariesCommand::InitialPass(std::vector<std::string> const& args,
                        "a library");
         return false;
       }
-      this->Makefile->AddLinkLibrary(*i, OPTIMIZED_LibraryType);
-    } else {
-      this->Makefile->AddLinkLibrary(*i);
+      this->Makefile->AppendProperty("LINK_LIBRARIES", "optimized");
     }
+    this->Makefile->AppendProperty("LINK_LIBRARIES", i->c_str());
   }
 
   return true;

Source/cmMakefile.cxx

@@ -1207,71 +1207,6 @@ bool cmMakefile::ParseDefineFlag(std::string const& def, bool remove)
   return true;
 }
 
-void cmMakefile::AddLinkLibrary(const std::string& lib,
-                                cmTargetLinkLibraryType llt)
-{
-  cmTarget::LibraryID tmp;
-  tmp.first = lib;
-  tmp.second = llt;
-  this->LinkLibraries.push_back(tmp);
-}
-
-void cmMakefile::AddLinkLibraryForTarget(const std::string& target,
-                                         const std::string& lib,
-                                         cmTargetLinkLibraryType llt)
-{
-  cmTargets::iterator i = this->Targets.find(target);
-  if (i != this->Targets.end()) {
-    cmTarget* tgt = this->GetGlobalGenerator()->FindTarget(lib);
-    if (tgt) {
-      // if it is not a static or shared library then you can not link to it
-      if (!((tgt->GetType() == cmState::STATIC_LIBRARY) ||
-            (tgt->GetType() == cmState::SHARED_LIBRARY) ||
-            (tgt->GetType() == cmState::INTERFACE_LIBRARY) ||
-            tgt->IsExecutableWithExports())) {
-        std::ostringstream e;
-        e << "Target \"" << lib << "\" of type "
-          << cmState::GetTargetTypeName(tgt->GetType())
-          << " may not be linked into another target.  "
-          << "One may link only to STATIC or SHARED libraries, or "
-          << "to executables with the ENABLE_EXPORTS property set.";
-        this->IssueMessage(cmake::FATAL_ERROR, e.str());
-      }
-    }
-    i->second.AddLinkLibrary(*this, target, lib, llt);
-  } else {
-    std::ostringstream e;
-    e << "Attempt to add link library \"" << lib << "\" to target \"" << target
-      << "\" which is not built in this directory.";
-    this->IssueMessage(cmake::FATAL_ERROR, e.str());
-  }
-}
-
-void cmMakefile::AddLinkDirectoryForTarget(const std::string& target,
-                                           const std::string& d)
-{
-  cmTargets::iterator i = this->Targets.find(target);
-  if (i != this->Targets.end()) {
-    if (this->IsAlias(target)) {
-      std::ostringstream e;
-      e << "ALIAS target \"" << target << "\" "
-        << "may not be linked into another target.";
-      this->IssueMessage(cmake::FATAL_ERROR, e.str());
-      return;
-    }
-    i->second.AddLinkDirectory(d);
-  } else {
-    cmSystemTools::Error(
-      "Attempt to add link directories to non-existent target: ",
-      target.c_str(), " for directory ", d.c_str());
-  }
-}
-
-void cmMakefile::AddLinkLibrary(const std::string& lib)
-{
-  this->AddLinkLibrary(lib, GENERAL_LibraryType);
-}
-
 void cmMakefile::InitializeFromParent(cmMakefile* parent)
 {
   this->SystemIncludeDirectories = parent->SystemIncludeDirectories;
@@ -1303,7 +1238,7 @@ void cmMakefile::InitializeFromParent(cmMakefile* parent)
   }
 
   // link libraries
-  this->LinkLibraries = parent->LinkLibraries;
+  this->SetProperty("LINK_LIBRARIES", parent->GetProperty("LINK_LIBRARIES"));
 
   // link directories
   this->SetProperty("LINK_DIRECTORIES",
@@ -1835,8 +1770,7 @@ void cmMakefile::SetProjectName(std::string const& p)
   this->StateSnapshot.SetProjectName(p);
 }
 
-void cmMakefile::AddGlobalLinkInformation(const std::string& name,
-                                          cmTarget& target)
+void cmMakefile::AddGlobalLinkInformation(cmTarget& target)
 {
   // for these targets do not add anything
   switch (target.GetType()) {
@@ -1857,13 +1791,34 @@ void cmMakefile::AddGlobalLinkInformation(const std::string& name,
       if (*j->rbegin() == '/') {
         newdir = j->substr(0, j->size() - 1);
       }
-      if (std::find(this->LinkDirectories.begin(), this->LinkDirectories.end(),
-                    newdir) == this->LinkDirectories.end()) {
-        target.AddLinkDirectory(*j);
-      }
+      target.AddLinkDirectory(*j);
+    }
+  }
+
+  if (const char* linkLibsProp = this->GetProperty("LINK_LIBRARIES")) {
+    std::vector<std::string> linkLibs;
+    cmSystemTools::ExpandListArgument(linkLibsProp, linkLibs);
+
+    for (std::vector<std::string>::iterator j = linkLibs.begin();
+         j != linkLibs.end(); ++j) {
+      std::string libraryName = *j;
+      cmTargetLinkLibraryType libType = GENERAL_LibraryType;
+      if (libraryName == "optimized") {
+        libType = OPTIMIZED_LibraryType;
+        ++j;
+        libraryName = *j;
+      } else if (libraryName == "debug") {
+        libType = DEBUG_LibraryType;
+        ++j;
+        libraryName = *j;
+      }
+      // This is equivalent to the target_link_libraries plain signature.
+      target.AddLinkLibrary(*this, libraryName, libType);
+      target.AppendProperty(
+        "INTERFACE_LINK_LIBRARIES",
+        target.GetDebugGeneratorExpressions(libraryName, libType).c_str());
     }
   }
-  target.MergeLinkLibraries(*this, name, this->LinkLibraries);
 }
 
 void cmMakefile::AddAlias(const std::string& lname, std::string const& tgtName)
@@ -1877,14 +1832,9 @@ cmTarget* cmMakefile::AddLibrary(const std::string& lname,
                                  const std::vector<std::string>& srcs,
                                  bool excludeFromAll)
 {
-  // wrong type ? default to STATIC
-  if ((type != cmState::STATIC_LIBRARY) && (type != cmState::SHARED_LIBRARY) &&
-      (type != cmState::MODULE_LIBRARY) && (type != cmState::OBJECT_LIBRARY) &&
-      (type != cmState::INTERFACE_LIBRARY)) {
-    this->IssueMessage(cmake::INTERNAL_ERROR,
-                       "cmMakefile::AddLibrary given invalid target type.");
-    type = cmState::STATIC_LIBRARY;
-  }
+  assert(type == cmState::STATIC_LIBRARY || type == cmState::SHARED_LIBRARY ||
+         type == cmState::MODULE_LIBRARY || type == cmState::OBJECT_LIBRARY ||
+         type == cmState::INTERFACE_LIBRARY);
 
   cmTarget* target = this->AddNewTarget(type, lname);
   // Clear its dependencies. Otherwise, dependencies might persist
@@ -1895,7 +1845,7 @@ cmTarget* cmMakefile::AddLibrary(const std::string& lname,
     target->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
   }
   target->AddSources(srcs);
-  this->AddGlobalLinkInformation(lname, *target);
+  this->AddGlobalLinkInformation(*target);
   return target;
 }
 
@@ -1908,7 +1858,7 @@ cmTarget* cmMakefile::AddExecutable(const char* exeName,
     target->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
   }
   target->AddSources(srcs);
-  this->AddGlobalLinkInformation(exeName, *target);
+  this->AddGlobalLinkInformation(*target);
   return target;
 }
 
@@ -2125,19 +2075,32 @@ void cmMakefile::ExpandVariablesCMP0019()
       }
     }
   }
-  for (cmTarget::LinkLibraryVectorType::iterator l =
-         this->LinkLibraries.begin();
-       l != this->LinkLibraries.end(); ++l) {
-    if (mightExpandVariablesCMP0019(l->first.c_str())) {
-      std::string orig = l->first;
-      this->ExpandVariablesInString(l->first, true, true);
-      if (pol == cmPolicies::WARN && l->first != orig) {
-        /* clang-format off */
+
+  if (const char* linkLibsProp = this->GetProperty("LINK_LIBRARIES")) {
+    std::vector<std::string> linkLibs;
+    cmSystemTools::ExpandListArgument(linkLibsProp, linkLibs);
+
+    for (std::vector<std::string>::iterator l = linkLibs.begin();
+         l != linkLibs.end(); ++l) {
+      std::string libName = *l;
+      if (libName == "optimized") {
+        ++l;
+        libName = *l;
+      } else if (libName == "debug") {
+        ++l;
+        libName = *l;
+      }
+      if (mightExpandVariablesCMP0019(libName.c_str())) {
+        std::string orig = libName;
+        this->ExpandVariablesInString(libName, true, true);
+        if (pol == cmPolicies::WARN && libName != orig) {
+          /* clang-format off */
         w << "Evaluated link library\n"
           << "  " << orig << "\n"
           << "as\n"
-          << "  " << l->first << "\n";
-        /* clang-format on */
+          << "  " << libName << "\n";
+          /* clang-format on */
+        }
       }
     }
   }

Source/cmMakefile.h

@@ -196,15 +196,6 @@ public:
     const cmCustomCommandLines& commandLines, bool escapeOldStyle = true,
     const char* comment = CM_NULLPTR, bool uses_terminal = false);
 
-  /**
-   * Add a link library to the build.
-   */
-  void AddLinkLibrary(const std::string&);
-  void AddLinkLibrary(const std::string&, cmTargetLinkLibraryType type);
-  void AddLinkLibraryForTarget(const std::string& tgt, const std::string&,
-                               cmTargetLinkLibraryType type);
-  void AddLinkDirectoryForTarget(const std::string& tgt, const std::string& d);
-
   /**
    * Add a subdirectory to the build.
    */
@@ -781,7 +772,7 @@ public:
 
 protected:
   // add link libraries and directories to the target
-  void AddGlobalLinkInformation(const std::string& name, cmTarget& target);
+  void AddGlobalLinkInformation(cmTarget& target);
 
   // Check for a an unused variable
   void LogUnused(const char* reason, const std::string& name) const;
@@ -805,9 +796,6 @@ protected:
   // Tests
   std::map<std::string, cmTest*> Tests;
 
-  // The link-library paths.  Order matters, use std::vector (not std::set).
-  std::vector<std::string> LinkDirectories;
-
   // The set of include directories that are marked as system include
   // directories.
   std::set<std::string> SystemIncludeDirectories;
@@ -815,8 +803,6 @@ protected:
   std::vector<std::string> ListFiles;
   std::vector<std::string> OutputFiles;
 
-  cmTarget::LinkLibraryVectorType LinkLibraries;
-
   std::vector<cmInstallGenerator*> InstallGenerators;
   std::vector<cmTestGenerator*> TestGenerators;
 

Source/cmTarget.cxx

@@ -476,23 +476,6 @@ cmSourceFile* cmTarget::AddSource(const std::string& src)
   return this->Makefile->GetOrCreateSource(src);
 }
 
-void cmTarget::MergeLinkLibraries(cmMakefile& mf, const std::string& selfname,
-                                  const LinkLibraryVectorType& libs)
-{
-  // Only add on libraries we haven't added on before.
-  // Assumption: the global link libraries could only grow, never shrink
-  LinkLibraryVectorType::const_iterator i = libs.begin();
-  i += this->PrevLinkedLibraries.size();
-  for (; i != libs.end(); ++i) {
-    // This is equivalent to the target_link_libraries plain signature.
-    this->AddLinkLibrary(mf, selfname, i->first, i->second);
-    this->AppendProperty(
-      "INTERFACE_LINK_LIBRARIES",
-      this->GetDebugGeneratorExpressions(i->first, i->second).c_str());
-  }
-  this->PrevLinkedLibraries = libs;
-}
-
 void cmTarget::AddLinkDirectory(const std::string& d)
 {
   // Make sure we don't add unnecessary search directories.
@@ -595,8 +578,7 @@ void cmTarget::GetTllSignatureTraces(std::ostream& s, TLLSignature sig) const
   }
 }
 
-void cmTarget::AddLinkLibrary(cmMakefile& mf, const std::string& target,
-                              const std::string& lib,
+void cmTarget::AddLinkLibrary(cmMakefile& mf, const std::string& lib,
                               cmTargetLinkLibraryType llt)
 {
   cmTarget* tgt = this->Makefile->FindTargetToUse(lib);
@@ -614,7 +596,7 @@ void cmTarget::AddLinkLibrary(cmMakefile& mf, const std::string& target,
 
   if (cmGeneratorExpression::Find(lib) != std::string::npos ||
       (tgt && tgt->GetType() == cmState::INTERFACE_LIBRARY) ||
-      (target == lib)) {
+      (this->Name == lib)) {
     return;
   }
 
@@ -632,7 +614,7 @@ void cmTarget::AddLinkLibrary(cmMakefile& mf, const std::string& target,
   // and we removing one instance will break the link line. Duplicates
   // will be appropriately eliminated at emit time.
   if (this->RecordDependencies) {
-    std::string targetEntry = target;
+    std::string targetEntry = this->Name;
     targetEntry += "_LIB_DEPENDS";
     std::string dependencies;
     const char* old_val = mf.GetDefinition(targetEntry);

Source/cmTarget.h

@@ -142,8 +142,8 @@ public:
    */
   void ClearDependencyInformation(cmMakefile& mf, const std::string& target);
 
-  void AddLinkLibrary(cmMakefile& mf, const std::string& target,
-                      const std::string& lib, cmTargetLinkLibraryType llt);
+  void AddLinkLibrary(cmMakefile& mf, const std::string& lib,
+                      cmTargetLinkLibraryType llt);
   enum TLLSignature
   {
     KeywordTLLSignature,
@@ -153,9 +153,6 @@ public:
                            cmListFileContext const& lfc);
   void GetTllSignatureTraces(std::ostream& s, TLLSignature sig) const;
 
-  void MergeLinkLibraries(cmMakefile& mf, const std::string& selfname,
-                          const LinkLibraryVectorType& libs);
-
   const std::vector<std::string>& GetLinkDirectories() const;
 
   void AddLinkDirectory(const std::string& d);
@@ -299,7 +296,6 @@ private:
   std::vector<cmCustomCommand> PreLinkCommands;
   std::vector<cmCustomCommand> PostBuildCommands;
   std::vector<std::pair<TLLSignature, cmListFileContext> > TLLCommands;
-  LinkLibraryVectorType PrevLinkedLibraries;
   LinkLibraryVectorType OriginalLinkLibraries;
   cmMakefile* Makefile;
   cmTargetInternalPointer Internal;

Source/cmTargetLinkLibrariesCommand.cxx

@@ -338,7 +338,35 @@ bool cmTargetLinkLibrariesCommand::HandleLibrary(const std::string& lib,
   // Handle normal case first.
   if (this->CurrentProcessingState != ProcessingKeywordLinkInterface &&
       this->CurrentProcessingState != ProcessingPlainLinkInterface) {
-    this->Makefile->AddLinkLibraryForTarget(this->Target->GetName(), lib, llt);
+
+    cmTarget* t =
+      this->Makefile->FindLocalNonAliasTarget(this->Target->GetName());
+    if (!t) {
+      std::ostringstream e;
+      e << "Attempt to add link library \"" << lib << "\" to target \""
+        << this->Target->GetName()
+        << "\" which is not built in this directory.";
+      this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
+    } else {
+
+      cmTarget* tgt = this->Makefile->GetGlobalGenerator()->FindTarget(lib);
+
+      if (tgt && (tgt->GetType() != cmState::STATIC_LIBRARY) &&
+          (tgt->GetType() != cmState::SHARED_LIBRARY) &&
+          (tgt->GetType() != cmState::INTERFACE_LIBRARY) &&
+          !tgt->IsExecutableWithExports()) {
+        std::ostringstream e;
+        e << "Target \"" << lib << "\" of type "
+          << cmState::GetTargetTypeName(tgt->GetType())
+          << " may not be linked into another target.  "
+          << "One may link only to STATIC or SHARED libraries, or "
+          << "to executables with the ENABLE_EXPORTS property set.";
+        this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
+      }
+
+      this->Target->AddLinkLibrary(*this->Makefile, lib, llt);
+    }
+
     if (this->CurrentProcessingState == ProcessingLinkLibraries) {
       this->Target->AppendProperty(
         "INTERFACE_LINK_LIBRARIES",

Source/cmake.cxx

@@ -568,7 +568,7 @@ bool cmake::FindPackage(const std::vector<std::string>& args)
     cmSystemTools::ExpandListArgument(libs, libList);
     for (std::vector<std::string>::const_iterator libIt = libList.begin();
          libIt != libList.end(); ++libIt) {
-      mf->AddLinkLibraryForTarget(targetName, *libIt, GENERAL_LibraryType);
+      tgt->AddLinkLibrary(*mf, *libIt, GENERAL_LibraryType);
     }
 
     std::string buildType = mf->GetSafeDefinition("CMAKE_BUILD_TYPE");