[PATCH] D124153: [CMake] Change default CMAKE_BUILD_TYPE to Release

Thu Apr 21 04:12:18 PDT 2022

thieta added a comment.

In D124153#3464303 <https://reviews.llvm.org/D124153#3464303>, @hans wrote:

> In D124153#3464058 <https://reviews.llvm.org/D124153#3464058>, @fhahn wrote:
>
>> I think if `Release` is the new default we should also enable assertions by default, as this should be closer to the Debug configuration and at least ensure that `-debug` works.
>
> I think this is a good point.
>
> However I don't think we want to change the behaviour for "cmake -DCMAKE_BUILD_TYPE=Release ..." which today has asserts disabled, and which I presume people use for their release builds.
>
> Enabling asserts when the user doesn't set CMAKE_BUILD_TYPE explicitly sounds good to me though.

I see two options:

- We can flip the default for `LLVM_ENABLE_ASSERTIONS` to `ON` instead of `OFF` - this is easy and uncomplicated
- We can flip it to ON if you don't specify CMAKE_BUILD_TYPE and OFF if you pass Release and ON when passing debug. But this seems a bit ... convoluted? Is that really what end users expect?

I think I prefer to set it to ON by default and then have people specify it if they want to change it to OFF.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124153