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

Justin Bogner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 5 15:05:08 PST 2017


bogner added a comment.

I don't really think we want to be messing with the CMAKE_CXX_FLAGS_{BUILD_TYPE} variables for this. All of this regex-replace stuff is very complicated and seems fragile.

A couple of things need to happen:

1. LLVM_ENABLE_DUMP definitely needs to be defined in llvm-config.h, or we won't be able to safely use it in headers. Headers that use it should include llvm-config.h

2. While we probably don't want to set a cache variable's value based on the value of assertions, if the user explicitly specifies it we probably do want to trust them. I think having LLVM_ENABLE_DUMP be computed / non-cached and basing it off of LLVM_ENABLE_ASSERTIONS and LLVM_FORCE_ENABLE_DUMP is probably the clearest thing to do here, though that does hit the problem that people already having LLVM_ENABLE_DUMP in their caches. Maybe it would work to set the non-cached version based off of asserts iff the cached version is unset?




https://reviews.llvm.org/D38406





More information about the llvm-commits mailing list