[Lldb-commits] [PATCH] D56400: [CMake] Some cleanup around test preparations

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova requested changes to this revision.
stella.stamenova added inline comments.
This revision now requires changes to proceed.


================
Comment at: CMakeLists.txt:49
   set(LLDB_DEFAULT_TEST_FILECHECK "${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/FileCheck${CMAKE_EXECUTABLE_SUFFIX}")
-
-  if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER AND TARGET clang)
-    set(LLDB_DEFAULT_TEST_C_COMPILER "${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
-  else()
-    set(LLDB_DEFAULT_TEST_C_COMPILER "")
-  endif()
-
-  if (NOT LLDB_TEST_USE_CUSTOM_CXX_COMPILER AND TARGET clang)
-    set(LLDB_DEFAULT_TEST_CXX_COMPILER "${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang++${CMAKE_EXECUTABLE_SUFFIX}")
-  else()
-    set(LLDB_DEFAULT_TEST_CXX_COMPILER "")
+  if(TARGET clang)
+    set(LLDB_DEFAULT_TEST_COMPILER "${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
----------------
Nit: it looks like your if statements lack a space before the parenthesis, while all the existing ones have a space. Please be consistent with the existing convention.


================
Comment at: CMakeLists.txt:54
-
-  if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER AND TARGET clang)
-    set(LLDB_DEFAULT_TEST_C_COMPILER "${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
----------------
You are making a significant change to the compiler selection logic in your second change. I think it would be better to not make any changes here in order to have all of them in a single change.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56400/new/

https://reviews.llvm.org/D56400