From 54381b5a81a2ef9e3e1e5508d1a8434413b9aace Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Wed, 18 Sep 2024 01:27:37 -0700 Subject: Linking: extract wrapping linker options to a lambda (NFC) Extract logic to wrap flags in wrapOptions, it will be reused in a follow-up commit. --- Source/cmGeneratorTarget_Options.cxx | 72 +++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/Source/cmGeneratorTarget_Options.cxx b/Source/cmGeneratorTarget_Options.cxx index f77ef72..df22b90 100644 --- a/Source/cmGeneratorTarget_Options.cxx +++ b/Source/cmGeneratorTarget_Options.cxx @@ -111,6 +111,43 @@ std::vector> wrapOptions( return result; } + auto insertWrapped = [&](std::vector& opts) { + if (!wrapperSep.empty()) { + if (concatFlagAndArgs) { + // insert flag elements except last one + for (auto i = wrapperFlag.begin(); i != wrapperFlag.end() - 1; ++i) { + result.emplace_back(*i, bt); + } + // concatenate last flag element and all list values + // in one option + result.emplace_back(wrapperFlag.back() + cmJoin(opts, wrapperSep), bt); + } else { + for (std::string const& i : wrapperFlag) { + result.emplace_back(i, bt); + } + // concatenate all list values in one option + result.emplace_back(cmJoin(opts, wrapperSep), bt); + } + } else { + // prefix each element of list with wrapper + if (concatFlagAndArgs) { + std::transform(opts.begin(), opts.end(), opts.begin(), + [&wrapperFlag](std::string const& o) -> std::string { + return wrapperFlag.back() + o; + }); + } + for (std::string& o : opts) { + for (auto i = wrapperFlag.begin(), + e = concatFlagAndArgs ? wrapperFlag.end() - 1 + : wrapperFlag.end(); + i != e; ++i) { + result.emplace_back(*i, bt); + } + result.emplace_back(std::move(o), bt); + } + } + }; + for (std::vector::size_type index = 0; index < options.size(); index++) { if (cmHasLiteralPrefix(options[index], "LINKER:")) { @@ -154,40 +191,7 @@ std::vector> wrapOptions( continue; } - if (!wrapperSep.empty()) { - if (concatFlagAndArgs) { - // insert flag elements except last one - for (auto i = wrapperFlag.begin(); i != wrapperFlag.end() - 1; ++i) { - result.emplace_back(*i, bt); - } - // concatenate last flag element and all list values - // in one option - result.emplace_back(wrapperFlag.back() + cmJoin(opts, wrapperSep), bt); - } else { - for (std::string const& i : wrapperFlag) { - result.emplace_back(i, bt); - } - // concatenate all list values in one option - result.emplace_back(cmJoin(opts, wrapperSep), bt); - } - } else { - // prefix each element of list with wrapper - if (concatFlagAndArgs) { - std::transform(opts.begin(), opts.end(), opts.begin(), - [&wrapperFlag](std::string const& o) -> std::string { - return wrapperFlag.back() + o; - }); - } - for (std::string& o : opts) { - for (auto i = wrapperFlag.begin(), - e = concatFlagAndArgs ? wrapperFlag.end() - 1 - : wrapperFlag.end(); - i != e; ++i) { - result.emplace_back(*i, bt); - } - result.emplace_back(std::move(o), bt); - } - } + insertWrapped(opts); } return result; } -- cgit v0.12 From 4185dfbe1b903f8290617cdfe14acd0881405890 Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Wed, 18 Sep 2024 01:48:59 -0700 Subject: Tests/LINK_OPTIONS: extract common code in test (NFC) --- .../target_link_options/LINKER_expansion.cmake | 30 ++++++++++------------ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion.cmake index f86d19f..8be6cef 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion.cmake @@ -14,27 +14,23 @@ add_executable(dump dump.c) string(REPLACE "${CMAKE_START_TEMP_FILE}" "" CMAKE_C_CREATE_SHARED_LIBRARY "${CMAKE_C_CREATE_SHARED_LIBRARY}") string(REPLACE "${CMAKE_END_TEMP_FILE}" "" CMAKE_C_CREATE_SHARED_LIBRARY "${CMAKE_C_CREATE_SHARED_LIBRARY}") +function (add_test_library target_name) + add_library(${target_name} SHARED LinkOptionsLib.c) -# Use LINKER alone -add_library(linker SHARED LinkOptionsLib.c) -target_link_options(linker PRIVATE "LINKER:-foo,bar") + # use LAUNCH facility to dump linker command + set_property(TARGET ${target_name} PROPERTY RULE_LAUNCH_LINK "\"${DUMP_EXE}\"") -# use LAUNCH facility to dump linker command -set_property(TARGET linker PROPERTY RULE_LAUNCH_LINK "\"${DUMP_EXE}\"") - -add_dependencies (linker dump) + add_dependencies(${target_name} dump) +endfunction() +# Use LINKER alone +add_test_library(linker) +target_link_options(linker PRIVATE "LINKER:-foo,bar") # Use LINKER with SHELL -add_library(linker_shell SHARED LinkOptionsLib.c) +add_test_library(linker_shell) target_link_options(linker_shell PRIVATE "LINKER:SHELL:-foo bar") -# use LAUNCH facility to dump linker command -set_property(TARGET linker_shell PROPERTY RULE_LAUNCH_LINK "\"${DUMP_EXE}\"") - -add_dependencies (linker_shell dump) - - # generate reference for LINKER flag if (CMAKE_C_LINKER_WRAPPER_FLAG) set(linker_flag ${CMAKE_C_LINKER_WRAPPER_FLAG}) @@ -46,9 +42,11 @@ if (CMAKE_C_LINKER_WRAPPER_FLAG) endif() list (JOIN linker_flag " " linker_flag) if (CMAKE_C_LINKER_WRAPPER_FLAG_SEP) - string (APPEND linker_flag "${linker_space}" "-foo${CMAKE_C_LINKER_WRAPPER_FLAG_SEP}bar") + set(linker_sep "${CMAKE_C_LINKER_WRAPPER_FLAG_SEP}") + string (APPEND linker_flag "${linker_space}" "-foo${linker_sep}bar") else() - set (linker_flag "${linker_flag}${linker_space}-foo ${linker_flag}${linker_space}bar") + set(linker_prefix "${linker_flag}${linker_space}") + set (linker_flag "${linker_prefix}-foo ${linker_prefix}bar") endif() else() set(linker_flag "-foo bar") -- cgit v0.12 From e3895f4a8bcce3d9caae7d0e5559e6f057725fad Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Tue, 17 Sep 2024 01:29:23 -0700 Subject: Linking: Preserve nested LINKER: prefixes as written Previously LINKER:,-Xlinker and -Wl, options nested inside LINKER: prefixes would be transformed to separate prefixed options. This is confusing and undocumented behavior, instead preserve these as written. Fixes: #26298 --- Source/cmGeneratorTarget_Options.cxx | 46 +++++++++++++++------- .../LINKER_expansion-LINKER-check.cmake | 1 + .../LINKER_expansion-LINKER_NESTED-check.cmake | 4 ++ ...INKER_expansion-LINKER_NESTED_SHELL-check.cmake | 4 ++ .../LINKER_expansion-LINKER_SHELL-check.cmake | 1 + .../LINKER_expansion-validation.cmake | 8 ++-- .../target_link_options/LINKER_expansion.cmake | 18 +++++++++ .../target_link_options/RunCMakeTest.cmake | 2 + 8 files changed, 65 insertions(+), 19 deletions(-) create mode 100644 Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED-check.cmake create mode 100644 Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED_SHELL-check.cmake diff --git a/Source/cmGeneratorTarget_Options.cxx b/Source/cmGeneratorTarget_Options.cxx index df22b90..d8b3eb3 100644 --- a/Source/cmGeneratorTarget_Options.cxx +++ b/Source/cmGeneratorTarget_Options.cxx @@ -7,6 +7,7 @@ #include "cmConfigure.h" #include +#include #include #include #include @@ -91,10 +92,16 @@ void processOptions(cmGeneratorTarget const* tgt, } } +enum class NestedLinkerFlags +{ + PreserveAsSpelled, + Normalize, +}; + std::vector> wrapOptions( std::vector& options, const cmListFileBacktrace& bt, const std::vector& wrapperFlag, const std::string& wrapperSep, - bool concatFlagAndArgs) + bool concatFlagAndArgs, NestedLinkerFlags nestedLinkerFlags) { std::vector> result; @@ -148,6 +155,11 @@ std::vector> wrapOptions( } }; + if (nestedLinkerFlags == NestedLinkerFlags::PreserveAsSpelled) { + insertWrapped(options); + return result; + } + for (std::vector::size_type index = 0; index < options.size(); index++) { if (cmHasLiteralPrefix(options[index], "LINKER:")) { @@ -488,8 +500,9 @@ std::vector> cmGeneratorTarget::GetLinkOptions( // host link options must be wrapped std::vector options; cmSystemTools::ParseUnixCommandLine(it->Value.c_str(), options); - auto hostOptions = wrapOptions(options, it->Backtrace, wrapperFlag, - wrapperSep, concatFlagAndArgs); + auto hostOptions = + wrapOptions(options, it->Backtrace, wrapperFlag, wrapperSep, + concatFlagAndArgs, NestedLinkerFlags::Normalize); it = result.erase(it); // some compilers (like gcc 4.8 or Intel 19.0 or XLC 16) do not respect // C++11 standard: 'std::vector::insert()' do not returns an iterator, @@ -538,12 +551,11 @@ std::vector>& cmGeneratorTarget::ResolveLinkerWrapper( const std::string SHELL{ "SHELL:" }; const std::string LINKER_SHELL = LINKER + SHELL; - std::vector>::iterator entry; - while ((entry = std::find_if(result.begin(), result.end(), - [&LINKER](BT const& item) -> bool { - return item.Value.compare(0, LINKER.length(), - LINKER) == 0; - })) != result.end()) { + for (auto entry = result.begin(); entry != result.end(); ++entry) { + if (entry->Value.compare(0, LINKER.length(), LINKER) != 0) { + continue; + } + std::string value = std::move(entry->Value); cmListFileBacktrace bt = std::move(entry->Backtrace); entry = result.erase(entry); @@ -573,15 +585,19 @@ std::vector>& cmGeneratorTarget::ResolveLinkerWrapper( return result; } - std::vector> options = wrapOptions( - linkerOptions, bt, wrapperFlag, wrapperSep, concatFlagAndArgs); + // Very old versions of the C++ standard library return void for insert, so + // can't use it to get the new iterator + const auto index = entry - result.begin(); + std::vector> options = + wrapOptions(linkerOptions, bt, wrapperFlag, wrapperSep, + concatFlagAndArgs, NestedLinkerFlags::PreserveAsSpelled); if (joinItems) { - result.insert(entry, - cmJoin(cmRange( - options.cbegin(), options.cend()), - " "_s)); + result.insert( + entry, cmJoin(cmMakeRange(options.begin(), options.end()), " "_s)); + entry = std::next(result.begin(), index); } else { result.insert(entry, options.begin(), options.end()); + entry = std::next(result.begin(), index + options.size() - 1); } } return result; diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER-check.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER-check.cmake index d0ef8de..99f5aa3 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER-check.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER-check.cmake @@ -1,2 +1,3 @@ +set(reference_file "LINKER.txt") include ("${CMAKE_CURRENT_LIST_DIR}/LINKER_expansion-validation.cmake") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED-check.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED-check.cmake new file mode 100644 index 0000000..522fe86 --- /dev/null +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED-check.cmake @@ -0,0 +1,4 @@ + +set(reference_file "LINKER_NESTED.txt") +set(linker_prefix_expected YES) +include ("${CMAKE_CURRENT_LIST_DIR}/LINKER_expansion-validation.cmake") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED_SHELL-check.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED_SHELL-check.cmake new file mode 100644 index 0000000..917455c --- /dev/null +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED_SHELL-check.cmake @@ -0,0 +1,4 @@ + +set(reference_file "LINKER_NESTED_SHELL.txt") +set(linker_prefix_expected YES) +include ("${CMAKE_CURRENT_LIST_DIR}/LINKER_expansion-validation.cmake") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_SHELL-check.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_SHELL-check.cmake index d0ef8de..99f5aa3 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_SHELL-check.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_SHELL-check.cmake @@ -1,2 +1,3 @@ +set(reference_file "LINKER.txt") include ("${CMAKE_CURRENT_LIST_DIR}/LINKER_expansion-validation.cmake") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-validation.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-validation.cmake index 1af8f13..27adde6 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion-validation.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-validation.cmake @@ -1,14 +1,14 @@ -if (actual_stdout MATCHES "LINKER:") +if (actual_stdout MATCHES "LINKER:" AND NOT linker_prefix_expected) set (RunCMake_TEST_FAILED "LINKER: prefix was not expanded.") return() endif() -if (NOT EXISTS "${RunCMake_TEST_BINARY_DIR}/LINKER.txt") - set (RunCMake_TEST_FAILED "${RunCMake_TEST_BINARY_DIR}/LINKER.txt: Reference file not found.") +if (NOT EXISTS "${RunCMake_TEST_BINARY_DIR}/${reference_file}") + set (RunCMake_TEST_FAILED "${RunCMake_TEST_BINARY_DIR}/${reference_file}: Reference file not found.") return() endif() -file(READ "${RunCMake_TEST_BINARY_DIR}/LINKER.txt" linker_flag) +file(READ "${RunCMake_TEST_BINARY_DIR}/${reference_file}" linker_flag) if (NOT actual_stdout MATCHES "${linker_flag}") set (RunCMake_TEST_FAILED "LINKER: was not expanded correctly.") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion.cmake index 8be6cef..2219218 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion.cmake @@ -31,6 +31,14 @@ target_link_options(linker PRIVATE "LINKER:-foo,bar") add_test_library(linker_shell) target_link_options(linker_shell PRIVATE "LINKER:SHELL:-foo bar") +# Nested LINKER: prefixes should be preserved as written, only the outermost LINKER: prefix removed +add_test_library(linker_nested) +target_link_options(linker_nested PRIVATE "LINKER:LINKER:-foo,-Xlinker=bar,-Xlinker,--baz,-Wl,/qux") + +# Same with LINKER:SHELL: +add_test_library(linker_nested_shell SHARED LinkOptionsLib.c) +target_link_options(linker_nested_shell PRIVATE "LINKER:SHELL:LINKER:-foo -Xlinker=bar -Xlinker --baz -Wl,/qux") + # generate reference for LINKER flag if (CMAKE_C_LINKER_WRAPPER_FLAG) set(linker_flag ${CMAKE_C_LINKER_WRAPPER_FLAG}) @@ -43,12 +51,22 @@ if (CMAKE_C_LINKER_WRAPPER_FLAG) list (JOIN linker_flag " " linker_flag) if (CMAKE_C_LINKER_WRAPPER_FLAG_SEP) set(linker_sep "${CMAKE_C_LINKER_WRAPPER_FLAG_SEP}") + + set(linker_flag_nested "${linker_flag}${linker_space}LINKER:-foo${linker_sep}-Xlinker=bar${linker_sep}-Xlinker${linker_sep}--baz${linker_sep}-Wl${linker_sep}/qux") + set(linker_flag_nested_shell "${linker_flag}${linker_space}LINKER:-foo${linker_sep}-Xlinker=bar${linker_sep}-Xlinker${linker_sep}--baz${linker_sep}-Wl,/qux") string (APPEND linker_flag "${linker_space}" "-foo${linker_sep}bar") else() set(linker_prefix "${linker_flag}${linker_space}") + + set(linker_flag_nested "${linker_prefix}LINKER:-foo ${linker_prefix}-Xlinker=bar ${linker_prefix}-Xlinker ${linker_prefix}--baz ${linker_prefix}-Wl ${linker_prefix}/qux") + set(linker_flag_nested_shell "${linker_prefix}LINKER:-foo ${linker_prefix}-Xlinker=bar ${linker_prefix}-Xlinker ${linker_prefix}--baz ${linker_prefix}-Wl,/qux") set (linker_flag "${linker_prefix}-foo ${linker_prefix}bar") endif() else() + set(linker_flag_nested "LINKER:-foo -Xlinker=bar -Xlinker --baz -Wl /qux") + set(linker_flag_nested_shell "LINKER:-foo -Xlinker=bar -Xlinker --baz -Wl,/qux") set(linker_flag "-foo bar") endif() file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/LINKER.txt" "${linker_flag}") +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/LINKER_NESTED.txt" "${linker_flag_nested}") +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/LINKER_NESTED_SHELL.txt" "${linker_flag_nested_shell}") diff --git a/Tests/RunCMake/target_link_options/RunCMakeTest.cmake b/Tests/RunCMake/target_link_options/RunCMakeTest.cmake index ff0c5a8..5562c9e 100644 --- a/Tests/RunCMake/target_link_options/RunCMakeTest.cmake +++ b/Tests/RunCMake/target_link_options/RunCMakeTest.cmake @@ -76,6 +76,8 @@ if(RunCMake_GENERATOR MATCHES "(Ninja|Makefile)") run_cmake_target(LINKER_expansion LINKER linker) run_cmake_target(LINKER_expansion LINKER_SHELL linker_shell) + run_cmake_target(LINKER_expansion LINKER_NESTED linker_nested) + run_cmake_target(LINKER_expansion LINKER_NESTED_SHELL linker_nested_shell) endif() run_cmake(empty_keyword_args) -- cgit v0.12