target_link_libraries: Add PUBLIC/PRIVATE/INTERFACE keyword signature

Add a new signature to help populate INTERFACE_LINK_LIBRARIES and
LINK_LIBRARIES cleanly in a single call.  Add policy CMP0023 to control
whether the keyword signatures can be mixed with uses of the plain
signatures on the same target.

6 files changed, 232 insertions, 19 deletions

diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index 70c28d4..0ba673e 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -575,6 +575,32 @@ cmPolicies::cmPolicies()
     "property for in-build targets, and ignore the old properties matching "
     "(IMPORTED_)?LINK_INTERFACE_LIBRARIES(_<CONFIG>)?.",
     2,8,11,20130516, cmPolicies::WARN);
+
+  this->DefinePolicy(
+    CMP0023, "CMP0023",
+    "Plain and keyword target_link_libraries signatures cannot be mixed.",
+    "CMake 2.8.12 introduced the target_link_libraries signature using "
+    "the PUBLIC, PRIVATE, and INTERFACE keywords to generalize the "
+    "LINK_PUBLIC and LINK_PRIVATE keywords introduced in CMake 2.8.7.  "
+    "Use of signatures with any of these keywords sets the link interface "
+    "of a target explicitly, even if empty.  "
+    "This produces confusing behavior when used in combination with the "
+    "historical behavior of the plain target_link_libraries signature.  "
+    "For example, consider the code:\n"
+    " target_link_libraries(mylib A)\n"
+    " target_link_libraries(mylib PRIVATE B)\n"
+    "After the first line the link interface has not been set explicitly "
+    "so CMake would use the link implementation, A, as the link interface.  "
+    "However, the second line sets the link interface to empty.  "
+    "In order to avoid this subtle behavior CMake now prefers to disallow "
+    "mixing the plain and keyword signatures of target_link_libraries for "
+    "a single target."
+    "\n"
+    "The OLD behavior for this policy is to allow keyword and plain "
+    "target_link_libraries signatures to be mixed.  "
+    "The NEW behavior for this policy is to not to allow mixing of the "
+    "keyword and plain signatures.",
+    2,8,11,20130724, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 20c953f..5b843a9 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -73,6 +73,7 @@ public:
     CMP0021, ///< Fatal error on relative paths in INCLUDE_DIRECTORIES
     /// target property
     CMP0022, ///< INTERFACE_LINK_LIBRARIES defines the link interface
+    CMP0023, ///< Disallow mixing keyword and plain tll signatures
 
     /** \brief Always the last entry.
      *
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index 26bf9b7..0b3b785 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -2494,6 +2494,57 @@ static std::string targetNameGenex(const char *lib)
 }
 
 //----------------------------------------------------------------------------
+bool cmTarget::PushTLLCommandTrace(TLLSignature signature)
+{
+  bool ret = true;
+  if (!this->TLLCommands.empty())
+    {
+    if (this->TLLCommands.back().first != signature)
+      {
+      ret = false;
+      }
+    }
+  cmListFileBacktrace lfbt;
+  this->Makefile->GetBacktrace(lfbt);
+  this->TLLCommands.push_back(std::make_pair(signature, lfbt));
+  return ret;
+}
+
+//----------------------------------------------------------------------------
+void cmTarget::GetTllSignatureTraces(cmOStringStream &s,
+                                     TLLSignature sig) const
+{
+  std::vector<cmListFileBacktrace> sigs;
+  typedef std::vector<std::pair<TLLSignature, cmListFileBacktrace> > Container;
+  for(Container::const_iterator it = this->TLLCommands.begin();
+      it != this->TLLCommands.end(); ++it)
+    {
+    if (it->first == sig)
+      {
+      sigs.push_back(it->second);
+      }
+    }
+  if (!sigs.empty())
+    {
+    const char *sigString
+                        = (sig == cmTarget::KeywordTLLSignature ? "keyword"
+                                                                : "plain");
+    s << "The uses of the " << sigString << " signature are here:\n";
+    for(std::vector<cmListFileBacktrace>::const_iterator it = sigs.begin();
+        it != sigs.end(); ++it)
+      {
+      cmListFileBacktrace::const_iterator i = it->begin();
+      if(i != it->end())
+        {
+        cmListFileContext const& lfc = *i;
+        s << " * " << (lfc.Line? "": " in ") << lfc << std::endl;
+        ++i;
+        }
+      }
+    }
+}
+
+//----------------------------------------------------------------------------
 void cmTarget::AddLinkLibrary(cmMakefile& mf,
                               const char *target, const char* lib,
                               LinkLibraryType llt)
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index 0da0f12..24a71ed 100644
--- a/Source/cmTarget.h
+++ b/Source/cmTarget.h
@@ -190,6 +190,12 @@ public:
   void AddLinkLibrary(cmMakefile& mf,
                       const char *target, const char* lib,
                       LinkLibraryType llt);
+  enum TLLSignature {
+    KeywordTLLSignature,
+    PlainTLLSignature
+  };
+  bool PushTLLCommandTrace(TLLSignature signature);
+  void GetTllSignatureTraces(cmOStringStream &s, TLLSignature sig) const;
 
   void MergeLinkLibraries( cmMakefile& mf, const char* selfname,
                            const LinkLibraryVectorType& libs );
@@ -548,6 +554,8 @@ private:
   // directories.
   std::set<cmStdString> SystemIncludeDirectories;
 
+  std::vector<std::pair<TLLSignature, cmListFileBacktrace> > TLLCommands;
+
   /**
    * A list of direct dependencies. Use in conjunction with DependencyMap.
    */
diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx
index c5f490e..0ee9420 100644
--- a/Source/cmTargetLinkLibrariesCommand.cxx
+++ b/Source/cmTargetLinkLibrariesCommand.cxx
@@ -116,7 +116,7 @@ bool cmTargetLinkLibrariesCommand
     {
     if(args[i] == "LINK_INTERFACE_LIBRARIES")
       {
-      this->CurrentProcessingState = ProcessingLinkInterface;
+      this->CurrentProcessingState = ProcessingPlainLinkInterface;
       if(i != 1)
         {
         this->Makefile->IssueMessage(
@@ -127,9 +127,26 @@ bool cmTargetLinkLibrariesCommand
         return true;
         }
       }
+    else if(args[i] == "INTERFACE")
+      {
+      if(i != 1
+          && this->CurrentProcessingState != ProcessingKeywordPrivateInterface
+          && this->CurrentProcessingState != ProcessingKeywordPublicInterface
+          && this->CurrentProcessingState != ProcessingKeywordLinkInterface)
+        {
+        this->Makefile->IssueMessage(
+          cmake::FATAL_ERROR,
+          "The INTERFACE option must appear as the second "
+          "argument, just after the target name."
+          );
+        return true;
+        }
+      this->CurrentProcessingState = ProcessingKeywordLinkInterface;
+      }
     else if(args[i] == "LINK_PUBLIC")
       {
-      if(i != 1 && this->CurrentProcessingState != ProcessingPrivateInterface)
+      if(i != 1
+          && this->CurrentProcessingState != ProcessingPlainPrivateInterface)
         {
         this->Makefile->IssueMessage(
           cmake::FATAL_ERROR,
@@ -138,11 +155,28 @@ bool cmTargetLinkLibrariesCommand
           );
         return true;
         }
-      this->CurrentProcessingState = ProcessingPublicInterface;
+      this->CurrentProcessingState = ProcessingPlainPublicInterface;
+      }
+    else if(args[i] == "PUBLIC")
+      {
+      if(i != 1
+          && this->CurrentProcessingState != ProcessingKeywordPrivateInterface
+          && this->CurrentProcessingState != ProcessingKeywordPublicInterface
+          && this->CurrentProcessingState != ProcessingKeywordLinkInterface)
+        {
+        this->Makefile->IssueMessage(
+          cmake::FATAL_ERROR,
+          "The PUBLIC or PRIVATE option must appear as the second "
+          "argument, just after the target name."
+          );
+        return true;
+        }
+      this->CurrentProcessingState = ProcessingKeywordPublicInterface;
       }
     else if(args[i] == "LINK_PRIVATE")
       {
-      if(i != 1 && this->CurrentProcessingState != ProcessingPublicInterface)
+      if(i != 1
+          && this->CurrentProcessingState != ProcessingPlainPublicInterface)
         {
         this->Makefile->IssueMessage(
           cmake::FATAL_ERROR,
@@ -151,7 +185,23 @@ bool cmTargetLinkLibrariesCommand
           );
         return true;
         }
-      this->CurrentProcessingState = ProcessingPrivateInterface;
+      this->CurrentProcessingState = ProcessingPlainPrivateInterface;
+      }
+    else if(args[i] == "PRIVATE")
+      {
+      if(i != 1
+          && this->CurrentProcessingState != ProcessingKeywordPrivateInterface
+          && this->CurrentProcessingState != ProcessingKeywordPublicInterface
+          && this->CurrentProcessingState != ProcessingKeywordLinkInterface)
+        {
+        this->Makefile->IssueMessage(
+          cmake::FATAL_ERROR,
+          "The PUBLIC or PRIVATE option must appear as the second "
+          "argument, just after the target name."
+          );
+        return true;
+        }
+      this->CurrentProcessingState = ProcessingKeywordPrivateInterface;
       }
     else if(args[i] == "debug")
       {
@@ -184,7 +234,10 @@ bool cmTargetLinkLibrariesCommand
       {
       // The link type was specified by the previous argument.
       haveLLT = false;
-      this->HandleLibrary(args[i].c_str(), llt);
+      if (!this->HandleLibrary(args[i].c_str(), llt))
+        {
+        return false;
+        }
       }
     else
       {
@@ -210,7 +263,10 @@ bool cmTargetLinkLibrariesCommand
           llt = cmTarget::OPTIMIZED;
           }
         }
-      this->HandleLibrary(args[i].c_str(), llt);
+      if (!this->HandleLibrary(args[i].c_str(), llt))
+        {
+        return false;
+        }
       }
     }
 
@@ -257,16 +313,69 @@ cmTargetLinkLibrariesCommand
 }
 
 //----------------------------------------------------------------------------
