[llvm] r186499 - Tweak the cmake interaction between CMAKE_BUILD_TYPE and LLVM_ENABLE_ASSERTIONS.

[Duncan Sands]

Hi Richard, the attached patch seems much neater.  What do you think of it?

Ciao, Duncan.

On 20/09/13 04:49, Richard Smith wrote:
> On Wed, Jul 17, 2013 at 2:34 AM, Duncan Sands <baldrick at free.fr
> <mailto:baldrick at free.fr>> wrote:
>
>     Author: baldrick
>     Date: Wed Jul 17 04:34:51 2013
>     New Revision: 186499
>
>     URL: http://llvm.org/viewvc/llvm-project?rev=186499&view=rev
>     Log:
>     Tweak the cmake interaction between CMAKE_BUILD_TYPE and LLVM_ENABLE_ASSERTIONS.
>     The issue is that CMAKE_BUILD_TYPE=RelWithDebInfo LLVM_ENABLE_ASSERTIONS=ON was
>     not building with assertions enabled.  (I was unable to find what in the LLVM
>     source tree was adding -DNDEBUG to the build line in this case, so decided that
>     it must be cmake itself that was adding it - this may depend on the cmake
>     version).  The fix treats any mode that is not Debug as being the same as
>     Release for this purpose (previously it was being assumed that cmake would only
>     add -DNDEBUG for Release and not for RelWithDebInfo or MinSizeRel).  If other
>     versions of cmake don't add -DNDEBUG for RelWithDebInfo then that's OK: with
>     this change you just get a useless but harmless -UNDEBUG or -DNDEBUG.
>
>
> I believe this change broke my build setup. Previously a default cmake setup
> (with no CMAKE_BUILD_TYPE set) would build with no debug, no optimization, and
> assertions enabled (a pretty good default setup for development). Now we get no
> assertions in that setup. Was that deliberate?
>
>     Modified:
>          llvm/trunk/CMakeLists.txt
>          llvm/trunk/cmake/modules/HandleLLVMOptions.cmake
>
>     Modified: llvm/trunk/CMakeLists.txt
>     URL:
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/CMakeLists.txt?rev=186499&r1=186498&r2=186499&view=diff
>     ==============================================================================
>     --- llvm/trunk/CMakeLists.txt (original)
>     +++ llvm/trunk/CMakeLists.txt Wed Jul 17 04:34:51 2013
>     @@ -153,7 +153,7 @@ endif()
>       option(LLVM_ENABLE_PEDANTIC "Compile with pedantic enabled." ON)
>       option(LLVM_ENABLE_WERROR "Fail and stop if a warning is triggered." OFF)
>
>     -if( uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE" )
>     +if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
>         option(LLVM_ENABLE_ASSERTIONS "Enable assertions" OFF)
>       else()
>         option(LLVM_ENABLE_ASSERTIONS "Enable assertions" ON)
>
>     Modified: llvm/trunk/cmake/modules/HandleLLVMOptions.cmake
>     URL:
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/HandleLLVMOptions.cmake?rev=186499&r1=186498&r2=186499&view=diff
>     ==============================================================================
>     --- llvm/trunk/cmake/modules/HandleLLVMOptions.cmake (original)
>     +++ llvm/trunk/cmake/modules/HandleLLVMOptions.cmake Wed Jul 17 04:34:51 2013
>     @@ -17,9 +17,9 @@ if( LLVM_ENABLE_ASSERTIONS )
>         if( NOT MSVC )
>           add_definitions( -D_DEBUG )
>         endif()
>     -  # On Release builds cmake automatically defines NDEBUG, so we
>     +  # On non-Debug builds cmake automatically defines NDEBUG, so we
>         # explicitly undefine it:
>     -  if( uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE" )
>     +  if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
>           add_definitions( -UNDEBUG )
>           # Also remove /D NDEBUG to avoid MSVC warnings about conflicting defines.
>           string (REGEX REPLACE "(^| )[/-]D *NDEBUG($| )" " "
>
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: cmake-dbg.diff
Type: text/x-patch
Size: 1462 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131001/99f1e812/attachment.bin>