[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files

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

sgraenitz added inline comments.


================
Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18
+set(CMAKE_BUILD_TYPE RelWithDebInfo)
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
----------------
compnerd wrote:
> sgraenitz wrote:
> > Can / Should we add this? Here?
> This is fine to add assuming that you are building on Darwin since that implies clang and that will work.  I would say that if you really want to be pedantically correct, it would be better to do:
> 
> ```
> if(CMAKE_CXX_COMPILER_ID MATCHES Clang)
>   set(LLVM_ENABLE_MODULES_ON CACHE BOOL "")
> endif()
> ```
> 
> Note that the `MATCHES` is required here to match both `Clang` and `AppleClang`.
That sounds reasonable. I would take your version and put it in the base cache?

The other question was, whether `RelWithDebInfo` is a good default. Personally, I use it far more often than other configurations. (Running the test suite with a debug Clang is just too slow.) Moving to the base cache too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956