[Lldb-commits] [PATCH] D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added inline comments.


================
Comment at: CMakeLists.txt:73
+  if (compiler_used)
+    set(LLDB_TEST_COMPILER_USED ${compiler_used})
+  else()
----------------
Note: Using a different variable here to avoid force-overwriting the cached value of `LLDB_TEST_COMPILER`. If we did that, it would appear as if it was set explicitly by the user, when re-running CMake. Doing it this way (and leaving the default empty) allows to distinguish an explicit setting from the default. `LLDB_TEST_COMPILER_IS_DEFAULT` tracks this info but is currently not used.


================
Comment at: lit/CMakeLists.txt:14
 
-if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER)
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER "${LLDB_TEST_C_COMPILER}")
----------------
stella.stamenova wrote:
> Your change is removing the logic to setup this path correctly. This will cause failures in the tests.
That should be fine. It only changed the value of `LLDB_TEST_C/CXX_COMPILER` in the scope of this CMakeLists.txt/directory. The value in the parent CMakeLists.txt as well as the one in the cache remained unchanged.

Thus it is unused since https://reviews.llvm.org/rC347216#change-H2HV4zA8ol05 removed their usage from `lit/lit.site.cfg.py.in` that gets configured below. Before this change the local value was passed to the config file like this:
```
config.cc = "@LLDB_TEST_C_COMPILER@"
config.cxx = "@LLDB_TEST_CXX_COMPILER@"
```

They seem to be inferred via `LLDB_LIT_TOOLS_DIR` now.


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

https://reviews.llvm.org/D56440