-
Notifications
You must be signed in to change notification settings - Fork 5.4k
NDEBUG #3120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NDEBUG #3120
Conversation
According to ISO/IEC 9899:2018 section 7.2 paragraph 1, Only presence or absence of the macro must take effect. Its value is insignificant. Even if NDEBUG is defined to be falsy, assertions must not take effect.
This option adds -DRUBY_DEBUG or -DNDEBUG accordingly to $optflags. Because $optflags could be passed directly, we have several choices: - When --enable-debug is set: force append options to $optflags. - When --disable-debug is set: force append options to $optflags. - When both are unspecified but $optflags is set: honour that. - Neither flags nor environment variables: defaults to --disable-debug.
On this code, I'm not sure why After consideration, I think the following strategy is better:
and other places I'm making a patch on this policy but there is strange behavior, maybe because of 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:
|
Using the following function instead of 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 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 |
previous issue is on clang-6 (Ubuntu 18.04, installed by |
At last, I confirmed |
We observed combination of multiple __builtin_assume() can cause strange compile results. To avoid this problem, __builtin_assume() is from clang-7. jkhttps://github.com/ruby/pull/3120#issuecomment-630821333
We observed combination of multiple __builtin_assume() can cause strange compile results on clang-6 (-DNDEBUG exposed this issue). To avoid this problem, __builtin_assume() is from clang-7. jkhttps://github.com/ruby/pull/3120#issuecomment-630821333
We observed combination of multiple __builtin_assume() can cause strange compile results on clang-6 (-DNDEBUG exposed this issue). To avoid this problem, __builtin_assume() is from clang-7. ruby#3120 (comment)
We observed combination of multiple __builtin_assume() can cause strange compile results on clang-6 (-DNDEBUG exposed this issue). To avoid this problem, __builtin_assume() is from clang-7. #3120 (comment)
You mean we should ignore
No objection.
Maybe we can simply ban |
Warning on build timing? For example,
|
Yes. But not immediately. |
Assertions in header files slows down an interpreter, so they should be turned off by default (simple `make`). To enable them, define a macro `RUBY_DEBUG=1` (e.g. `make cppflags=-DRUBY_DEBUG` or use `#define` at the very beggining of the file. Note that even if `NDEBUG=1` is defined, `RUBY_DEBUG=1` enables all assertions. [Feature #16837] related: ruby#3120 `assert()` lines in MRI *.c is not disabled even if `RUBY_DEBUG=0` and it can be disabled with `NDEBUG=1`. So please consider to use `RUBY_ASSERT()` if you want to disable them when `RUBY_DEBUG=0`.
Let's close this in favor of #3124 |
This was a bad idea. Several files under |
So in short, we cannot abandon |
Assertions in header files slows down an interpreter, so they should be turned off by default (simple `make`). To enable them, define a macro `RUBY_DEBUG=1` (e.g. `make cppflags=-DRUBY_DEBUG` or use `#define` at the very beggining of the file. Note that even if `NDEBUG=1` is defined, `RUBY_DEBUG=1` enables all assertions. [Feature #16837] related: #3120 `assert()` lines in MRI *.c is not disabled even if `RUBY_DEBUG=0` and it can be disabled with `NDEBUG=1`. So please consider to use `RUBY_ASSERT()` if you want to disable them when `RUBY_DEBUG=0`.
No description provided.