Warn when an optimized build doesn't set NDEBUG and then #define it ourselves --…

Warn when an optimized build doesn't set NDEBUG and then #define it ourselves -- unless the build explicitly sets DEBUG.

c++/src/capnp/encoding-test.c++

@@ -221,10 +221,10 @@ TEST(Encoding, GenericObjects) {
 #define EXPECT_NONFATAL_FAILURE EXPECT_ANY_THROW
 #endif
 
-#ifdef NDEBUG
-#define EXPECT_DEBUG_ANY_THROW(EXP)
-#else
+#ifdef KJ_DEBUG
 #define EXPECT_DEBUG_ANY_THROW EXPECT_ANY_THROW
+#else
+#define EXPECT_DEBUG_ANY_THROW(EXP)
 #endif
 
 TEST(Encoding, Unions) {

c++/src/kj/common-test.c++

@@ -203,9 +203,9 @@ TEST(Common, Downcast) {
   Foo& foo = bar;
 
   EXPECT_EQ(&bar, &downcast<Bar>(foo));
-#if !defined(NDEBUG) && !KJ_NO_RTTI
+#if defined(KJ_DEBUG) && !KJ_NO_RTTI
 #if KJ_NO_EXCEPTIONS
-#ifndef NDEBUG
+#ifdef KJ_DEBUG
   EXPECT_DEATH_IF_SUPPORTED(downcast<Baz>(foo), "Value cannot be downcast");
 #endif
 #else

c++/src/kj/common.h

@@ -98,6 +98,18 @@ typedef unsigned char byte;
   #endif
 #endif
 
+#if __OPTIMIZE__ && !defined(NDEBUG) && !defined(DEBUG) && !defined(KJ_DEBUG)
+#warning "You've enabled optimization but not NDEBUG. Usually optimized builds should #define \
+NDEBUG to disable debug asserts, so I am #defining it for you. If you actually want debug asserts, \
+please #define DEBUG or KJ_DEBUG. To make this warning go away, #define NDEBUG yourself, e.g. with \
+the compiler flag -DNDEBUG."
+#define NDEBUG 1
+#endif
+
+#if !defined(NDEBUG) && !defined(KJ_DEBUG)
+#define KJ_DEBUG
+#endif
+
 #define KJ_DISALLOW_COPY(classname) \
   classname(const classname&) = delete; \
   classname& operator=(const classname&) = delete
@@ -109,12 +121,12 @@ typedef unsigned char byte;
 // expect the condition to be true/false enough of the time that it's worth hard-coding branch
 // prediction.
 
-#if defined(NDEBUG) && !__NO_INLINE__
-#define KJ_ALWAYS_INLINE(prototype) inline prototype __attribute__((always_inline))
-// Force a function to always be inlined.  Apply only to the prototype, not to the definition.
-#else
+#if defined(KJ_DEBUG) || __NO_INLINE__
 #define KJ_ALWAYS_INLINE(prototype) inline prototype
 // Don't force inline in debug mode.
+#else
+#define KJ_ALWAYS_INLINE(prototype) inline prototype __attribute__((always_inline))
+// Force a function to always be inlined.  Apply only to the prototype, not to the definition.
 #endif
 
 #define KJ_NORETURN __attribute__((noreturn))
@@ -138,9 +150,7 @@ void unreachable() KJ_NORETURN;
 
 }  // namespace _ (private)
 
-#ifdef NDEBUG
-#define KJ_IREQUIRE(condition, ...)
-#else
+#ifdef KJ_DEBUG
 #define KJ_IREQUIRE(condition, ...) \
     if (KJ_LIKELY(condition)); else ::kj::_::inlineRequireFailure( \
         __FILE__, __LINE__, #condition, #__VA_ARGS__, ##__VA_ARGS__)
@@ -148,6 +158,8 @@ void unreachable() KJ_NORETURN;
 // check preconditions inside inline methods.  KJ_IREQUIRE is particularly useful in that
 // it will be enabled depending on whether the application is compiled in debug mode rather than
 // whether libkj is.
+#else
+#define KJ_IREQUIRE(condition, ...)
 #endif
 
 #define KJ_UNREACHABLE ::kj::_::unreachable();

c++/src/kj/debug.h

@@ -111,7 +111,7 @@ namespace kj {
     ::kj::_::Debug::log(__FILE__, __LINE__, ::kj::_::Debug::Severity::severity, \
                         #__VA_ARGS__, __VA_ARGS__)
 
-#define KJ_DBG(...) KJ_LOG(DEBUG, ##__VA_ARGS__)
+#define KJ_DBG(...) KJ_LOG(DBG, ##__VA_ARGS__)
 
 #define _kJ_FAULT(nature, cond, ...) \
   if (KJ_LIKELY(cond)) {} else \
@@ -159,14 +159,14 @@ namespace kj {
 #define KJ_ASSERT_NONNULL(value, ...) _kJ_NONNULL(LOCAL_BUG, value, ##__VA_ARGS__)
 #define KJ_REQUIRE_NONNULL(value, ...) _kJ_NONNULL(PRECONDITION, value, ##__VA_ARGS__)
 
-#ifdef NDEBUG
-#define KJ_DLOG(...) do {} while (false)
-#define KJ_DASSERT(...) do {} while (false)
-#define KJ_DREQUIRE(...) do {} while (false)
-#else
+#ifdef KJ_DEBUG
 #define KJ_DLOG LOG
 #define KJ_DASSERT KJ_ASSERT
 #define KJ_DREQUIRE KJ_REQUIRE
+#else
+#define KJ_DLOG(...) do {} while (false)
+#define KJ_DASSERT(...) do {} while (false)
+#define KJ_DREQUIRE(...) do {} while (false)
 #endif
 
 namespace _ {  // private
@@ -182,7 +182,7 @@ public:
     WARNING,   // A problem was detected but execution can continue with correct output.
     ERROR,     // Something is wrong, but execution can continue with garbage output.
     FATAL,     // Something went wrong, and execution cannot continue.
-    DEBUG      // Temporary debug logging.  See KJ_DBG.
+    DBG        // Temporary debug logging.  See KJ_DBG.
 
     // Make sure to update the stringifier if you add a new severity level.
   };

c++/src/kj/exception.c++

@@ -32,7 +32,7 @@
 #include <execinfo.h>
 #endif
 
-#if defined(__linux__) && !defined(NDEBUG)
+#if defined(__linux__) && defined(KJ_DEBUG)
 #include <stdio.h>
 #include <pthread.h>
 #endif
@@ -42,7 +42,7 @@ namespace kj {
 namespace {
 
 String getStackSymbols(ArrayPtr<void* const> trace) {
-#if defined(__linux__) && !defined(NDEBUG)
+#if defined(__linux__) && defined(KJ_DEBUG)
   // We want to generate a human-readable stack trace.
 
   // TODO(someday):  It would be really great if we could avoid farming out to addr2line and do

c++/src/kj/main.c++

@@ -453,7 +453,33 @@ void MainBuilder::MainImpl::operator()(StringPtr programName, ArrayPtr<const Str
             subMain(str(programName, ' ', params[i + 1]), arrayPtr(&dummyArg, 1));
             return;
           } else if (params[i + 1] == "help") {
-            impl->context.exitInfo("Help, I'm trapped in a help text factory!");
+            uint count = 0;
+            for (uint j = i + 2;
+                 j < params.size() && (params[j] == "help" || params[j] == "--help");
+                 j++) {
+              ++count;
+            }
+
+            switch (count) {
+              case 0:
+                impl->context.exitInfo("Help about help?  We must go deeper...");
+                break;
+              case 1:
+                impl->context.exitInfo(
+                    "Yo dawg, I heard you like help.  So I wrote you some help about how to use "
+                    "help so you can get help on help.");
+                break;
+              case 2:
+                impl->context.exitInfo("Help, I'm trapped in a help text factory!");
+                break;
+              default:
+                if (count < 10) {
+                  impl->context.exitError("Killed by signal 911 (SIGHELP)");
+                } else {
+                  impl->context.exitInfo("How to keep an idiot busy...");
+                }
+                break;
+            }
           } else {
             usageError(programName, str(params[i + 1], ": unknown command"));
           }

c++/src/kj/mutex-test.c++

@@ -41,10 +41,10 @@ inline void delay() { usleep(10000); }
 #define EXPECT_NONFATAL_FAILURE EXPECT_ANY_THROW
 #endif
 
-#ifdef NDEBUG
-#define EXPECT_DEBUG_ANY_THROW(EXP)
-#else
+#ifdef KJ_DEBUG
 #define EXPECT_DEBUG_ANY_THROW EXPECT_ANY_THROW
+#else
+#define EXPECT_DEBUG_ANY_THROW(EXP)
 #endif
 
 TEST(Mutex, MutexGuarded) {

c++/src/kj/mutex.h

@@ -277,21 +277,21 @@ inline Locked<const T> MutexGuarded<T>::lockShared() const {
 
 template <typename T>
 inline const T& MutexGuarded<T>::getAlreadyLockedShared() const {
-#ifndef NDEBUG
+#ifdef KJ_DEBUG
   mutex.assertLockedByCaller(_::Mutex::SHARED);
 #endif
   return value;
 }
 template <typename T>
 inline T& MutexGuarded<T>::getAlreadyLockedShared() {
-#ifndef NDEBUG
+#ifdef KJ_DEBUG
   mutex.assertLockedByCaller(_::Mutex::SHARED);
 #endif
   return value;
 }
 template <typename T>
 inline T& MutexGuarded<T>::getAlreadyLockedExclusive() const {
-#ifndef NDEBUG
+#ifdef KJ_DEBUG
   mutex.assertLockedByCaller(_::Mutex::EXCLUSIVE);
 #endif
   return const_cast<T&>(value);

super-test.sh

@@ -151,6 +151,10 @@ __EOF__
   shift
 done
 
+# Build optimized builds because they catch more problems, but also enable debugging macros.
+# Enable lots of warnings and make sure the build breaks if they fire.
+export CXXFLAGS="-O2 -DDEBUG -Wall -Werror"
+
 STAGING=$PWD/tmp-staging
 
 if [ "$QUICK" != quick ]; then
@@ -203,7 +207,7 @@ test "x$(which capnpc-c++)" = "x$STAGING/bin/capnpc-c++"
 
 cd samples
 doit capnp compile -oc++ addressbook.capnp -I"$STAGING"/include --no-standard-import
-doit ${CXX:-g++} -std=c++11 $SAMPLE_CXXFLAGS -I"$STAGING"/include -L"$STAGING"/lib \
+doit ${CXX:-g++} -std=c++11 $CXXFLAGS $SAMPLE_CXXFLAGS -I"$STAGING"/include -L"$STAGING"/lib \
     addressbook.c++ addressbook.capnp.c++ -lcapnp -lkj -pthread -o addressbook
 echo "@@@@ ./addressbook (in various configurations)"
 ./addressbook write | ./addressbook read
@@ -268,13 +272,13 @@ echo "========================================================================="
 echo "Testing with -fno-rtti and -fno-exceptions"
 echo "========================================================================="
 
-doit ./configure --disable-shared CXXFLAGS=-fno-rtti
+doit ./configure --disable-shared CXXFLAGS="$CXXFLAGS -fno-rtti"
 doit make -j6 check
 doit make distclean
-doit ./configure --disable-shared CXXFLAGS=-fno-exceptions
+doit ./configure --disable-shared CXXFLAGS="$CXXFLAGS -fno-exceptions"
 doit make -j6 check
 doit make distclean
-doit ./configure --disable-shared CXXFLAGS="-fno-rtti -fno-exceptions"
+doit ./configure --disable-shared CXXFLAGS="$CXXFLAGS -fno-rtti -fno-exceptions"
 doit make -j6 check
 
 doit make maintainer-clean