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(LLDB_TEST_COMPILER)
----------------
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
-if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER)
- string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER
"${LLDB_TEST_C_COMPILER}")
----------------
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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56440/new/
https://reviews.llvm.org/D56440
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