-void
+bool
 cmTargetLinkLibrariesCommand::HandleLibrary(const char* lib,
                                             cmTarget::LinkLibraryType llt)
 {
+  cmTarget::TLLSignature sig =
+        (this->CurrentProcessingState == ProcessingPlainPrivateInterface
+      || this->CurrentProcessingState == ProcessingPlainPublicInterface
+      || this->CurrentProcessingState == ProcessingKeywordPrivateInterface
+      || this->CurrentProcessingState == ProcessingKeywordPublicInterface
+      || this->CurrentProcessingState == ProcessingKeywordLinkInterface)
+        ? cmTarget::KeywordTLLSignature : cmTarget::PlainTLLSignature;
+  if (!this->Target->PushTLLCommandTrace(sig))
+    {
+    const char *modal = 0;
+    cmake::MessageType messageType = cmake::AUTHOR_WARNING;
+    switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0023))
+      {
+      case cmPolicies::WARN:
+        modal = "should";
+      case cmPolicies::OLD:
+        break;
+      case cmPolicies::REQUIRED_ALWAYS:
+      case cmPolicies::REQUIRED_IF_USED:
+      case cmPolicies::NEW:
+        modal = "must";
+        messageType = cmake::FATAL_ERROR;
+      }
+
+      if(modal)
+        {
+        cmOStringStream e;
+        // If the sig is a keyword form and there is a conflict, the existing
+        // form must be the plain form.
+        const char *existingSig
+                    = (sig == cmTarget::KeywordTLLSignature ? "plain"
+                                                            : "keyword");
+        e << this->Makefile->GetPolicies()
+                              ->GetPolicyWarning(cmPolicies::CMP0023) << "\n"
+            "The " << existingSig << " signature for target_link_libraries "
+            "has already been used with the target \""
+          << this->Target->GetName() << "\".  All uses of "
+             "target_link_libraries with a target " << modal << " be either "
+             "all-keyword or all-plain.\n";
+        this->Target->GetTllSignatureTraces(e,
+                                          sig == cmTarget::KeywordTLLSignature
+                                            ? cmTarget::PlainTLLSignature
+                                            : cmTarget::KeywordTLLSignature);
+        this->Makefile->IssueMessage(messageType, e.str().c_str());
+        if(messageType == cmake::FATAL_ERROR)
+          {
+          return false;
+          }
+        }
+    }
+
   // Handle normal case first.
