Merge topic 'tll-genex-concat'

c1e812ad4f target_link_libraries: Improve tolerance of unquoted generator expressions
5571a31648 target_link_libraries: Handle keyword arguments in dedicated code path
42590df9f9 target_link_libraries: Remove likely-broken ancient compatibility check

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

Source/cmTargetLinkLibrariesCommand.cxx

@@ -2,11 +2,14 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmTargetLinkLibrariesCommand.h"
 
+#include <cassert>
 #include <memory>
 #include <sstream>
 #include <unordered_set>
 #include <utility>
 
+#include <cm/optional>
+
 #include "cmExecutionStatus.h"
 #include "cmGeneratorExpression.h"
 #include "cmGlobalGenerator.h"
@@ -178,123 +181,156 @@ bool cmTargetLinkLibrariesCommand(std::vector<std::string> const& args,
   // specification if the keyword is encountered as the first argument.
   ProcessingState currentProcessingState = ProcessingLinkLibraries;
 
+  // Accumulate consectuive non-keyword arguments into one entry in
+  // order to handle unquoted generator expressions containing ';'.
+  cm::optional<std::string> currentEntry;
+  auto processCurrentEntry = [&]() -> bool {
+    if (currentEntry) {
+      assert(!haveLLT);
+      if (!tll.HandleLibrary(currentProcessingState, *currentEntry,
+                             GENERAL_LibraryType)) {
+        return false;
+      }
+      currentEntry = cm::nullopt;
+    }
+    return true;
+  };
+  auto extendCurrentEntry = [&currentEntry](std::string const& arg) {
+    if (currentEntry) {
+      currentEntry = cmStrCat(*currentEntry, ';', arg);
+    } else {
+      currentEntry = arg;
+    }
+  };
+
+  // Keep this list in sync with the keyword dispatch below.
+  static std::unordered_set<std::string> const keywords{
+    "LINK_INTERFACE_LIBRARIES",
+    "INTERFACE",
+    "LINK_PUBLIC",
+    "PUBLIC",
+    "LINK_PRIVATE",
+    "PRIVATE",
+    "debug",
+    "optimized",
+    "general",
+  };
+
   // Add libraries, note that there is an optional prefix
   // of debug and optimized that can be used.
   for (unsigned int i = 1; i < args.size(); ++i) {
-    if (args[i] == "LINK_INTERFACE_LIBRARIES") {
-      currentProcessingState = ProcessingPlainLinkInterface;
-      if (i != 1) {
-        mf.IssueMessage(
-          MessageType::FATAL_ERROR,
-          "The LINK_INTERFACE_LIBRARIES option must appear as the second "
-          "argument, just after the target name.");
-        return true;
-      }
-    } else if (args[i] == "INTERFACE") {
-      if (i != 1 &&
-          currentProcessingState != ProcessingKeywordPrivateInterface &&
-          currentProcessingState != ProcessingKeywordPublicInterface &&
-          currentProcessingState != ProcessingKeywordLinkInterface) {
-        mf.IssueMessage(
-          MessageType::FATAL_ERROR,
-          "The INTERFACE, PUBLIC or PRIVATE option must appear as the second "
-          "argument, just after the target name.");
-        return true;
-      }
-      currentProcessingState = ProcessingKeywordLinkInterface;
-    } else if (args[i] == "LINK_PUBLIC") {
-      if (i != 1 &&
-          currentProcessingState != ProcessingPlainPrivateInterface &&
-          currentProcessingState != ProcessingPlainPublicInterface) {
-        mf.IssueMessage(
-          MessageType::FATAL_ERROR,
-          "The LINK_PUBLIC or LINK_PRIVATE option must appear as the second "
-          "argument, just after the target name.");
-        return true;
-      }
-      currentProcessingState = ProcessingPlainPublicInterface;
-    } else if (args[i] == "PUBLIC") {
-      if (i != 1 &&
-          currentProcessingState != ProcessingKeywordPrivateInterface &&
-          currentProcessingState != ProcessingKeywordPublicInterface &&
-          currentProcessingState != ProcessingKeywordLinkInterface) {
-        mf.IssueMessage(
-          MessageType::FATAL_ERROR,
-          "The INTERFACE, PUBLIC or PRIVATE option must appear as the second "
-          "argument, just after the target name.");
-        return true;
-      }
-      currentProcessingState = ProcessingKeywordPublicInterface;
-    } else if (args[i] == "LINK_PRIVATE") {
-      if (i != 1 && currentProcessingState != ProcessingPlainPublicInterface &&
-          currentProcessingState != ProcessingPlainPrivateInterface) {
-        mf.IssueMessage(
-          MessageType::FATAL_ERROR,
-          "The LINK_PUBLIC or LINK_PRIVATE option must appear as the second "
-          "argument, just after the target name.");
-        return true;
-      }
-      currentProcessingState = ProcessingPlainPrivateInterface;
-    } else if (args[i] == "PRIVATE") {
-      if (i != 1 &&
-          currentProcessingState != ProcessingKeywordPrivateInterface &&
-          currentProcessingState != ProcessingKeywordPublicInterface &&
-          currentProcessingState != ProcessingKeywordLinkInterface) {
-        mf.IssueMessage(
-          MessageType::FATAL_ERROR,
-          "The INTERFACE, PUBLIC or PRIVATE option must appear as the second "
-          "argument, just after the target name.");
-        return true;
-      }
-      currentProcessingState = ProcessingKeywordPrivateInterface;
-    } else if (args[i] == "debug") {
-      if (haveLLT) {
-        LinkLibraryTypeSpecifierWarning(mf, llt, DEBUG_LibraryType);
-      }
-      llt = DEBUG_LibraryType;
-      haveLLT = true;
-    } else if (args[i] == "optimized") {
-      if (haveLLT) {
-        LinkLibraryTypeSpecifierWarning(mf, llt, OPTIMIZED_LibraryType);
+    if (keywords.count(args[i])) {
+      // A keyword argument terminates any preceding accumulated entry.
+      if (!processCurrentEntry()) {
+        return false;
       }
-      llt = OPTIMIZED_LibraryType;
-      haveLLT = true;
-    } else if (args[i] == "general") {
-      if (haveLLT) {
-        LinkLibraryTypeSpecifierWarning(mf, llt, GENERAL_LibraryType);
+
+      // Process this keyword argument.
+      if (args[i] == "LINK_INTERFACE_LIBRARIES") {
+        currentProcessingState = ProcessingPlainLinkInterface;
+        if (i != 1) {
+          mf.IssueMessage(
+            MessageType::FATAL_ERROR,
+            "The LINK_INTERFACE_LIBRARIES option must appear as the "
+            "second argument, just after the target name.");
+          return true;
+        }
+      } else if (args[i] == "INTERFACE") {
+        if (i != 1 &&
+            currentProcessingState != ProcessingKeywordPrivateInterface &&
+            currentProcessingState != ProcessingKeywordPublicInterface &&
+            currentProcessingState != ProcessingKeywordLinkInterface) {
+          mf.IssueMessage(MessageType::FATAL_ERROR,
+                          "The INTERFACE, PUBLIC or PRIVATE option must "
+                          "appear as the second argument, just after the "
+                          "target name.");
+          return true;
+        }
+        currentProcessingState = ProcessingKeywordLinkInterface;
+      } else if (args[i] == "LINK_PUBLIC") {
+        if (i != 1 &&
+            currentProcessingState != ProcessingPlainPrivateInterface &&
+            currentProcessingState != ProcessingPlainPublicInterface) {
+          mf.IssueMessage(
+            MessageType::FATAL_ERROR,
+            "The LINK_PUBLIC or LINK_PRIVATE option must appear as the "
+            "second argument, just after the target name.");
+          return true;
+        }
+        currentProcessingState = ProcessingPlainPublicInterface;
+      } else if (args[i] == "PUBLIC") {
+        if (i != 1 &&
+            currentProcessingState != ProcessingKeywordPrivateInterface &&
+            currentProcessingState != ProcessingKeywordPublicInterface &&
+            currentProcessingState != ProcessingKeywordLinkInterface) {
+          mf.IssueMessage(MessageType::FATAL_ERROR,
+                          "The INTERFACE, PUBLIC or PRIVATE option must "
+                          "appear as the second argument, just after the "
+                          "target name.");
+          return true;
+        }
+        currentProcessingState = ProcessingKeywordPublicInterface;
+      } else if (args[i] == "LINK_PRIVATE") {
+        if (i != 1 &&
+            currentProcessingState != ProcessingPlainPublicInterface &&
+            currentProcessingState != ProcessingPlainPrivateInterface) {
+          mf.IssueMessage(
+            MessageType::FATAL_ERROR,
+            "The LINK_PUBLIC or LINK_PRIVATE option must appear as the "
+            "second argument, just after the target name.");
+          return true;
+        }
+        currentProcessingState = ProcessingPlainPrivateInterface;
+      } else if (args[i] == "PRIVATE") {
+        if (i != 1 &&
+            currentProcessingState != ProcessingKeywordPrivateInterface &&
+            currentProcessingState != ProcessingKeywordPublicInterface &&
+            currentProcessingState != ProcessingKeywordLinkInterface) {
+          mf.IssueMessage(MessageType::FATAL_ERROR,
+                          "The INTERFACE, PUBLIC or PRIVATE option must "
+                          "appear as the second argument, just after the "
+                          "target name.");
+          return true;
+        }
+        currentProcessingState = ProcessingKeywordPrivateInterface;
+      } else if (args[i] == "debug") {
+        if (haveLLT) {
+          LinkLibraryTypeSpecifierWarning(mf, llt, DEBUG_LibraryType);
+        }
+        llt = DEBUG_LibraryType;
+        haveLLT = true;
+      } else if (args[i] == "optimized") {
+        if (haveLLT) {
+          LinkLibraryTypeSpecifierWarning(mf, llt, OPTIMIZED_LibraryType);
+        }
+        llt = OPTIMIZED_LibraryType;
+        haveLLT = true;
+      } else if (args[i] == "general") {
+        if (haveLLT) {
+          LinkLibraryTypeSpecifierWarning(mf, llt, GENERAL_LibraryType);
+        }
+        llt = GENERAL_LibraryType;
+        haveLLT = true;
       }
-      llt = GENERAL_LibraryType;
-      haveLLT = true;
     } else if (haveLLT) {
       // The link type was specified by the previous argument.
       haveLLT = false;
+      assert(!currentEntry);
       if (!tll.HandleLibrary(currentProcessingState, args[i], llt)) {
         return false;
       }
-    } else {
-      // Lookup old-style cache entry if type is unspecified.  So if you
-      // do a target_link_libraries(foo optimized bar) it will stay optimized
-      // and not use the lookup.  As there may be the case where someone has
-      // specified that a library is both debug and optimized.  (this check is
-      // only there for backwards compatibility when mixing projects built
-      // with old versions of CMake and new)
       llt = GENERAL_LibraryType;
-      std::string linkType = cmStrCat(args[0], "_LINK_TYPE");
-      cmValue linkTypeString = mf.GetDefinition(linkType);
-      if (linkTypeString) {
-        if (*linkTypeString == "debug") {
-          llt = DEBUG_LibraryType;
-        }
-        if (*linkTypeString == "optimized") {
-          llt = OPTIMIZED_LibraryType;
-        }
-      }
-      if (!tll.HandleLibrary(currentProcessingState, args[i], llt)) {
-        return false;
-      }
+    } else {
+      // Accumulate this argument in the current entry.
+      extendCurrentEntry(args[i]);
     }
   }
 
+  // Process the last accumulated entry, if any.
+  if (!processCurrentEntry()) {
+    return false;
+  }
+
   // Make sure the last argument was not a library type specifier.
   if (haveLLT) {
     mf.IssueMessage(MessageType::FATAL_ERROR,

Tests/CMakeCommands/target_link_libraries/cmp0022/CMakeLists.txt

@@ -1,3 +1,4 @@
+cmake_policy(SET CMP0028 NEW)
 
 include(GenerateExportHeader)
 set(CMAKE_INCLUDE_CURRENT_DIR ON)
@@ -17,6 +18,15 @@ assert_property(cmp0022ifacelib INTERFACE_LINK_LIBRARIES "")
 add_executable(cmp0022exe cmp0022exe.cpp)
 target_link_libraries(cmp0022exe cmp0022lib)
 
+# Test adding unquoted genex with ';' to LINK_LIBRARIES and INTERFACE_LINK_LIBRARIES.
+target_link_libraries(cmp0022lib
+  PUBLIC $<0:imp::missing1;imp::missing2>
+  PRIVATE $<0:imp::missing3;imp::missing4>
+  INTERFACE $<0:imp::missing5;imp::missing6>
+  )
+assert_property(cmp0022lib INTERFACE_LINK_LIBRARIES "cmp0022ifacelib;$<0:imp::missing1;imp::missing2>;$<0:imp::missing5;imp::missing6>")
+assert_property(cmp0022lib LINK_LIBRARIES "cmp0022ifacelib;$<0:imp::missing1;imp::missing2>;$<0:imp::missing3;imp::missing4>")
+
 add_library(staticlib1 STATIC staticlib1.cpp)
 generate_export_header(staticlib1)
 add_library(staticlib2 STATIC staticlib2.cpp)