HiveBrain v1.2.0
Get Started
← Back to all entries
debugcppMinor

Asserting when cleanup is required

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
cleanupassertingrequiredwhen

Problem

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 
#include 
#include 
#include 
#include 

typedef boost::error_info 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() 
void print_diagnostic_info(std::ostream& o, EXCEPTION const& e) {
#ifndef NDEBUG
    o  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(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
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
#include
#include
#include
#include

typedef boost::error_info err_msg;

struct BaseError : virtual boost::exception, virtual std::exception {};
struct FatalError : BaseError {};
struct AssertionError : FatalError {
AssertionError(std::string const& err)

Solution

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

Code Snippets

#else
#define PROJECT_ASSERT(expr) do {} while(false)
#endif

// Just do this:
#else
#define PROJECT_ASSERT(expr)
#endif
struct BaseError : virtual boost::exception, virtual std::exception {};
struct FatalError : BaseError {};
struct AssertionError : FatalError {};

Context

StackExchange Code Review Q#8047, answer score: 2

Revisions (0)

No revisions yet.