[assert] Control Flow Warnings with BOOST_ASSERT(false)
I've been using the BOOST_ASSERT macro. I like it because it can throw an exception rather than use the abort signal. And I can catch exceptions in automated unit-tests. However, I sometimes use assert(false) in locations that should be unreachable. And the compiler (well, GCC at least) understands this, because assert() calls abort() if the expression equals (evaluates to) false. If I use BOOST_ASSERT in this way, the compiler shows me various warnings about the flow of control. It does not know that my implementation of assertion_failed() throws an exception. To get rid of those warnings, I've changed BOOST_ASSERT for my personal use: #define BOOST_ASSERT(expr) { \ if (!(expr)) { \ ::boost::assertion_failed(#expr, BOOST_CURRENT_FUNCTION, \ __FILE__, __LINE__); \ abort(); \ } \ } It may not be the best way. For example, there may have been a reason to push everything into the ternary operator that I am not aware of. But it seems to work. I believe this functionality should be part of the library. Probably optional by defining another constant (BOOST_ENABLE_ASSERT_ABORT or somesuch). Are the developers aware of this problem? Do you think it will be solved in the library? Thanks in advance for your reply, -- Michiel Helvensteijn
Michiel Helvensteijn wrote:
I've changed BOOST_ASSERT for my personal use:
#define BOOST_ASSERT(expr) { \ if (!(expr)) { \ ::boost::assertion_failed(#expr, BOOST_CURRENT_FUNCTION, \ __FILE__, __LINE__); \ abort(); \ } \ }
there may have been a reason to push everything into the ternary operator that I am not aware of.
In general, I think the major issue with changing macro contents from a simple statement to a compound statement is that it can have peculiar effects on code surrounding macro invocations. For instance (contrived example): if (! data.valid()) BOOST_ASSERT(false); else { // ...process 'data'... } The classic workaround is to wrap the compound statement in a no-op 'do' statement: #define MACRO(stuff) do /* intended macro contents */ while (0) If there are special constraints unique to BOOST_ASSERT, I don't know them.
Nat Goodspeed wrote:
In general, I think the major issue with changing macro contents from a simple statement to a compound statement is that it can have peculiar effects on code surrounding macro invocations. For instance (contrived example):
if (! data.valid()) BOOST_ASSERT(false); else { // ...process 'data'... }
Yes, I see. The semicolon would be a second empty statement. It would cause a compiler error.
The classic workaround is to wrap the compound statement in a no-op 'do' statement:
#define MACRO(stuff) do /* intended macro contents */ while (0)
Of course. the do-loop expects a semicolon as part of the statement. I've made the change. It works fine.
If there are special constraints unique to BOOST_ASSERT, I don't know them.
I see two possible reasons not to use this construction. 1. The programmer may not always want to abort (or throw) at an assertion failure. That's why I propose to make this behavior optional. 2. It may somehow become unsafe in multi-threaded programs. But I don't think this is a big issue. I would like to hear from the developer on this. -- Michiel Helvensteijn
Michiel Helvensteijn:
I would like to hear from the developer on this.
The reason I used a ?: operator is to make BOOST_ASSERT an expression. It's supposed to be a drop-in replacement for assert, and I think that assert expands an expression and not a statement. Maybe we ought to decorate assertion_failed with __attribute__((noreturn))? Or replace the call to assertion_failed with (assertion_failed(...), abort())?
Peter Dimov wrote:
The reason I used a ?: operator is to make BOOST_ASSERT an expression. It's supposed to be a drop-in replacement for assert, and I think that assert expands an expression and not a statement.
I can't think of any reason to use assert as an expression. But you're right, you might as well give BOOST_ASSERT the same behavior.
Maybe we ought to decorate assertion_failed with __attribute__((noreturn))? Or replace the call to assertion_failed with (assertion_failed(...), abort())?
They both seem to work. :-) I guess the __attribute__ technique would be nicest. But it's a GCC extension. Does this problem exist with other compilers at all? If so, perhaps they could benefit from using the abort method instead. I don't have any experience with other compilers. -- Michiel Helvensteijn
participants (3)
-
Michiel Helvensteijn -
Nat Goodspeed -
Peter Dimov