target_link_libraries: Improve tolerance of unquoted generator expressions

Prior to commit 1d709ea2f5 (cmGeneratorTarget: Propagate backtraces from
INTERFACE_LINK_LIBRARIES, 2021-12-15, v3.23.0-rc1~228^2), the value of
`INTERFACE_LINK_LIBRARIES` was a single string.  If an unquoted
generator expression passed to `target_link_libraries` contained `;` and
became multiple arguments, they would all accumulate as a single
`;`-separated list in the value of `INTERFACE_LINK_LIBRARIES`.  Since
that commit, each argument to `target_link_libraries` is placed in
`INTERFACE_LINK_LIBRARIES` as a separate entry, as has long been the
case for `LINK_LIBRARIES`. That behavior change broke projects that were
accidentally relying on accumulation in `INTERFACE_LINK_LIBRARIES` to
produce complete generator expressions containing `;`.

Teach `target_link_libraries` to accumulate consecutive non-keyword
arguments into a single entry before placing them in `LINK_LIBRARIES` or
`INTERFACE_LINK_LIBRARIES`.  For example, treat `a b c` as if they were
written as `"a;b;c"`.  This restores the previously accidental support
for unquoted generator expressions in `INTERFACE_LINK_LIBRARIES`, and
also enables it for `LINK_LIBRARIES`.

For now, do not drop the `target_link_libraries` documentation that
recommends quoting generator expressions.  Quoting is still important to
populate `LINK_LIBRARIES` in CMake 3.22 and below, and is also good
practice to keep generator expressions whole.

Fixes: #23203

2 files changed, 48 insertions, 4 deletions

diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx
index 1cd8a7e6ad..94fcdd931c 100644
--- a/Source/cmTargetLinkLibrariesCommand.cxx
+++ b/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,6 +181,28 @@ 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",
@@ -195,6 +220,11 @@ bool cmTargetLinkLibrariesCommand(std::vector<std::string> const& args,
   // of debug and optimized that can be used.
   for (unsigned int i = 1; i < args.size(); ++i) {
     if (keywords.count(args[i])) {
+      // A keyword argument terminates any preceding accumulated entry.
+      if (!processCurrentEntry()) {
+        return false;
+      }
+
       // Process this keyword argument.
       if (args[i] == "LINK_INTERFACE_LIBRARIES") {
         currentProcessingState = ProcessingPlainLinkInterface;
@@ -285,18 +315,22 @@ bool cmTargetLinkLibrariesCommand(std::vector<std::string> const& args,
     } 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;
       }
       llt = GENERAL_LibraryType;
     } else {
-      if (!tll.HandleLibrary(currentProcessingState, args[i],
-                             GENERAL_LibraryType)) {
-        return false;
-      }
+      // 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,
diff --git a/Tests/CMakeCommands/target_link_libraries/cmp0022/CMakeLists.txt b/Tests/CMakeCommands/target_link_libraries/cmp0022/CMakeLists.txt
index 83103cf383..ca6309be69 100644
--- a/Tests/CMakeCommands/target_link_libraries/cmp0022/CMakeLists.txt
+++ b/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)