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

[Don Hinton via Phabricator via llvm-commits]

hintonda added inline comments.


================
Comment at: CMakeLists.txt:390-393
+  option(LLVM_ENABLE_DUMP "Enable dump functions in release builds" OFF)
 else()
   option(LLVM_ENABLE_ASSERTIONS "Enable assertions" ON)
+  option(LLVM_ENABLE_DUMP "Enable dump functions in release builds" ON)
----------------
MatzeB wrote:
> MatzeB wrote:
> > Dynamically switching defaults in cmake is usually a bad idea. As that will only have an effect the first time cmake runs, AFAIK in subsequent cmake runs changing the default has no effect anymore as there is already a value in the cmake cache which will be unexpected by the user.
> > 
> > (i.e. `cmake -DCMAKE_BUILD_TYPE=DEBUG ; cmake -DCMAKE_BUILD_TYPE=RELEASE` will give you a different value for LLVM_ENABLE_DUMP than `cmake -DCMAKE_BUILD_TYPE=RELEASE` assuming an empty build dir)
> (Yes we already screwed this up with ENABLE_ASSERTIONS IMO, but no reason to screw it up with ENABLE_DUMP too IMO)
This now works for your example.  If we want to be able to pass LLVM_ENABLE_DUMP independently, we can do that with the additional LLVM_FORCE_ENABLE_DUMP, but it should still work that same as ASSERTIONS.


https://reviews.llvm.org/D38406