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

Stella Stamenova via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Jan 9 11:35:17 PST 2019

stella.stamenova added inline comments.

Comment at: CMakeLists.txt:61
+  # Use LLDB_TEST_COMPILER_IS_DEFAULT to determine whether or not to replace
+  # CMAKE_CFG_INTDIR with LLVM_BUILD_MODE for dotest.
If it's true that we don't need the CMAKE_CFG_INTDIR replacement in the test cmake file, then is this comment still correct?

Comment at: lit/CMakeLists.txt:14
sgraenitz wrote:
> 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.
Have you tested it with a tool set that supports multiple configurations to make sure that is the case? I believe XCode and VS both support multiple configurations.



More information about the lldb-commits mailing list