Better alternatives to assert(false) in C/C++
Asked Answered
A

9

20

Currently, I write

assert(false);

at places that my code is never supposed to reach. One example, in a very C-ish style, is:

int findzero( int length, int * array ) {
  for( int i = 0; i < length; i++ )
    if( array[i] == 0 )
      return i;
  assert(false);
}

My compiler recognizes that the program finishes once assert(false) has been reached. However, whenever I compile with -DNDEBUG for performance reasons, the last assertion vanishes and the compiler warns that the execution finishes the function without a return statement.

What are better alternatives of finishing off a program if a supposedly unreachable part of the code has been reached? The solution should

  • be recognized by the compiler and not produce warnings (like the ones above or others)
  • perhaps even allow for a custom error message.

I am explicitly interested in solutions no matter whether it's modern C++ or like 90s C.

Ageless answered 12/9, 2019 at 14:31 Comment(7)
Throw an exception?Ayotte
Won't help in the general case but just return length if nothing is found. The call site can check for that if they want and you don't get any warnings.Sheley
abort()? I am not sure if I understood the question correctly thoughPlatonism
How should your code behave if 0 is truly not found in the array? You are assuming that 0 is there somewhere. How is that guaranteed??Watch
"Better alternatives to assert(false) in C/C++" How about adding actual error handling in the production code...Unreserved
@Ayxan I would like to capture that this abort is specifically about reaching something unreachable. I guess that #define unreachable_reached abort would do it as well.Ageless
I would never let "unreachable" code pass a code review. It's a design flaw. The caller knows what array stores, and he is responsible for asserting the impossible. Let findzero return an optional and assert on the call site.Danny
P
15

Replacing your assert(false) is exactly what "unreachable" built-ins are for.

They are a semantic equivalent to your use of assert(false). In fact, VS's is spelt very similarly.

GCC/Clang/Intel:

__builtin_unreachable()

MSVS:

 __assume(false)

These have effect regardless of NDEBUG (unlike assert) or optimisation levels.

Your compiler, particularly with the above built-ins but also possibly with your assert(false), nods its head in understanding that you're promising that part of the function will never be reached. It can use this to perform some optimisations on certain code paths, and it will silence warnings about missing returns because you've already promised that it was deliberate.

The trade-off is that the statement itself has undefined behaviour (much like going forth and flowing off the end of the function was already). In some situations, you may instead wish to consider throwing an exception (or returning some "error code" value instead), or calling std::abort() (in C++) if you want to just terminate the program.


There's a proposal (P0627R0), to add this to C++ as a standard attribute.


From the GCC docs on Builtins:

If control flow reaches the point of the __builtin_unreachable, the program is undefined. It is useful in situations where the compiler cannot deduce the unreachability of the code. [..]

Postboy answered 12/9, 2019 at 14:33 Comment(8)
Why not call std::abort() directly?Platonism
@Ayxan because std::abort() promises a specific behaviour. __assume(false) makes no such promisesCarner
@Ayxan Because std::abort is a different thing with different meaning and different effects. The builtins are for when you know a bit of code won't be reached (unless all bets are really off anyway); abort is for when you want to catch a bit of code being reached.Postboy
@Ayxan Interesting wording, though. You make it sound like you think std::abort() is called indirectly by these builtins?Postboy
That seems to come as close as it gets to what I am looking for. The only downside is that it is technically vendor-dependent though almost a standard across different compilers. I believe something like that should be standardized.Ageless
Yes, again, as I wrote above there is a proposal to do just that.Postboy
@LightnessRacesinOrbit yes, but std::abort is in the standard and all OP wants is a portable way terminate the program immediately (which is what std::abort does)Platonism
@LightnessRacesinOrbit I said directly because assert(false) calls std::abort() indirectlyPlatonism
P
8

As a fully portable solution, consider this:

[[ noreturn ]] void unreachable(std::string_view msg = "<No Message>") {
    std::cerr << "Unreachable code reached. Message: " << msg << std::endl;
    std::abort();
}

The message part is, of course, optional.

Platonism answered 12/9, 2019 at 15:20 Comment(4)
Is it safe to not flush the stream (so the message will get printed) before calling abort()?Current
@Current std::cerr should flush automaticallty after each operation.Caron
std::endl flushes as well.Whack
@Whack originally I had '\n' instead of endl and I changed it in response to the comment.Platonism
A
6

I like to use

assert(!"This should never happen.");

...which can also be used with a condition, as in

assert(!vector.empty() || !"Cannot take element from empty container." );

What's nice about this is that the string shows up in the error message in case an assertion does not hold.

Alanealanine answered 17/9, 2019 at 17:50 Comment(3)
Can one safely (sort of safely) wrap the second line into a macro that takes two arguments?Ageless
Yes, but it doesn't solve your problem, since assert itself is a macro that becomes void for NDEBUG builds regardless.Swan
@Swan Yes, you're right -- I think my answer should have been a comment. I wanted to propose a slight improvement over assert(false); which satisfies the "allow for a custom error message." requirement.Alanealanine
W
5

Looks like std::unreachable() made it to C++23:

https://en.cppreference.com/w/cpp/utility/unreachable

