From 42590df9f9fe07ba2aa0bbc19069a6f8cf561cb1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Feb 2022 15:48:22 -0500 Subject: target_link_libraries: Remove likely-broken ancient compatibility check Since commit daa6d2bc04 (ENH: updated handling of debug and optimized target link libraries, 2006-11-29, v2.6.0~2471) the ancient `_LINK_TYPE` compatibility lookup was done using the name of the dependent target for which `target_link_libraries` is called, rather than the name of the library dependency being considered. This code probably does nothing. Remove it. --- Source/cmTargetLinkLibrariesCommand.cxx | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index e15c941..38b1601 100644 --- a/Source/cmTargetLinkLibrariesCommand.cxx +++ b/Source/cmTargetLinkLibrariesCommand.cxx @@ -271,25 +271,10 @@ bool cmTargetLinkLibrariesCommand(std::vector const& args, 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)) { + } else { + if (!tll.HandleLibrary(currentProcessingState, args[i], + GENERAL_LibraryType)) { return false; } } -- cgit v0.12 From 5571a316489ff71e4139e967464a7ad97574fce2 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Feb 2022 16:49:07 -0500 Subject: target_link_libraries: Handle keyword arguments in dedicated code path --- Source/cmTargetLinkLibrariesCommand.cxx | 183 +++++++++++++++++--------------- 1 file changed, 100 insertions(+), 83 deletions(-) diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index 38b1601..1cd8a7e 100644 --- a/Source/cmTargetLinkLibrariesCommand.cxx +++ b/Source/cmTargetLinkLibrariesCommand.cxx @@ -178,93 +178,110 @@ bool cmTargetLinkLibrariesCommand(std::vector const& args, // specification if the keyword is encountered as the first argument. ProcessingState currentProcessingState = ProcessingLinkLibraries; + // Keep this list in sync with the keyword dispatch below. + static std::unordered_set 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); - } - llt = OPTIMIZED_LibraryType; - haveLLT = true; - } else if (args[i] == "general") { - if (haveLLT) { - LinkLibraryTypeSpecifierWarning(mf, llt, GENERAL_LibraryType); + if (keywords.count(args[i])) { + // 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; -- cgit v0.12 From c1e812ad4ff3f175dfc7c0de52b56dbaff9d3d16 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 15 Feb 2022 16:53:15 -0500 Subject: 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 --- Source/cmTargetLinkLibrariesCommand.cxx | 42 +++++++++++++++++++--- .../target_link_libraries/cmp0022/CMakeLists.txt | 10 ++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index 1cd8a7e..94fcdd9 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 #include #include #include #include +#include + #include "cmExecutionStatus.h" #include "cmGeneratorExpression.h" #include "cmGlobalGenerator.h" @@ -178,6 +181,28 @@ bool cmTargetLinkLibrariesCommand(std::vector 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 currentEntry; + auto processCurrentEntry = [&]() -> bool { + if (currentEntry) { + assert(!haveLLT); + if (!tll.HandleLibrary(currentProcessingState, *currentEntry, + GENERAL_LibraryType)) { + return false; + } + currentEntry = cm::nullopt; + } + return true; + }; + auto extendCurrentEntry = [¤tEntry](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 const keywords{ "LINK_INTERFACE_LIBRARIES", @@ -195,6 +220,11 @@ bool cmTargetLinkLibrariesCommand(std::vector 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 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 83103cf..ca6309b 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) -- cgit v0.12