[PATCH] D38406: [dump] Remove NDEBUG from test to enable dump methods [NFC]

Don Hinton via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 6 16:54:26 PST 2017


hintonda added a comment.

In https://reviews.llvm.org/D38406#947470, @MatzeB wrote:

> Let's wait a bit to see if this is fine with people. It would probably be also nice to add the `set(LLVM_ENABLE_DUMP OFF CACHE BOOL "")` line to the varous caches in clang/cmake/caches that build llvm in release build types.


I don't mind changing the default, but the original default was OFF, and only flipped to ON if LLVM_ENABLE_ASSERTIONS was ON.  But that had no affect, since enabling assertions removed -DNDEBUG and added -UNDEBUG, so all the dump definitions were already enabled.  Setting LLVM_ENABLE_DUMP=ON only makes sense for non-debug builds where LLVM_ENABLE_ASSERTIONS=OFF.

So, I think maintaining the default of OFF is the correct choice, but will set it to whatever you want.


https://reviews.llvm.org/D38406





More information about the llvm-commits mailing list