Whack answered 11/10, 2022 at 18:55 Comment(2)
As of now, std::unreachable() only guarantees undefined behavior. The custom unreachable() function of Ayxan Haqverdili will suit the OP's purpose better. en.cppreference.com/w/cpp/utility/unreachableReluctance
Yeah, it seems to be a very bad idea to use std::unreachable for program correctness. Since you're invoking undefined behavior with it, it doesn't mean: "If the program is correct this should never happen, so I'd like to be warned when it does and the program should crash." Rather, you're indicating: "I'm certain this will never happen, compiler, feel free to optimize away!" A footgun if I've ever seen one.Panter
C
3

I use a custom assert that turns into __builtin_unreachable() or *(char*)0=0 when NDEBUG is on (I also use an enum variable instead of a macro so that I can easily set NDEBUG per scope).

In pseudocode, it's something like:

#define my_assert(X) do{ \ 
       if(!(X)){ \
           if (my_ndebug) MY_UNREACHABLE();  \
           else my_assert_fail(__FILE__,__LINE__,#X); \
       } \
     }while(0)

The __builtin_unreachable() should eliminate the warning and help with optimization at the same time, but in debug mode, it's better to have an assert or an abort(); there so you get a reliable panic. (__builtin_unreachable() just gives you undefined behavior when reached).

Clementineclementis answered 12/9, 2019 at 14:50 Comment(3)
I believe they should include be a standardized unreachable macro similar to assert in the C (or C++) standard.Ageless
@Ageless Per my answer, there is a proposal to do just that.Postboy
@Ageless Honestly I hate committees unnecessarily bloating my favorite language. Compilers could just recognize *(char*)0=0; as equivalent to __builtin_unreachable(); because it semantically is equivalent, and then there's no need for any proposals or extensions.Clementineclementis
M
1

I recommend C++ Core Gudelines's Expects and Ensures. They can be configured to abort (default), throw, or do nothing on violation.

To suppress compiler warnings on unreachable branches you can also use GSL_ASSUME.

#include <gsl/gsl>

int findzero( int length, int * array ) {
  Expects(length >= 0);
  Expects(array != nullptr);

  for( int i = 0; i < length; i++ )
    if( array[i] == 0 )
      return i;

  Expects(false);
  // or
  // GSL_ASSUME(false);
}
Mohler answered 12/9, 2019 at 14:59 Comment(0)
D
0

I believe the reason you are getting the errors is because assertions are generally used for debugging on your own code. When these functions are run in release, exceptions should be used instead with an exit by std::abort() to indicate abnormal program termination.

If you still want to use asserts, there is an answer about defining a custom one by PSkocik, as well as a link here where someone proposes the use of custom asserts and how to enable them in cmake here as well.

Divorcement answered 12/9, 2019 at 15:19 Comment(0)
C
-1

assert is meant for scenarios that are ACTUALLY supposed to be impossible to happen during execution. It is useful in debugging to point out "Hey, turns out what you thought to be impossible is, in fact, not impossible." It looks like what you should be doing in the given example is expressing the function's failure, perhaps by returning -1 as that would not be a valid index. In some instances, it might be useful to set errno to clarify the exact nature of an error. Then, with this information, the calling function can decide how to handle such error.

Depending on how critical this error is to the rest of the application, you might try to recover from it, or you might just log the error and call exit to put it out of its misery.

Charis answered 12/9, 2019 at 14:56 Comment(0)
W
-2

One rule that is sometimes found in style-guides is

"Never return from the middle of a function"
All functions should have a single return, at the end of the function.

Following this rule, your code would look like:

int findzero( int length, int * array ) {
  int i;
  for( i = 0; i < length; i++ )
  {
    if( array[i] == 0 )
      break;             // Break the loop now that i is the correct value
  }

  assert(i < length);    // Assert that a valid index was found.
  return i;              // Return the value found, or "length" if not found!
}
Watch answered 12/9, 2019 at 14:35 Comment(14)
It's a bad rule thoughPostboy
That is a Very Bad Rule indeed. Following it leaves you with nothing but pain. The whole idea of SBRO / RAII is to allow those early returns which greatly simplify program code and allows for better reasoning about it.Borden
Lightness: In code where RAII is not available, it will make sure that every free and close is called. If you return from the middle of a function, you risk skipping later steps with important side effects.Watch
Since the question is cross-tagged c, I will permit your argument ;)Postboy
@Watch RAII is always available in C++. In C, goto end; is accepted way of dealing with those. Step-ladder of nested scopes is not a solution.Borden
goto?? no. Goto is never acceptable (to anyone learning). Experts may use it with extreme caution, in rare cases.Watch
Goto has valid uses. Let's not start this again :PPostboy
@Borden Wrapper function -> call function that initializes stuff -> return with error code upon error -> cleanup in the wrapper. Multiple times more readable and maintainable than 1980s BASIC "on error goto". Coincidently, that also proves why multiple returns can be good practice.Unreserved
I am not sure I follow. Can you show me the code in any online IDE?Borden
@Unreserved you missed my point of being able to call deinit function only for successfully inited objects.Borden
@Borden So you add the additional error handling in the wrapper, such as if(bar_error) free(foo);Unreserved
@Watch That however, is far uglier than "on error goto". Not to mention slower.Unreserved
@Unreserved Your wrapping function inits N objects. Would it return N statuses?Borden
@Borden It's all about how you'd design the error handling. If you create a private enum with error codes that appear in the same order as the internal function's return calls, then it is equivalent to "on error goto". Though with one major benefit: you don't need to have the goto considered harmful debate yet another time.Unreserved

© 2022 - 2024 — McMap. All rights reserved.