Allow CMAKE_BUILD_TYPE=None

[olesalscheider]

Summary

None is a valid build type for CMake and indicates that the CMAKE_C{,XX}_FLAGS should be used.
This patch is written by Bernd Steinhauser.

[beanz]

Either I'm misunderstanding you, or you're misunderstanding how CMake works.

CMAKE_${LANG}_FLAGS is *always* used, regardless of the build type. CMAKE_${LANG}_FLAGS_${CMAKE_BUILD_TYPE} is used only when the build type is specified.

Generally speaking the CMAKE_${LANG}_FLAGS_${CMAKE_BUILD_TYPE} flags should only relate to optimization level, debug information and asserts.

What are you trying to accomplish? I don't think this patch is the right way to go about it.

[olesalscheider]

Exherbo is a source based distribution and we allow our users to specify the compile flags. Our package manager then passes the flags in CMAKE_${LANG}_FLAGS and sets the build type to None.
The reason is that only the flags from CMAKE_${LANG}_FLAGS should be used.

[beanz]

According to CMake's documentation (https://cmake.org/cmake/help/v3.0/variable/CMAKE_BUILD_TYPE.html), NONE is not one of the intended supported build types. I don't believe LLVM should be supporting this.

If you want to support the workflow you describe you can do it by setting CMAKE_BUILD_TYPE=Release, then setting CMAKE_${LANG}_FLAGS_RELEASE="". That will have the same end result.

[olesalscheider]

https://cmake.org/Wiki/CMake_Useful_Variables#Compilers_and_Tools mentions it, though.
And the source code does so as well (https://github.com/Kitware/CMake/blob/master/Modules/CMakeCInformation.cmake#L125) and even mentions that "none" is the default.

I'd therefore argue that the documentation is misleading. But it would also be wrong to add "NONE" in the current wording because it describes only the additional CMAKE_${LANG}_FLAGS_${CMAKE_BUILD_TYPE} flags.

[beanz]

The documentation you called out is CMake 2.8.x. The documentation I referenced is CMake 3.x. I interpret the change in wording between the two as intentional. Maybe I'm reading too much into it, but the LLVM build system has moved forward assuming that as intention.

The line you're trying to modify was added explicitly because we make decisions in the build system based on build type, and not having a valid build type makes the build behave oddly. I don't want to support that, and I don't think we should.

For example, if you grep LLVM for "uppercase_CMAKE_BUILD_TYPE", you'll see that we actually modify compiler and linker flags based on the build type by comparing it against known supported values.

Your argument that None is the default value in the sources doesn't hold much weight with me either. Given the most recent public documentation doesn't list None as an intended type the fact that it is defaulted to None in the sources seems to me like an artifact of history. Admittedly it is an artifact that is surely quite difficult to change.

CMakeLists.txt

 string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
 
 if (CMAKE_BUILD_TYPE AND
-    NOT uppercase_CMAKE_BUILD_TYPE MATCHES "^(DEBUG|RELEASE|RELWITHDEBINFO|MINSIZEREL)$")
+    NOT uppercase_CMAKE_BUILD_TYPE MATCHES "^(DEBUG|NONE|RELEASE|RELWITHDEBINFO|MINSIZEREL)$")
   message(FATAL_ERROR "Invalid value for CMAKE_BUILD_TYPE: ${CMAKE_BUILD_TYPE}")
 endif()