[cmake] Add LLVM_ENABLE_DEBUG variable which can be used to replace NDEBUG

[hintonda]

Summary

Remove the dependancy on NDEBUG, especially in public headers, by using
a new variable, LLVM_ENABLE_DEBUG, which will make it easier to include llvm
headers in other applications which may define NDEBUG differently.

This change only defines the new variable, but if accepted, the following
changes can be made to headers as needed:

s/#ifndef NDEBUG/#ifdef LLVM_ENABLE_DEBUG/
s/!defined(NDEBUG)/defined(LLVM_ENABLE_DEBUG)/

This is similar to changes originally proposed by @tstellar:
https://reviews.llvm.org/D11833 and https://reviews.llvm.org/D11834

• This patch includes changes to CMakeLists.txt from D38406

which is still under review.

[MatzeB]

Thanks for working on this!

Will we only use LLVM_DEBUG in the public headers or also in the implementation files and private headers?

Apart from NDEBUG there's also the peculiar case of assert()s in the headers; though they are probably less of a problem as they do not change the datastructures.

There is a danger that we will regress given that we are forced to set NDEBUG as well to keep assert() working, so we wouldn't notice if someone incorrectly adds new assert() or #ifdef NDEBUG into the headers. I sometimes wonder if we shouldn't add some verification/linting scripts to LLVM that are then run for each commit by a buildbot machine. In this case for example we could search the public headers for NDEBUG and warn/bail out if we find them. We can have this discussion separate from this patch of course.

CMakeLists.txt

393–402

I think we already have the LLVM_ENABLE_ASSERTIONS flag and should not introduce yet another one. We may or may not use LLVM_ENABLE_DEBUG instead of LLVM_ENABLE_ASSERTIONS for the preprocessor define.

include/llvm/Config/llvm-config.h.cmake

17–18

I think LLVM_ENABLE_DEBUG would be more in line with other existing flags.

[hintonda]

In D38511#887415, @MatzeB wrote:

Will we only use LLVM_DEBUG in the public headers or also in the implementation files and private headers?

I'd suggest outlawing NDEBUG, but initially the problem is usage of NDEBUG in public headers.

Apart from NDEBUG there's also the peculiar case of assert()s in the headers; though they are probably less of a problem as they do not change the datastructures.

We could always add something like LLVM_ASSERT (or llvm_assert), if that becomes an issue.

There is a danger that we will regress given that we are forced to set NDEBUG as well to keep assert() working, so we wouldn't notice if someone incorrectly adds new assert() or #ifdef NDEBUG into the headers. I sometimes wonder if we shouldn't add some verification/linting scripts to LLVM that are then run for each commit by a buildbot machine. In this case for example we could search the public headers for NDEBUG and warn/bail out if we find them. We can have this discussion separate from this patch of course.

Okay, sounds good.

CMakeLists.txt

393–402

This change only adds LLVM_DEBUG.

The other changes you see are from D38406 which hasn't been approved yet, and just clean up setting LLVM_ENABLE_DUMP.

include/llvm/Config/llvm-config.h.cmake

17–18

Will do...

[hintonda]

• s/LLVM_DEBUG/LLVM_ENABLE_DEBUG/

[hintonda]

In D38511#887435, @hintonda wrote:

In D38511#887415, @MatzeB wrote:

Will we only use LLVM_DEBUG in the public headers or also in the implementation files and private headers?

I'd suggest outlawing NDEBUG, but initially the problem is usage of NDEBUG in public headers.

Apart from NDEBUG there's also the peculiar case of assert()s in the headers; though they are probably less of a problem as they do not change the datastructures.

Actually, I'd like to backpeddle a bit. It would not be appropriate to replace all instances of NDEBUG. Also, asserts should not be affected, so any code enabled when NDEBUG is not defined should still be available when asserts are enabled.

So the goal should be to remove only debugging code in headers not required by asserts, if they are enabled.

[hintonda]

Superseded by D38406.

CMakeLists.txt

 endif()
 option(LLVM_ENABLE_CXX1Y "Compile with C++1y enabled." OFF)
 option(LLVM_ENABLE_CXX1Z "Compile with C++1z enabled." OFF)
 option(LLVM_ENABLE_LIBCXX "Use libc++ if available." OFF)
 option(LLVM_ENABLE_LLD "Use lld as C and C++ linker." OFF)
 option(LLVM_ENABLE_PEDANTIC "Compile with pedantic enabled." ON)
 option(LLVM_ENABLE_WERROR "Fail and stop if a warning is triggered." OFF)
 
-option(LLVM_ENABLE_DUMP "Enable dump functions in release builds" OFF)
-
 if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
   option(LLVM_ENABLE_ASSERTIONS "Enable assertions" OFF)
+  option(LLVM_FORCE_ENABLE_DUMP "Enable dump functions in release builds" OFF)
 else()
   option(LLVM_ENABLE_ASSERTIONS "Enable assertions" ON)
+  option(LLVM_FORCE_ENABLE_DUMP "Enable dump functions in release builds" ON)
+  set(LLVM_ENABLE_DEBUG ON)
 endif()
 
-if( LLVM_ENABLE_ASSERTIONS )
+if( LLVM_ENABLE_ASSERTIONS OR LLVM_FORCE_ENABLE_DUMP )
   set(LLVM_ENABLE_DUMP ON)
+  set(LLVM_ENABLE_DEBUG ON)
 endif()
 
 option(LLVM_ENABLE_EXPENSIVE_CHECKS "Enable expensive checks" OFF)
 
 set(LLVM_ABI_BREAKING_CHECKS "WITH_ASSERTS" CACHE STRING
   "Enable abi-breaking checks.  Can be WITH_ASSERTS, FORCE_ON or FORCE_OFF.")
 
 option(LLVM_FORCE_USE_OLD_HOST_TOOLCHAIN

include/llvm/Config/llvm-config.h.cmake

 /*===------- llvm/Config/llvm-config.h - llvm configuration -------*- C -*-===*/
 /*                                                                            */
 /*                     The LLVM Compiler Infrastructure                       */
 /*                                                                            */
 /* This file is distributed under the University of Illinois Open Source      */
 /* License. See LICENSE.TXT for details.                                      */
 /*                                                                            */
 /*===----------------------------------------------------------------------===*/
 
 /* This file enumerates variables from the LLVM configuration so that they
    can be in exported headers and won't override package specific directives.
    This is a C header that can be included in the llvm-c headers. */
 
 #ifndef LLVM_CONFIG_H
 #define LLVM_CONFIG_H
 
+/* Define if CMAKE_BUILD_TYPE is Debug and/or NDEBUG is undefined */
+#cmakedefine LLVM_ENABLE_DEBUG
+
 /* Define if we link Polly to the tools */
 #cmakedefine LINK_POLLY_INTO_TOOLS
 
 /* Target triple LLVM will generate code for by default */
 #cmakedefine LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}"
 
 /* Define if threads enabled */
 #cmakedefine01 LLVM_ENABLE_THREADS