Asserting when cleanup is required

Question

I've been looking at ways to use asserts in programs that must do some kind of cleanup on fatal errors, and have arrived at the following pattern (PROJECT being replaced by the name of the project):

#include <boost/exception/all.hpp>
#include <boost/exception/diagnostic_information.hpp>
#include <string>
#include <iostream>
#include <ostream>
typedef boost::error_info<struct tag_err_info,std::string> err_msg;
struct BaseError : virtual boost::exception, virtual std::exception {};
struct FatalError : BaseError {};
struct AssertionError : FatalError {};
#ifndef NDEBUG
#define PROJECT_ASSERT(expr) do { \
 if (expr); \
 else { \
 std::string error = "Assertion failed: `"; \
 error += #expr; \
 error += "'."; \
 BOOST_THROW_EXCEPTION(AssertionError() << err_msg(error)); \
 } \
 } while(false)
#else
#define PROJECT_ASSERT(expr) do {} while(false)
#endif
template<typename EXCEPTION>
void print_diagnostic_info(std::ostream& o, EXCEPTION const& e) {
#ifndef NDEBUG
 o << boost::diagnostic_information(e) << std::endl;
#else
 // Suppress unused parameter warnings
 (void)o;
 (void)e;
#endif
}
void fail() {
 PROJECT_ASSERT(true > 0);
 PROJECT_ASSERT(!!!true);
}
int main() {
 try {
 fail();
 }
 catch (BaseError& e) {
 print_diagnostic_info(std::cerr, e);
 if (std::string const* err_msg_p = boost::get_error_info<err_msg>(e))
 std::cerr << "Error: " << *err_msg_p << std::endl;
 // Cleanup goes here
 }
}

Running this gives

exception.cpp(39): Throw in function void fail()
Dynamic exception type: boost::exception_detail::clone_impl<AssertionError>
std::exception::what: std::exception
[tag_err_info*] = Assertion failed: `!!!true'.
Error: Assertion failed: `!!!true'.

What are the possible problems with this approach, and are there any improvements to be made?

One part I'm not sure about is defining error in the assertion. Seeing as this is an inner scope it should always hide out values, shouldn't it? Could this lead to unexpected warnings (maybe glue __LINE__ to it?)?

EDIT: With suggestions incorporated:

#include <boost/exception/all.hpp>
#include <boost/exception/diagnostic_information.hpp>
#include <string>
#include <iostream>
#include <ostream>
typedef boost::error_info<struct tag_err_info,std::string> err_msg;
struct BaseError : virtual boost::exception, virtual std::exception {};
struct FatalError : BaseError {};
struct AssertionError : FatalError {
 AssertionError(std::string const& err) {
 std::string error = "Assertion failed: `";
 error += err;
 error += "'.";
 *this << err_msg(error);
 }
};
#define PROJECT_ALWAYS_ASSERT(expr) do { \
 if (expr); \
 else { \
 BOOST_THROW_EXCEPTION(AssertionError(#expr)); \
 } \
 } while(false)
#ifndef NDEBUG
#define PROJECT_ASSERT(expr) PROJECT_ALWAYS_ASSERT(expr)
#else
#define PROJECT_ASSERT(expr)
#endif
template<typename EXCEPTION>
void print_diagnostic_info(std::ostream& o, EXCEPTION const& e) {
#ifndef NDEBUG
 o << boost::diagnostic_information(e) << std::endl;
#else
 // Suppress unused parameter warnings
 (void)o;
 (void)e;
#endif
}
void fail() {
 PROJECT_ASSERT(true > 0);
 PROJECT_ASSERT(1 + 1 > 0);
 PROJECT_ASSERT(!!!true);
}
int main() {
 try {
 fail();
 }
 catch (BaseError& e) {
 print_diagnostic_info(std::cerr, e);
 if (std::string const* err_msg_p = boost::get_error_info<err_msg>(e))
 std::cerr << "Error: " << *err_msg_p << std::endl;
 // Cleanup goes here
 }
}

Commentary: I'm not sure I like the way the case when NDEBUG is defined is handled (I don't like it when expressions are evaluated), but I don't think there's any way of handling both that and the unused variable error.

