5

Context: In this answer, I learned that gcc's __builtin_unreachable() can have some surprisingly impactful performance implications, as it seems that the following:

if(condition) __builtin_unreachable();

is being entirely stripped, and used as an optimization hint as long as condition can be guaranteed to not have any side effect.

So my immediate reaction to this is that I should create the following macro, and use it absolutely everywhere I would normally use assert(), since side-effect inducing code inside an assert() would be a major bug in the first place:

// TODO: add handling of other compilers as appropriate.
#if defined(__GNUC__) && defined(NDEBUG)
  #define my_assert(condition) \
    if(!(condition)) __builtin_unreachable()
#else 
  #define my_assert(condition) assert(condition)
#endif

From a standards perspective, this would create a split in functionality between normal and NDEBUG builds, which you could make an argument excludes this macro from being the standard behavior of assert(). However, since my code would be functionally dead in the water in the case of assertion failures regardless, it's fully equivalent from a behavioral standpoint.

So my question is: Can anyone think of a reason not to do this (appart from asserts that involve a lot of indirections)?

Before you ask, yes, I've checked that gcc's behavior is to void cast the assertion away in NDEBUG builds.

  • Relevant https://stackoverflow.com/a/40447259/817643 – StoryTeller - Unslander Monica Sep 11 '17 at 07:55
  • No reason not to do this, as far as I can tell. You assume something for optimization in release builds, and replace it by an assertion in debug builds to catch programming errors. – StoryTeller - Unslander Monica Sep 11 '17 at 07:57
  • 1
    "use it absolutely everywhere" approach won't really work because debug assertions often include variables and calls that are only available in debug mode. The better idea would be define a dedicated macro with a more suitable name such as `my_assume` and utilize it only in appropriate places. – user7860670 Sep 11 '17 at 08:05
  • In general, one can put things in asserts which have side effects, so I would not do this. For example, there can be an expensive function which checks some invariant: `assert(isStructureValid());`. I definitely don't want to see that function called in release builds. So, as others suggested, create a new `assume` (or some other name) macro for this functionality. Or, with come compiler magic, only apply the check in release builds if the `condition` can be evaluated compile-time. – geza Sep 11 '17 at 11:25
  • @geza limiting this to compile-time conditions would defeat the entire purpose (see the linked question for an example). –  Sep 11 '17 at 13:27
  • Sorry, I meant to say "that a variable satisfies a compile-time constraint", like `assert(variable<=5);`. The intent of this comment was that the compiler optimize away the whole expression, and use the additional knowledge to optimize better. – geza Sep 11 '17 at 14:39

1 Answers1

2

Yes, there is a reason to not use it. Some people use the following defensive code practice that combines assert and exception ( assert(x>0); if (!(x<0)) throw std::logic_error("..") ) - see this answer:

Test Cases AND assertion statements

Your macro silently breaks the exception-part for release builds.

Hans Olsson
  • 11,123
  • 15
  • 38
  • 2
    I really don't like this practice, as it conflates programming errors and runtime errors. That assert has no business being there in my opinion. My codebase wouldn't suffer from this, but a this is a good thing to keep in mind. –  Sep 11 '17 at 08:11
  • 2
    I don't see the value in that tbh. It seems to defeat the purpose of `assert()` as having zero runtime impact on release code. May as well just use the exception if that's what you want. – Galik Sep 11 '17 at 08:55
  • 1
    This is not defensive but redundant code. If you need to put checks in release builds, then either create a special, compiled-even-in-release-mode release_assert, or use exception throwing only. Assert+throw is bad practice. – geza Sep 11 '17 at 11:19
  • @Galik It's not so much as whether than idiom is valuable or not, but the fact that usage of my macro would lead to UB in seemingly well-defined programs, which is a perfectly valid point, and reinforces the need for the macro to be appropriately named. –  Sep 11 '17 at 13:52
  • @Frank: I disagree. If a program doesn't satisfies an assert, then it is not a well-defined program. Even if that assert is not there, because it is compiled away. But I agree with you that a different named macro should be used for this purpose. – geza Sep 11 '17 at 14:43
  • When I find myself writing that, I avoid repeating myself so I write `if (!(x < 0)) { assert(false); throw std::logic_error(".."); }`. I guess OP's proposal would break on that too. Seems like the only safe thing is a different macro that clearly refers to the unreachability. – Ben Aug 20 '21 at 18:52