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

[Tobias Hieta via Phabricator via llvm-commits]

thieta added a comment.

In D124153#3476598 <https://reviews.llvm.org/D124153#3476598>, @jhenderson wrote:

> I rarely, if ever, build the whole of LLVM on my Linux VM, because I don't need to, as I tend to work on only a small subset of the LLVM tools. This means that I rarely run into linker issues due to exhausted memory. I don't believe I ever bothered changing my linker default. Whilst my usage may not be the same as others, I want to raise it to highlight that the configuration isn't //always// broken - it mostly works just fine if you don't try and build everything at once.

Yeah I don't doubt that there are many usages or other options to set where it will work, for experienced developers I don't see this as a problem.

But the point of this change would be to help users that haven't worked with LLVM before and just following a guide to build it - and with the default settings as they are now, it's more likely than not that they end up with a broken build because the memory runs out and it's not obvious unless you have a lot of experience.

Reading back over the comments above it seems like there is:

- Everyone agrees we can do better with documentation. Which is why I have a second review for some documentation updates here: https://reviews.llvm.org/D124367
- Pretty strong support for switching the default build_type to Release, but there is also some pretty strong pushback on this. Where the main disagreement is that it will just create a different problem where we put people that want a Debug build in a similar bad situation.
- Pretty strong support for requiring a CMAKE_BUILD_TYPE set - and error out if it's not set. The main push back on this one is that it would make our build "pretty different" from other CMake based projects.
- Mixed support for warning / erroring out when using DEBUG + BFD linker.

With this I suggest we should go ahead and:

- Improve documentation - Please help out in the review posted above.
- Make omitting CMAKE_BUILD_TYPE an error and link to the improved documentation.

Personally I still feel like a warning/error for Debug + bfd is still a good idea, but I think it's less important if we decide to make omitting CMAKE_BUILD_TYPE an error and we can test that first for a release and see how / if it changes the number of people complaining about the problem.

Any big disagreement on this plan?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124153