[libcxx] [libcxxabi] [libunwind] [CMake] Handle multiple flags in ADDITIONAL_COMPILE_FLAGS properly (PR #112703)

via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 17 05:39:56 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx

@llvm/pr-subscribers-libcxxabi

Author: Stephanos Ioannidis (stephanosio)

<details>
<summary>Changes</summary>

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 ...

---
Full diff: https://github.com/llvm/llvm-project/pull/112703.diff


5 Files Affected:

- (modified) libcxx/CMakeLists.txt (+2-1) 
- (modified) libcxxabi/CMakeLists.txt (+1) 
- (modified) libcxxabi/src/CMakeLists.txt (+2-2) 
- (modified) libunwind/CMakeLists.txt (+1) 
- (modified) libunwind/src/CMakeLists.txt (+2-2) 


``````````diff
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

``````````

</details>


https://github.com/llvm/llvm-project/pull/112703


More information about the cfe-commits mailing list