Functionality Added to CMAKE

[ps-19]

Summary

Added functionality to CMAKE file. Added NONE as type to CMAKE_RELEASE.

[ps-19]

Formatted again using Clang-Format.

[ps-19]

Can any one please specify the issue in building? I am not being able to understand.

[aaron.ballman]

In D122814#3421177, @ps-19 wrote:

Can any one please specify the issue in building? I am not being able to understand.

The Debian failures are unrelated to your patch.

Can you explain what you're trying to accomplish with this patch? I don't think we want a NONE category for the build type as that doesn't make sense to me -- what does it mean to have no build type?

[ps-19]

i have seen some cases in repos where no build variants are found and that creates error in building at some moment. It's good standard practice to include NONE type even we don't need this time, else everything is fine for now, i may close the patch for now.

[aaron.ballman]

In D122814#3422183, @ps-19 wrote:

i have seen some cases in repos where no build variants are found and that creates error in building at some moment. It's good standard practice to include NONE type even we don't need this time, else everything is fine for now, i may close the patch for now.

I don't know enough about CMake best practices to know whether this is or isn't a standard practice (it's not from what I know, but I don't know much in this space). Personally, I think if the user ends up with "none" as the build type, an error is a kindness for them because something's gone weird in the build system. Adding a few more reviewers in case I'm wrong though.

[beanz]

Looping in a few others with CMake experience.

NONE isn't really a CMAKE_BUILD_TYPE in the normal flow (see: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html), and it would have undefined behavior inside LLVM because we key quite a few things off the CMAKE_BUILD_TYPE, so I'm a little concerned about this change.

I'm not sure why you think supporting NONE is a good practice, my opinion is that an undefined build type would be a bad thing to support...

[phosek]

In D122814#3422965, @beanz wrote:

Looping in a few others with CMake experience.

NONE isn't really a CMAKE_BUILD_TYPE in the normal flow (see: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html), and it would have undefined behavior inside LLVM because we key quite a few things off the CMAKE_BUILD_TYPE, so I'm a little concerned about this change.

I'm not sure why you think supporting NONE is a good practice, my opinion is that an undefined build type would be a bad thing to support...

I agree, I haven't yet encountered NONE in CMake projects and it's not clear to me why we should support it in LLVM.

[smeenai]

Requesting changes since there are unaddressed reviewer questions.

[ldionne]

(This should be abandoned if no work is happening on this to clear up everybody's review queues)

llvm/CMakeLists.txt

 to this source directory as the last argument.
 This process created the file `CMakeCache.txt' and the directory `CMakeFiles'.
 Please delete them.")
 endif()
 
 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 "^(NONE|DEBUG|RELEASE|RELWITHDEBINFO|MINSIZEREL)$")
   message(FATAL_ERROR "Invalid value for CMAKE_BUILD_TYPE: ${CMAKE_BUILD_TYPE}")
 endif()
 
 set(LLVM_LIBDIR_SUFFIX "" CACHE STRING "Define suffix of library directory name (32/64)" )
 
 set(LLVM_TOOLS_INSTALL_DIR "${CMAKE_INSTALL_BINDIR}" CACHE STRING
     "Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
 mark_as_advanced(LLVM_TOOLS_INSTALL_DIR)