-  if(this->CurrentProcessingState != ProcessingLinkInterface)
+  if(this->CurrentProcessingState != ProcessingKeywordLinkInterface
+      && this->CurrentProcessingState != ProcessingPlainLinkInterface)
     {
     this->Makefile
       ->AddLinkLibraryForTarget(this->Target->GetName(), lib, llt);
-    if (this->CurrentProcessingState != ProcessingPublicInterface)
+    if (this->CurrentProcessingState != ProcessingKeywordPublicInterface
+        && this->CurrentProcessingState != ProcessingPlainPublicInterface)
       {
       if (this->Target->GetType() == cmTarget::STATIC_LIBRARY)
         {
@@ -275,8 +384,9 @@ cmTargetLinkLibrariesCommand::HandleLibrary(const char* lib,
                   this->Target->GetDebugGeneratorExpressions(lib, llt) +
                   ">").c_str());
         }
-      // Not LINK_INTERFACE_LIBRARIES or LINK_PUBLIC, do not add to interface.
-      return;
+      // Not a 'public' or 'interface' library. Do not add to interface
+      // property.
+      return true;
       }
     }
 
@@ -289,7 +399,7 @@ cmTargetLinkLibrariesCommand::HandleLibrary(const char* lib,
   if (policy22Status != cmPolicies::OLD
       && policy22Status != cmPolicies::WARN)
     {
-    return;
+    return true;
     }
 
   // Get the list of configurations considered to be DEBUG.
@@ -327,4 +437,5 @@ cmTargetLinkLibrariesCommand::HandleLibrary(const char* lib,
         }
       }
     }
