[PATCH] D12174: [CMake] Fix building unit tests on Darwin
Filipe Cabecinhas via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 20 10:37:31 PDT 2015
filcab added a comment.
It seems you left some conditions on the `if` that shouldn't be there. At least for now (when OS X is the only OS where we run unit tests).
================
Comment at: cmake/config-ix.cmake:245
@@ +244,3 @@
+ # This is only called in constructing cflags for tests, so we assume OS X.
+ set(${out_var} -arch ${arch} PARENT_SCOPE)
+ endif()
----------------
Should we add the `DARWIN_osx_CFLAGS` here too, for now? That would minimize the number of places where you have to add Apple-specific conditions.
Eventually we might want to also run the tests for iOS, but for now, if you're on `APPLE`, then you know you're compiling for OS X. Might as well just add all the flags now.
================
Comment at: lib/asan/tests/CMakeLists.txt:120
@@ +119,3 @@
+ if(APPLE)
+ set(TARGET_CFLAGS ${DARWIN_osx_CFLAGS} -arch ${arch})
+ endif()
----------------
Didn't you already add `-arch ${arch}` in `get_target_flags_for_arch`?
================
Comment at: test/asan/CMakeLists.txt:45
@@ -39,3 +44,3 @@
set(ASAN_TEST_TARGET_CFLAGS ${COMPILER_RT_TEST_COMPILER_CFLAGS})
- else()
+ elseif(NOT APPLE)
get_target_flags_for_arch(${arch} ASAN_TEST_TARGET_CFLAGS)
----------------
Why the `NOT APPLE`? Shouldn't you do the same for `APPLE` too?
================
Comment at: test/sanitizer_common/CMakeLists.txt:34
@@ -28,3 +33,3 @@
${COMPILER_RT_TEST_COMPILER_CFLAGS})
- else()
+ elseif(NOT APPLE)
get_target_flags_for_arch(${arch} SANITIZER_COMMON_TEST_TARGET_CFLAGS)
----------------
Same as `test/asan/CMakeLists.txt`.
Repository:
rL LLVM
http://reviews.llvm.org/D12174
More information about the llvm-commits
mailing list