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

Thu May 16 11:10:09 PDT 2019

labath 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 "")
----------------
sgraenitz wrote:
> 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.
Are you sure that `CMAKE_CXX_COMPILER_ID` is available this early in the initialization ?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61956