Further Commentary: I've thought about it, and I'm fairly certain I do not want expressions to be evaluated, as that would discourage expensive asserts (which I think is a bad thing, assuming those asserts improve matters).

Answer 1

Note: Personal opinions done matter. Just voicing them.

Personally I don't like (runtime) asserts (checks that disappear in production just asking for trouble and if something goes wrong you can't reproduce it in debug mode now). But OK its better than no checks.

Personally I don't like hiding throwing exceptions inside macros.
All the work you do to build the exception could be done inside the exception constructor.

I have not seen multiple inheritance in exceptions before. But then again I have not used boost::exception so that may be normal. But I don't see the need for virtual inheritance as they don't have a common root base (as they are both base classes). What does your catch look like that you need to inherit from both?

You are obviously trying to have zero cost when asserts are disabled, yet you are using the function: void print_diagnostic_info(std::ostream& o, EXCEPTION const& e). To be consistent with your other usage why is this not a macro (god I hate saying that since I hate using macros as function calls when we have template functions to solve that problem, but I think this is a unique type of situation where we are explicitly trying for zero overhead when asserts are disabled).

No point in this:

#else
#define PROJECT_ASSERT(expr) do {} while(false)
#endif
// Just do this:
#else
#define PROJECT_ASSERT(expr)
#endif

Is there any point in this:

struct BaseError : virtual boost::exception, virtual std::exception {};
struct FatalError : BaseError {};
struct AssertionError : FatalError {};

The only reason to have an exception hierarchy is if there is the ability to catch and explicitly recover from particular types of error. As this is an assertion you (by definition) can't recover so there is only the need for one exception type (and the generic one should suffice).

• Komi Golov: Would a call to print_diagnostics_info really take enough resources for it to be of any harm? Seeing as it's a template, I would expect the compiler to turn it into a no-op in release, and it's only ever called when an exception is caught: wouldn't the output done at that point be far more expensive?
• Komi Golov: The point of defining PROJECT_ASSERT that way when NDEBUG is defined is that there is no unnecessary ; in the program -- as far as I know, it could cause the compiler to issue a warning. Is this no longer the case? (I can't seem to reproduce it with GCC.)
• Komi Golov: The reason for deriving virtually is covered here: boost.org/doc/libs/1_40_0/libs/exception/doc/… . Your advice not to explicitly derive an assertion error and to construct the error string in the constructor are somewhat contradictory: I'd like assertions to be clearly unlike other fatal errors. I can see why having all fatal errors represented by one class would be a good idea, though -- which would you say is more important?
• Loki Astari: @AntonGolov: print_diagnostics_info: The cost would not be high and I personally would keep it as a function. But I am just making the comment you are attempting to have zero cost assertion stuff and this is not.
• Loki Astari: Empty statement is completely valid.

Answer 2

I would change the macro to this :

#ifndef NDEBUG
#define PROJECT_ASSERT(expr) \
 if (!(expr)) \
 { \
 std::string error = "Assertion failed: `"; \
 error += #expr; \
 error += "'."; \
 BOOST_THROW_EXCEPTION(AssertionError() << err_msg(error)); \
 }
#else
#define PROJECT_ASSERT(expr) (void)expr;
#endif

to avoid compiler's warnings about unused variables.

But as Loki said, I would leave assertions there.

---

The compiler (if you use maximum warning level) will report warning in next case :

void foo( int a )
{
 const bool aIsPos = a > 0;
 PROJECT_ASSERT( aIsPos );
 // use a
}

• BЈовић: @AntonGolov Right. That was a small bug (fixed now). However, it is possible to get unused variables, like in the edit
• Komi Golov: True; I wasn't aware it was common to do that. Thanks, redefining it as specified. About removing/including asserts in release builds: I'm mostly on the fence about that; will edit in a way to keep them.