Page MenuHomePhabricator

Setting up CMake to default to RelWithDebInfo when no build type is specified.
ClosedPublic

Authored by beanz on Feb 2 2015, 2:16 PM.

Details

Summary

Turns out if you don't set CMAKE_BUILD_TYPE the default is an empty string. This results in some of the behaviors of debug builds, but not all of them. For example ENABLE_ASSERTIONS is false.

Diff Detail

Repository
rL LLVM

Event Timeline

beanz updated this revision to Diff 19190.Feb 2 2015, 2:16 PM
beanz retitled this revision from to Setting up CMake to default to RelWithDebInfo when no build type is specified..
beanz updated this object.
beanz edited the test plan for this revision. (Show Details)
beanz added a subscriber: Unknown Object (MLST).
beanz added a reviewer: rnk.Feb 6 2015, 4:59 PM
rnk edited edge metadata.Feb 9 2015, 2:30 PM

I thought Richard actually used this configuration, but I could be wrong. I'd rather default it to Debug. IIRC ninja doesn't support the "empty string" build type and it just forces the build type to Debug. That seems like good prior art.

beanz added a comment.Feb 9 2015, 4:17 PM

I have no objection to making the default Debug rather than RelWithDebInfo. I use RelWithDebInfo primarily because it builds faster, but Debug works too.

The real problem is that the behavior when CMAKE_BUILD_TYPE is the empty string is very inconsistent. The CMAKE_C_FLAGS and CMAKE_CXX_FLAGS variables are empty unless you explicitly set them or the build type, so you actually end up with an unoptimized build with no debug info.

Here's an example compile command from a CMake + Ninja build with no build type set:

/usr/bin/c++   -DGTEST_HAS_RTTI=0 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -std=c++11 -fcolor-diagnostics -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk -Ilib/Support -I/Users/cbieneman/dev/open-source/llvm/lib/Support -Iinclude -I/Users/cbieneman/dev/open-source/llvm/include    -fno-exceptions -fno-rtti -MMD -MT lib/Support/CMakeFiles/LLVMSupport.dir/Allocator.cpp.o -MF lib/Support/CMakeFiles/LLVMSupport.dir/Allocator.cpp.o.d -o lib/Support/CMakeFiles/LLVMSupport.dir/Allocator.cpp.o -c /Users/cbieneman/dev/open-source/llvm/lib/Support/Allocator.cpp
beanz updated this revision to Diff 20345.Feb 19 2015, 2:55 PM
beanz edited edge metadata.

Changed default to Debug based on feedback from rnk.

rnk accepted this revision.Feb 19 2015, 2:57 PM
rnk edited edge metadata.

lgtm

This revision is now accepted and ready to land.Feb 19 2015, 2:57 PM
rnk requested changes to this revision.Feb 19 2015, 2:59 PM
rnk edited edge metadata.

Actually, you should probably conditionalize this on whether the generator is single-target or not. We should have a predicate for that somewhere....

This revision now requires changes to proceed.Feb 19 2015, 2:59 PM
beanz updated this revision to Diff 20346.Feb 19 2015, 3:04 PM
beanz edited edge metadata.

Conditionalized based on CMAKE_CONFIGURATION_TYPES to not set a default for multi-configuration generators.

I don't prefer "Debug" nor "RelWithDebInfo" by default.
For Clang/LLVM users, I suggest "Release".
To develop with our tree, they may set with debug symbols, I think.

beanz added a comment.Feb 19 2015, 3:29 PM

Defaulting to debug makes the behavior almost the same as today sans undefined behavior.

I think that is the right way to go.

rnk accepted this revision.Feb 19 2015, 3:50 PM
rnk edited edge metadata.

lgtm

I agree, the cmake default is closer to debug anyway. We also have a status message.

This revision is now accepted and ready to land.Feb 19 2015, 3:50 PM
This revision was automatically updated to reflect the committed changes.