On this code, -DRUBY_DEBUG=1 doesn't enable all of assertions because some assertions check RUBY_NDEBUG (nearly equal to NDEBUG). It is convenient because we only need to define #define RUBY_DEBUG 1 on the beginning of a debugging file to enable all detailed assertions.

I'm not sure why -UNDEBUG is needed (who define it before this option?).

After consideration, I think the following strategy is better:

                 RUBY_ASSERT()    assert()
RUBY_DEBUG=1     enabled          enabled
RUBY_DEBUG=0
    def(NDEBUG)  disabled         disabled
  undef(NDEBUG)  disabled         enabled

• RUBY_DEBUG enables all of assertions in MRI.
• On the default, all assertions (dynamic checking) should be disabled because of performance penalty, it is controlled by RUBY_DEBUG.
• assert() is disabled if NDEBUG is defined because it is compatible with C's assert(). We can replace them with RUBY_DEBUG().

assert() should be defined by the following code in ruby_assert.h:

#define RUBY_ASSERT_NDEBUG(expr)     RUBY_ASSERT_MESG_WHEN(RUBY_NDEBUG, expr, #expr)

and other places RUBY_NDEBUG should not be used.

I'm making a patch on this policy but there is strange behavior, maybe because of __builtin_assume() on clang (a variable super is 0, but if (super) condition is evaluated as true).

The patch is here:

diff --git a/include/ruby/assert.h b/include/ruby/assert.h
index ac9b9c8cc3..17aef91921 100644
--- a/include/ruby/assert.h
+++ b/include/ruby/assert.h
@@ -82,11 +82,12 @@ RBIMPL_SYMBOL_EXPORT_END()
         RUBY_ASSERT_MESG(!(cond) || (expr), mesg))
 #endif /* RUBY_DEBUG */
 
-#define RUBY_ASSERT(expr) RUBY_ASSERT_MESG_WHEN((!RUBY_NDEBUG+0), expr, #expr)
+#define RUBY_ASSERT(expr)            RUBY_ASSERT_MESG_WHEN(FALSE, expr, #expr)
+#define RUBY_ASSERT_NDEBUG(expr)     RUBY_ASSERT_MESG_WHEN(RUBY_NDEBUG, expr, #expr)
 #define RUBY_ASSERT_WHEN(cond, expr) RUBY_ASSERT_MESG_WHEN(cond, expr, #expr)
-#define RUBY_ASSERT_ALWAYS(expr) RUBY_ASSERT_MESG_WHEN(TRUE, expr, #expr)
+#define RUBY_ASSERT_ALWAYS(expr)     RUBY_ASSERT_MESG_WHEN(TRUE, expr, #expr)
 
-#if ! RUBY_NDEBUG
+#if RUBY_DEBUG
 # define RBIMPL_ASSERT_OR_ASSUME(_) RUBY_ASSERT(_)
 #elif defined(RBIMPL_HAVE___ASSUME)
 # define RBIMPL_ASSERT_OR_ASSUME(_) RBIMPL_ASSUME(_)
diff --git a/include/ruby/internal/attr/const.h b/include/ruby/internal/attr/const.h
index ca9baa78a5..08629d1996 100644
--- a/include/ruby/internal/attr/const.h
+++ b/include/ruby/internal/attr/const.h
@@ -37,7 +37,7 @@
 #endif
 
 /** Enables #RBIMPL_ATTR_CONST iff. #RUBY_NDEBUG. */
-#if RUBY_NDEBUG
+#if !RUBY_DEBUG
 # define RBIMPL_ATTR_CONST_ON_NDEBUG() RBIMPL_ATTR_CONST()
 #else
 # define RBIMPL_ATTR_CONST_ON_NDEBUG() /* void */
diff --git a/include/ruby/internal/attr/constexpr.h b/include/ruby/internal/attr/constexpr.h
index 0ccd8f2b0f..d830fb2fa1 100644
--- a/include/ruby/internal/attr/constexpr.h
+++ b/include/ruby/internal/attr/constexpr.h
@@ -76,7 +76,7 @@
 #endif
 
 /** Enables #RBIMPL_ATTR_CONSTEXPR iff. #RUBY_NDEBUG. */
-#if RUBY_NDEBUG
+#if !RUBY_DEBUG
 # define RBIMPL_ATTR_CONSTEXPR_ON_NDEBUG(_) RBIMPL_ATTR_CONSTEXPR(_)
 #else
 # define RBIMPL_ATTR_CONSTEXPR_ON_NDEBUG(_) /* void */