+  return true;
 }
diff --git a/Source/cmTargetLinkLibrariesCommand.h b/Source/cmTargetLinkLibrariesCommand.h
index e390343..f2b2543 100644
--- a/Source/cmTargetLinkLibrariesCommand.h
+++ b/Source/cmTargetLinkLibrariesCommand.h
@@ -113,6 +113,16 @@ public:
       "directory of the framework will also be processed as a \"usage "
       "requirement\".  This has the same effect as passing the framework "
       "directory as an include directory."
+      "  target_link_libraries(<target>\n"
+      "                      <PRIVATE|PUBLIC|INTERFACE> <lib> ...\n"
+      "                      [<PRIVATE|PUBLIC|INTERFACE> <lib> ... ] ...])\n"
+      "The PUBLIC, PRIVATE and INTERFACE keywords can be used to specify "
+      "both the link dependencies and the link interface in one command.  "
+      "Libraries and targets following PUBLIC are linked to, and are "
+      "made part of the link interface.  Libraries and targets "
+      "following PRIVATE are linked to, but are not made part of the "
+      "link interface.  Libraries following INTERFACE are appended "
+      "to the link interface and are not used for linking <target>."
       "\n"
       "  target_link_libraries(<target> LINK_INTERFACE_LIBRARIES\n"
       "                        [[debug|optimized|general] <lib>] ...)\n"
@@ -120,7 +130,8 @@ public:
       "to the INTERFACE_LINK_LIBRARIES target property instead of using them "
       "for linking.  If policy CMP0022 is not NEW, then this mode also "
       "appends libraries to the LINK_INTERFACE_LIBRARIES and its "
-      "per-configuration equivalent.  "
+      "per-configuration equivalent.  This signature "
+      "is for compatibility only. Prefer the INTERFACE mode instead.  "
       "Libraries specified as \"debug\" are wrapped in a generator "
       "expression to correspond to debug builds.  If policy CMP0022 is not "
       "NEW, the libraries are also appended to the "
@@ -139,7 +150,9 @@ public:
       "                        [<LINK_PRIVATE|LINK_PUBLIC>\n"
       "                          [[debug|optimized|general] <lib>] ...])\n"
       "The LINK_PUBLIC and LINK_PRIVATE modes can be used to specify both "
-      "the link dependencies and the link interface in one command.  "
+      "the link dependencies and the link interface in one command.  This "
+      "signature is for compatibility only. Prefer the PUBLIC or PRIVATE "
+      "keywords instead.  "
       "Libraries and targets following LINK_PUBLIC are linked to, and are "
       "made part of the INTERFACE_LINK_LIBRARIES.  If policy CMP0022 is not "
       "NEW, they are also made part of the LINK_INTERFACE_LIBRARIES.  "
@@ -185,14 +198,17 @@ private:
   cmTarget* Target;
   enum ProcessingState {
     ProcessingLinkLibraries,
-    ProcessingLinkInterface,
-    ProcessingPublicInterface,
-    ProcessingPrivateInterface
+    ProcessingPlainLinkInterface,
+    ProcessingKeywordLinkInterface,
+    ProcessingPlainPublicInterface,
+    ProcessingKeywordPublicInterface,
+    ProcessingPlainPrivateInterface,
+    ProcessingKeywordPrivateInterface
   };
 
   ProcessingState CurrentProcessingState;
 
-  void HandleLibrary(const char* lib, cmTarget::LinkLibraryType llt);
+  bool HandleLibrary(const char* lib, cmTarget::LinkLibraryType llt);
 };