I stumbled upon an informative article: http://cnicholson.net/2009/02/stupid-c-tricks-adventures-in-assert/ which pointed out a great number of problems that exist in my current suite of debugging macros.
The full code for the final version of the macro is given near the end of the article if you follow the link.
The general form as presented is like this (somebody please correct me if i am wrong in transposing it):
#ifdef DEBUG
#define ASSERT(cond) \
do \
{ \
if (!(cond)) \
{ \
ReportFailure(#cond, __FILE__, __LINE__, 0); \
HALT(); \
} \
} while(0)
#else
#define ASSERT(cond) \
do { (void)sizeof(cond); } while(0)
While thinking about modifying my code with what I have learned, I noticed a couple of interesting variations posted in the comments for that article:
One was that you cannot use this macro with the ternary operator (i.e. cond?ASSERT(x):func()), and the suggestion was to replace the if() with the ternary operator and some parentheses as well as the comma operator. Later on another commenter provided this:
#ifdef DEBUG
#define ASSERT(x) ((void)(!(x) && assert_handler(#x, __FILE__, __LINE__) && (HALT(), 1)))
#else
#define ASSERT(x) ((void)sizeof(x))
#endif
I thought the use of logical and && is particularly smart in this case and it seems to me that this version is more flexible than one using the if or even the ternary ?:. Even nicer is that the return value of assert_handler can be used to determine if the program should halt. Although I am not sure why it is (HALT(), 1) instead of just HALT().
Are there any particular shortcomings with the second version here that I have overlooked? It does away with the do{ } while(0) wrapped around the macros but it seems to be unnecessary here because we don't need to deal with ifs.
What do you think?
Answer
In C and C++ standard library, assert is a macro that is required to act as a function. A part of that requirement is that users must be able to use it in expressions. For example, with standard assert I can do
int sum = (assert(a > 0), a) + (assert(b < 0), b);
which is functionally the same as
assert(a > 0 && b < 0)
int sum = a + b;
Even though the former might not be a very good way to write an expression, the trick is still very useful in many more appropriate cases.
This immediately means that if one wants their own custom ASSERT macro to mimic standard assert behavior and usability, then using if or do { } while (0) in the definition of ASSERT is out of question. One is limited to expressions in that case, meaning using either ?: operator or short-circuiting logical operators.
Of course, if one doesn't care about making a standard-like custom ASSERT, then one can use anything, including if. The linked article doesn't even seem to consider this issue, which is rather strange. In my opinion, a function-like assert macro is definitely more useful than a non-function-like one.
As for the (HALT(), 1)... It is done that way because && operator requires a valid argument. The return value of HALT() might not represent a valid argument for &&. It could be void for what I know, which means that a mere HALT() simply won't compile as an argument of &&. The (HALT(), 1) always evaluates to 1 and has type int, which is always a valid argument for &&. So, (HALT(), 1) is always a valid argument for && regardless of the type of HALT().
Your last comment about do{ } while(0) does not seem to make much sense. The point of enclosing a macro into do{ } while(0) is to deal with external ifs, not the ifs inside the macro definition. You always have to deal with external ifs, since there's always a chance that your macro will be used in an external if. In the latter definition do{ } while(0) is not needed because that macro is an expression. And being an expression, it already naturally has no problems with external ifs. So, there's no need to do anything about them. Moreover, as I said above, enclosing it into do{ } while(0) would completely defeat its purpose, turning it into a non-expression.