[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