[Lldb-commits] [PATCH] D61956: [CMake] Add first CMake cache files
Alex Langford via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu May 16 11:36:00 PDT 2019
xiaobai added inline comments.
Comment at: lldb/cmake/caches/Apple-lldb-macOS.cmake:18
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
> labath wrote:
> > 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 ?
> I think RelWithDebInfo is an okay default. I personally don't like to set build types in caches because I think it's reasonable to expect the user to specify their build type, but if you mostly use that one build type then it's fine.
I don't think `CMAKE_CXX_COMPILER_ID` is available at the cache processing stage of initialization, so this is probably not something you can do. Maybe you can guard with the condition that the OS is Darwin? Maybe that's not needed, since the cache is Apple-specific anyway.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the lldb-commits