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

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 4 19:12:43 PST 2017


MatzeB 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:
> 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)


https://reviews.llvm.org/D38406





More information about the llvm-commits mailing list