16

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.

shuhalo
  • 5,732
  • 12
  • 43
  • 60
  • 3
    Throw an exception? –  Sep 12 '19 at 14:33
  • 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. – NathanOliver Sep 12 '19 at 14:35
  • 3
    `abort()`? I am not sure if I understood the question correctly though – Aykhan Hagverdili Sep 12 '19 at 14:39
  • 1
    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?? – abelenky Sep 12 '19 at 14:47
  • 2
    "Better alternatives to assert(false) in C/C++" How about adding actual error handling in the production code... – Lundin Sep 12 '19 at 14:52
  • @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. – shuhalo Sep 12 '19 at 15:04
  • 2
    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. – local-ninja Sep 12 '19 at 15:15

9 Answers9

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. [..]

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • 1
    Why not call `std::abort()` directly? – Aykhan Hagverdili Sep 12 '19 at 14:43
  • 1
    @Ayxan because `std::abort()` promises a specific behaviour. `__assume(false)` makes no such promises – Caleth Sep 12 '19 at 14:54
  • 1
    @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. – Lightness Races in Orbit Sep 12 '19 at 14:56
  • @Ayxan Interesting wording, though. You make it sound like you think `std::abort()` is called _indirectly_ by these builtins? – Lightness Races in Orbit Sep 12 '19 at 14:57
  • 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. – shuhalo Sep 12 '19 at 15:00
  • Yes, again, as I wrote above there is a proposal to do just that. – Lightness Races in Orbit Sep 12 '19 at 15:01
  • @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) – Aykhan Hagverdili Sep 12 '19 at 15:35
  • @LightnessRacesinOrbit I said directly because `assert(false)` calls `std::abort()` indirectly – Aykhan Hagverdili Sep 12 '19 at 20:25
6

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.

Aykhan Hagverdili
  • 28,141
  • 6
  • 41
  • 93
5

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.

shuhalo
  • 5,732
  • 12
  • 43
  • 60
Frerich Raabe
  • 90,689
  • 19
  • 115
  • 207
  • Can one safely (sort of safely) wrap the second line into a macro that takes two arguments? – shuhalo Sep 17 '19 at 20:09
  • 1
    Yes, but it doesn't solve your problem, since `assert` itself is a macro that becomes void for `NDEBUG` builds regardless. – bug Mar 23 '21 at 06:15
  • 1
    @bug 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. – Frerich Raabe Mar 23 '21 at 08:55
4

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

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

user79878
  • 775
  • 9
  • 13
  • 2
    As of now, std::unreachable() only guarantees undefined behavior. The custom unreachable() function of Ayxan Haqverdili will suit the OP's purpose better. https://en.cppreference.com/w/cpp/utility/unreachable – Hari Mar 09 '23 at 09:09
  • 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. – rhaps0dy Apr 03 '23 at 00:15
2

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).

Petr Skocik
  • 58,047
  • 6
  • 95
  • 142
  • I believe they should include be a standardized `unreachable` macro similar to `assert` in the C (or C++) standard. – shuhalo Sep 12 '19 at 14:58
  • 1
    @shuhalo Per my answer, there is a proposal to do just that. – Lightness Races in Orbit Sep 12 '19 at 14:59
  • 1
    @shuhalo 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. – Petr Skocik Sep 12 '19 at 15:03
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);
}
bolov
  • 72,283
  • 15
  • 145
  • 224
0

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.

Christian Gibbons
  • 4,272
  • 1
  • 16
  • 29
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.

Enthus3d
  • 1,727
  • 12
  • 26
-3

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!
}
abelenky
  • 63,815
  • 23
  • 109
  • 159
  • 16
    It's a bad rule though – Lightness Races in Orbit Sep 12 '19 at 14:38
  • 7
    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. – SergeyA Sep 12 '19 at 14:39
  • 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. – abelenky Sep 12 '19 at 14:39
  • 3
    Since the question is cross-tagged [tag:c], I will permit your argument ;) – Lightness Races in Orbit Sep 12 '19 at 14:40
  • 1
    @abelenky 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. – SergeyA Sep 12 '19 at 14:40
  • `goto`?? no. Goto is ***never*** acceptable (to anyone learning). Experts may use it with extreme caution, in rare cases. – abelenky Sep 12 '19 at 14:41
  • 5
    Goto has valid uses. Let's not start this again :P – Lightness Races in Orbit Sep 12 '19 at 14:42
  • 1
    @SergeyA 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. – Lundin Sep 12 '19 at 14:49
  • I am not sure I follow. Can you show me the code in any online IDE? – SergeyA Sep 12 '19 at 14:54
  • @Lundin you missed my point of being able to call deinit function only for successfully inited objects. – SergeyA Sep 12 '19 at 15:03
  • @SergeyA So you add the additional error handling in the wrapper, such as `if(bar_error) free(foo);` – Lundin Sep 12 '19 at 15:05
  • 1
    @abelenky That however, is far uglier than "on error goto". Not to mention slower. – Lundin Sep 12 '19 at 15:07
  • @Lundin Your wrapping function inits N objects. Would it return N statuses? – SergeyA Sep 12 '19 at 15:07
  • @SergeyA 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. – Lundin Sep 12 '19 at 15:09