diff --git a/include/ruby/internal/attr/pure.h b/include/ruby/internal/attr/pure.h
index b6aca7b720..72c7734753 100644
--- a/include/ruby/internal/attr/pure.h
+++ b/include/ruby/internal/attr/pure.h
@@ -34,7 +34,7 @@
 #endif
 
 /** Enables #RBIMPL_ATTR_PURE iff. #RUBY_NDEBUG. */
-#if RUBY_NDEBUG
+#if !RUBY_DEBUG
 # define RBIMPL_ATTR_PURE_ON_NDEBUG() RBIMPL_ATTR_PURE()
 #else
 # define RBIMPL_ATTR_PURE_ON_NDEBUG() /* void */

And I get the following result:

// in class.c
517         super = RCLASS_SUPER(klass);
518         while (RB_TYPE_P(super, T_ICLASS)) super = RCLASS_SUPER(super);
519         RCLASS_SET_SUPER(metaclass, super ? ENSURE_EIGENCLASS(super) : rb_cClass);

super is 0, but ENSURE_EIGENCLASS(super) is evaluated and SEGV. Do you have any idea on it?

Using the following function instead of RB_TYPE_P can reproduce the problem.

RBIMPL_ATTR_PURE_ON_NDEBUG()
RBIMPL_ATTR_ARTIFICIAL()
RBIMPL_ATTR_FORCEINLINE()
static bool
my_type_p(VALUE obj, enum ruby_value_type t)
{
    if (RB_SPECIAL_CONST_P(obj)) {
        return false;
    }
    else if (RB_BUILTIN_TYPE(obj)) {
        return true;
    }
    else {
        return false;
    }
}

and use RBASIC(obj)->flags & RUBY_T_MASK solves the issue instead of RB_BUILTIN_TYPE(). It seems clang bug?

RBIMPL_ATTR_PURE_ON_NDEBUG()
RBIMPL_ATTR_ARTIFICIAL()
static inline enum ruby_value_type
RB_BUILTIN_TYPE(VALUE obj)
{
    RBIMPL_ASSERT_OR_ASSUME(! RB_SPECIAL_CONST_P(obj));

    VALUE ret = RBASIC(obj)->flags & RUBY_T_MASK;
    return RBIMPL_CAST((enum ruby_value_type)ret);
}

The assumption RBIMPL_ASSERT_OR_ASSUME(! RB_SPECIAL_CONST_P(obj)); seems affecting the following conditions. removing RBIMPL_ASSERT_OR_ASSUME line solves the problem.

previous issue is on clang-6 (Ubuntu 18.04, installed by apt install clang) and clang-7 doesn't have an issue. Does clang-6 has issues on __builtin_assume()?

At last, I confirmed CC="clang-6.0 -DNDEBUG" with current master causes this issue (SEGV on startup).

After consideration, I think the following strategy is better:

                 RUBY_ASSERT()    assert()
RUBY_DEBUG=1     enabled          enabled
RUBY_DEBUG=0
    def(NDEBUG)  disabled         disabled
  undef(NDEBUG)  disabled         enabled

• RUBY_DEBUG enables all of assertions in MRI.

You mean we should ignore -DNDEBUG on RUBY_DEBUG=1 ? I don't think that's a good idea. User's intention is very clear that they don't want assert(). At least warning shall be issued for such situation.

• On the default, all assertions (dynamic checking) should be disabled because of performance penalty, it is controlled by RUBY_DEBUG.

No objection.

• assert() is disabled if NDEBUG is defined because it is compatible with C's assert(). We can replace them with RUBY_DEBUG().

Maybe we can simply ban assert() from the entire repository, to force everything to use RUBY_ASSERT(). GCC has #pragma gcc poison which seems to be useful for this purpose. Care must be taken to allow extension libraries using assert() though, because they have their own policy.

You mean we should ignore -DNDEBUG on RUBY_DEBUG=1 ? I don't think that's a good idea. User's intention is very clear that they don't want assert(). At least warning shall be issued for such situation.

Warning on build timing? For example,

#if RUBY_DEBUG > 0 && defined(NDEBUG)
#warning NDEBUG is ignored because RUBY_DEBUG>0.
#endif

Maybe we can simply ban assert() from the entire repository, to force everything to use RUBY_ASSERT(). GCC has #pragma gcc poison which seems to be useful for this purpose. Care must be taken to allow extension libraries using assert() though, because they have their own policy.

Yes. But not immediately.

Let's close this in favor of #3124

Maybe we can simply ban assert() from the entire repository, to force everything to use RUBY_ASSERT(). GCC has #pragma gcc poison which seems to be useful for this purpose. Care must be taken to allow extension libraries using assert() though, because they have their own policy.

This was a bad idea. Several files under ccan, coroutine, and missing do have assert(). We cannot patch them.

So in short, we cannot abandon assert(). We have to cope with NDEBUG.