[CMake] Handle multiple flags in ADDITIONAL_COMPILE_FLAGS properly #112703
Conversation
When multiple space-separated compile flags are specified in an
`ADDITIONAL_COMPILE_FLAGS` cache string, the resulting flags are
enclosed by double quotes because `ADDITIONAL_COMPILE_FLAGS` is a
string (i.e. not a list) and CMake `target_compile_options` treats the
multiple space-separated arguments as single argument containing spaces.
For example, when libcxx is configured with the following multiple
space-separated additional compile flags:
cmake ... "-DLIBCXX_ADDITIONAL_COMPILE_FLAGS=--flag1 --flag2" ...
The resulting compiler command line is as follows:
cc ... "--flag1 --flag2" ...
The above can be problematic for some compilers -- for instance, GCC
treats it as a file path and prints out an error.
This patch, by calling `separate_arguments` on
`ADDITIONAL_COMPILE_FLAGS` to convert it into the standard
semicolon-separated list form, which is properly handled by
`target_compile_options`, ensures that multiple compile flags are
handled as such.
With this change, the resulting compiler command line is as follows:
cc ... --flag1 --flag2 ...
Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
libc++
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-libcxxabi Author: Stephanos Ioannidis (stephanosio) ChangesWhen multiple space-separated compile flags are specified in an For example, when libcxx is configured with the following multiple space-separated additional compile flags: The resulting compiler command line is as follows: The above can be problematic for some compilers -- for instance, GCC treats it as a file path and prints out an error. This patch, by calling With this change, the resulting compiler command line is as follows: Full diff: https://github.com/llvm/llvm-project/pull/112703.diff 5 Files Affected:
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 75c926f5432aea..f9cdeaa0fe84b6 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -460,6 +460,7 @@ set(LIBCXX_LINK_FLAGS "")
set(LIBCXX_LIBRARIES "")
set(LIBCXX_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
"Additional Compile only flags which can be provided in cache")
+separate_arguments(LIBCXX_ADDITIONAL_COMPILE_FLAGS)
set(LIBCXX_ADDITIONAL_LIBRARIES "" CACHE STRING
"Additional libraries libc++ is linked to which can be provided in cache")
@@ -549,7 +550,7 @@ function(cxx_add_basic_build_flags target)
target_compile_definitions(${target} PRIVATE -D_LIBCPP_LINK_RT_LIB)
endif()
endif()
- target_compile_options(${target} PUBLIC "${LIBCXX_ADDITIONAL_COMPILE_FLAGS}")
+ target_compile_options(${target} PUBLIC ${LIBCXX_ADDITIONAL_COMPILE_FLAGS})
endfunction()
# Exception flags =============================================================
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index ac1ee69d5f11c9..ef7473bf1a6cbb 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -224,6 +224,7 @@ set(LIBCXXABI_LINK_FLAGS "")
set(LIBCXXABI_LIBRARIES "")
set(LIBCXXABI_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
"Additional Compile only flags which can be provided in cache")
+separate_arguments(LIBCXXABI_ADDITIONAL_COMPILE_FLAGS)
set(LIBCXXABI_ADDITIONAL_LIBRARIES "" CACHE STRING
"Additional libraries libc++abi is linked to which can be provided in cache")
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index 84fe2784bec5ca..3fc822ffdbcd6b 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -182,7 +182,7 @@ set_target_properties(cxxabi_shared_objects
if (CMAKE_POSITION_INDEPENDENT_CODE OR NOT DEFINED CMAKE_POSITION_INDEPENDENT_CODE)
set_target_properties(cxxabi_shared_objects PROPERTIES POSITION_INDEPENDENT_CODE ON) # must set manually because it's an object library
endif()
-target_compile_options(cxxabi_shared_objects PRIVATE "${LIBCXXABI_ADDITIONAL_COMPILE_FLAGS}")
+target_compile_options(cxxabi_shared_objects PRIVATE ${LIBCXXABI_ADDITIONAL_COMPILE_FLAGS})
add_library(cxxabi_shared SHARED)
set_target_properties(cxxabi_shared
@@ -273,7 +273,7 @@ set_target_properties(cxxabi_static_objects
CXX_STANDARD_REQUIRED OFF # TODO: Make this REQUIRED once we don't need to accommodate the LLVM documentation builders using an ancient CMake
COMPILE_FLAGS "${LIBCXXABI_COMPILE_FLAGS}"
)
-target_compile_options(cxxabi_static_objects PRIVATE "${LIBCXXABI_ADDITIONAL_COMPILE_FLAGS}")
+target_compile_options(cxxabi_static_objects PRIVATE ${LIBCXXABI_ADDITIONAL_COMPILE_FLAGS})
if(LIBCXXABI_HERMETIC_STATIC_LIBRARY)
target_add_compile_flags_if_supported(cxxabi_static_objects PRIVATE -fvisibility=hidden)
diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index b911f482fc26b2..7d57417e122e04 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -164,6 +164,7 @@ set(LIBUNWIND_COMPILE_FLAGS "")
set(LIBUNWIND_LINK_FLAGS "")
set(LIBUNWIND_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
"Additional Compile only flags which can be provided in cache")
+separate_arguments(LIBUNWIND_ADDITIONAL_COMPILE_FLAGS)
set(LIBUNWIND_ADDITIONAL_LIBRARIES "" CACHE STRING
"Additional libraries libunwind is linked to which can be provided in cache")
diff --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index 2e18b109656331..05a266aa1c7451 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -140,7 +140,7 @@ else()
target_compile_options(unwind_shared_objects PRIVATE -fno-rtti)
endif()
target_link_libraries(unwind_shared_objects PRIVATE unwind-headers ${LIBUNWIND_LIBRARIES})
-target_compile_options(unwind_shared_objects PUBLIC "${LIBUNWIND_ADDITIONAL_COMPILE_FLAGS}")
+target_compile_options(unwind_shared_objects PUBLIC ${LIBUNWIND_ADDITIONAL_COMPILE_FLAGS})
target_link_libraries(unwind_shared_objects PUBLIC "${LIBUNWIND_ADDITIONAL_LIBRARIES}")
set_target_properties(unwind_shared_objects
PROPERTIES
@@ -181,7 +181,7 @@ else()
target_compile_options(unwind_static_objects PRIVATE -fno-rtti)
endif()
target_link_libraries(unwind_static_objects PRIVATE unwind-headers ${LIBUNWIND_LIBRARIES})
-target_compile_options(unwind_static_objects PUBLIC "${LIBUNWIND_ADDITIONAL_COMPILE_FLAGS}")
+target_compile_options(unwind_static_objects PUBLIC ${LIBUNWIND_ADDITIONAL_COMPILE_FLAGS})
target_link_libraries(unwind_static_objects PUBLIC "${LIBUNWIND_ADDITIONAL_LIBRARIES}")
set_target_properties(unwind_static_objects
PROPERTIES
|
|
@llvm/pr-subscribers-libunwind Author: Stephanos Ioannidis (stephanosio) ChangesWhen multiple space-separated compile flags are specified in an For example, when libcxx is configured with the following multiple space-separated additional compile flags: The resulting compiler command line is as follows: The above can be problematic for some compilers -- for instance, GCC treats it as a file path and prints out an error. This patch, by calling With this change, the resulting compiler command line is as follows: Full diff: https://github.com/llvm/llvm-project/pull/112703.diff 5 Files Affected:
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 75c926f5432aea..f9cdeaa0fe84b6 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -460,6 +460,7 @@ set(LIBCXX_LINK_FLAGS "")
set(LIBCXX_LIBRARIES "")
set(LIBCXX_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
"Additional Compile only flags which can be provided in cache")
+separate_arguments(LIBCXX_ADDITIONAL_COMPILE_FLAGS)
set(LIBCXX_ADDITIONAL_LIBRARIES "" CACHE STRING
"Additional libraries libc++ is linked to which can be provided in cache")
@@ -549,7 +550,7 @@ function(cxx_add_basic_build_flags target)
target_compile_definitions(${target} PRIVATE -D_LIBCPP_LINK_RT_LIB)
endif()
endif()
- target_compile_options(${target} PUBLIC "${LIBCXX_ADDITIONAL_COMPILE_FLAGS}")
+ target_compile_options(${target} PUBLIC ${LIBCXX_ADDITIONAL_COMPILE_FLAGS})
endfunction()
# Exception flags =============================================================
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index ac1ee69d5f11c9..ef7473bf1a6cbb 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -224,6 +224,7 @@ set(LIBCXXABI_LINK_FLAGS "")
set(LIBCXXABI_LIBRARIES "")
set(LIBCXXABI_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
"Additional Compile only flags which can be provided in cache")
+separate_arguments(LIBCXXABI_ADDITIONAL_COMPILE_FLAGS)
set(LIBCXXABI_ADDITIONAL_LIBRARIES "" CACHE STRING
"Additional libraries libc++abi is linked to which can be provided in cache")
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index 84fe2784bec5ca..3fc822ffdbcd6b 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -182,7 +182,7 @@ set_target_properties(cxxabi_shared_objects
if (CMAKE_POSITION_INDEPENDENT_CODE OR NOT DEFINED CMAKE_POSITION_INDEPENDENT_CODE)
set_target_properties(cxxabi_shared_objects PROPERTIES POSITION_INDEPENDENT_CODE ON) # must set manually because it's an object library
endif()
-target_compile_options(cxxabi_shared_objects PRIVATE "${LIBCXXABI_ADDITIONAL_COMPILE_FLAGS}")
+target_compile_options(cxxabi_shared_objects PRIVATE ${LIBCXXABI_ADDITIONAL_COMPILE_FLAGS})
add_library(cxxabi_shared SHARED)
set_target_properties(cxxabi_shared
@@ -273,7 +273,7 @@ set_target_properties(cxxabi_static_objects
CXX_STANDARD_REQUIRED OFF # TODO: Make this REQUIRED once we don't need to accommodate the LLVM documentation builders using an ancient CMake
COMPILE_FLAGS "${LIBCXXABI_COMPILE_FLAGS}"
)
-target_compile_options(cxxabi_static_objects PRIVATE "${LIBCXXABI_ADDITIONAL_COMPILE_FLAGS}")
+target_compile_options(cxxabi_static_objects PRIVATE ${LIBCXXABI_ADDITIONAL_COMPILE_FLAGS})
if(LIBCXXABI_HERMETIC_STATIC_LIBRARY)
target_add_compile_flags_if_supported(cxxabi_static_objects PRIVATE -fvisibility=hidden)
diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index b911f482fc26b2..7d57417e122e04 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -164,6 +164,7 @@ set(LIBUNWIND_COMPILE_FLAGS "")
set(LIBUNWIND_LINK_FLAGS "")
set(LIBUNWIND_ADDITIONAL_COMPILE_FLAGS "" CACHE STRING
"Additional Compile only flags which can be provided in cache")
+separate_arguments(LIBUNWIND_ADDITIONAL_COMPILE_FLAGS)
set(LIBUNWIND_ADDITIONAL_LIBRARIES "" CACHE STRING
"Additional libraries libunwind is linked to which can be provided in cache")
diff --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index 2e18b109656331..05a266aa1c7451 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -140,7 +140,7 @@ else()
target_compile_options(unwind_shared_objects PRIVATE -fno-rtti)
endif()
target_link_libraries(unwind_shared_objects PRIVATE unwind-headers ${LIBUNWIND_LIBRARIES})
-target_compile_options(unwind_shared_objects PUBLIC "${LIBUNWIND_ADDITIONAL_COMPILE_FLAGS}")
+target_compile_options(unwind_shared_objects PUBLIC ${LIBUNWIND_ADDITIONAL_COMPILE_FLAGS})
target_link_libraries(unwind_shared_objects PUBLIC "${LIBUNWIND_ADDITIONAL_LIBRARIES}")
set_target_properties(unwind_shared_objects
PROPERTIES
@@ -181,7 +181,7 @@ else()
target_compile_options(unwind_static_objects PRIVATE -fno-rtti)
endif()
target_link_libraries(unwind_static_objects PRIVATE unwind-headers ${LIBUNWIND_LIBRARIES})
-target_compile_options(unwind_static_objects PUBLIC "${LIBUNWIND_ADDITIONAL_COMPILE_FLAGS}")
+target_compile_options(unwind_static_objects PUBLIC ${LIBUNWIND_ADDITIONAL_COMPILE_FLAGS})
target_link_libraries(unwind_static_objects PUBLIC "${LIBUNWIND_ADDITIONAL_LIBRARIES}")
set_target_properties(unwind_static_objects
PROPERTIES
|
|
This is really tricky, however I believe the current behavior is correct (and in fact it's the only correct way of handling this). The For example, after your patch, passing The right way to pass such arguments is to use |
When multiple space-separated compile flags are specified in an
ADDITIONAL_COMPILE_FLAGScache string, the resulting flags are enclosed by double quotes becauseADDITIONAL_COMPILE_FLAGSis a string (i.e. not a list) and CMaketarget_compile_optionstreats the multiple space-separated arguments as single argument containing spaces.For example, when libcxx is configured with the following multiple space-separated additional compile flags:
The resulting compiler command line is as follows:
The above can be problematic for some compilers -- for instance, GCC treats it as a file path and prints out an error.
This patch, by calling
separate_argumentsonADDITIONAL_COMPILE_FLAGSto convert it into the standard semicolon-separated list form, which is properly handled bytarget_compile_options, ensures that multiple compile flags are handled as such.With this change, the resulting compiler command line is as follows: