C++ exception class for error reporting, compatible with C++98 through C++20 (ver 2)

Question

This question contains revised code from the question What should a C++ error reporting exception class have to be portable across language versions?. I summarize what is different from that code at the end.

In short, I have designed a C++ exception class suitable for use for error reporting and would like feedback on it.

The context for this code is a library that does parsing based on user-supplied parsing rules, but my focus here is on the exception class design, which I would like to be usable in any situation where errors are reported by throwing exceptions.

The requirements are:

1. Allow errors to be reported from a library in a way that library clients can recover from. That implies it has enough information, in machine-readable form, to allow clients to discern the appropriate recovery response.

2. Produce a human-readable description tailored to the error conditions. This should start with something related to the type of the exception, and be followed by increasing levels of detail as appropriate and available. I want the freedom to be fairly expressive here; fixed limits on string lengths would be unfortunate, and restricting myself to string literals is not acceptable.

3. That description is what the what() method produces, so a generic catch (std::exception &e) block will produce it.

4. Sites that throw the exception should only provide the machine-readable elements; construction of the human-readable string should happen on only one place.

5. It should work with C++98 through C++20, assuming a mainstream compiler like GCC or Microsoft. Limited use of #if __cplusplus is acceptable, but I don't want multiple completely different versions.

6. It should ideally be a mostly conventional design that other C++ programmers are likely to perceive as familiar. I want best practices, not a research project.

Below is an implementation that I think meets these requirements, in part by incorporating suggestions received on the previous question. It compiles without complaints with GCC-8.1 -Wall -pedantic when using -std=c++98, -std=c++11, -std=c++14, -std=c++17, and -std=c++2a.

// exception.cpp
// Example of a user-defined exception class for error reporting.

#include <cstring>                     // std::strcpy
#include <exception>                   // std::exception
#include <iostream>                    // std::cout, std::endl
#include <new>                         // std::bad_alloc
#include <sstream>                     // std::ostringstream
#include <string>                      // std::string


// A declaration specifier that says the method does not throw.
#ifndef NOEXCEPT
#  if __cplusplus >= 201103L
#    define NOEXCEPT noexcept
#  else
#    define NOEXCEPT throw()
#  endif
#endif

// Assert something at compile time.
#ifndef STATIC_ASSERT
#  if __cplusplus >= 201703L
#    define STATIC_ASSERT(cond) static_assert(cond)
#  else
#    define STATIC_ASSERT(cond) ((void)(int (*)[((cond)? +1 : -1)])0)
#  endif
#endif


// Possible error codes used in MyLibraryError.
enum MyLibraryErrorCode {
  // Used to indicate the absence of an error in certain situations.
  ML_ERR_NO_ERROR = 0,

  // Something in the library logic is wrong, or perhaps the API was
  // misused without that being detected as such.
  ML_ERR_INTERNAL_ERROR,

  // The API was misused by passing data that violates its constraints
  // or invoking functions in an invalid sequence.
  ML_ERR_API_MISUSE,

  // An attempt to read from the input source failed.
  ML_ERR_INPUT_ERROR,

  // This is an examle of a library-specific error condition.
  ML_ERR_NO_RULE_MATCHES,

  // Failed to dynamically allocate more memory when needed.
  ML_ERR_OUT_OF_MEMORY,

  // Count of valid codes.
  ML_NUM_ERROR_CODES
};


// Return a pointer to statically-allocated memory describing 'code' in
// English.
char const *myLibraryErrorString(MyLibraryErrorCode code)
{
  // Table of error messages.
  static char const * const table[] = {
    "no error",
    "internal error",
    "API misused",
    "error reading input source",
    "no rule matches the input text",
    "out of memory"
  };

  // Make sure the table is kept in sync with the enumeration.
  STATIC_ASSERT(sizeof(table) / sizeof(table[0]) == ML_NUM_ERROR_CODES);

  // Catch negatives while also avoiding a possible compiler warning if
  // the underlying type of 'MyLibraryErrorCode' is unsigned.
  if ((unsigned)code < (unsigned)ML_NUM_ERROR_CODES) {
    return table[code];
  }
  else {
    return "invalid error code";
  }
}


// Thrown in response to one of the conditions enumerated in
// MyLibraryErrorCode.
//
// Inheritance is virtual because someone might use multiple inheritance
// to signal that an exception belongs to multiple categories, and then
// catch using the common base class.
class MyLibraryError : virtual public std::exception {
private:     // data
  // This contains the string returned from 'what()' after that
  // method has been called.  Its only purpose is to ensure the
  // 'what()' string gets deallocated.
  mutable char *m_what;

public:      // data
  // The kind of error that occurred.
  MyLibraryErrorCode m_code;

  // Some additional application-specific data that may or may not be
  // relevant, depending on 'm_code'.
  int m_ruleCount;

  // Nullable pointer to static storage containing a human-readable,
  // supplementary detail string.  When present, it refines 'm_code'
  // with some additional context that is not important enough to
  // warrant its own code, but still potentially useful as a hint to a
  // human.  There should be no reason to programmatically inspect it.
  char const *m_detail;

private:     // methods
  void clearWhat() NOEXCEPT;

public:      // methods
  MyLibraryError(MyLibraryErrorCode code, int ruleCount, char const *detail) NOEXCEPT;
  MyLibraryError(MyLibraryError const &obj) NOEXCEPT;
  virtual ~MyLibraryError() NOEXCEPT;

  MyLibraryError& operator=(MyLibraryError const &obj) NOEXCEPT;
  virtual char const *what() const NOEXCEPT;
};


MyLibraryError::MyLibraryError(MyLibraryErrorCode code, int ruleCount,
                               char const *detail) NOEXCEPT
  : std::exception(),
    m_what(NULL),
    m_code(code),
    m_ruleCount(ruleCount),
    m_detail(detail)
{}

MyLibraryError::MyLibraryError(MyLibraryError const &obj) NOEXCEPT
  : std::exception(obj),
    m_what(NULL),            // Do not copy obj.m_what!
    m_code(obj.m_code),
    m_ruleCount(obj.m_ruleCount),
    m_detail(obj.m_detail)
{}

MyLibraryError::~MyLibraryError() NOEXCEPT
{
  clearWhat();
}

void MyLibraryError::clearWhat() NOEXCEPT
{
  if (m_what) {
    delete[] m_what;
    m_what = NULL;
  }
}

MyLibraryError& MyLibraryError::operator=(MyLibraryError const &obj) NOEXCEPT
{
  if (this != &obj) {
    this->std::exception::operator=(obj);

    // We do not copy obj.m_what because of the risk of throwing.  But
    // we can't leave the old data either.
    this->clearWhat();

    this->m_code = obj.m_code;
    this->m_ruleCount = obj.m_ruleCount;
    this->m_detail = obj.m_detail;
  }
  return *this;
}

char const *MyLibraryError::what() const NOEXCEPT
{
  try {
    if (!m_what) {
      if (m_ruleCount == 12345) {
        // For testing purposes, pretend that allocation failed while
        // building the 'what' string.  This code would not be retained
        // in production usage.
        throw std::bad_alloc();
      }

      // Build my message.
      std::ostringstream oss;
      oss << "my library error: " << myLibraryErrorString(m_code);
      if (m_code == ML_ERR_NO_RULE_MATCHES) {
        oss << " (ruleCount=" << m_ruleCount << ')';
      }
      if (m_detail) {
        oss << ": " << m_detail;
      }

      // Transfer it into 'm_what'.
      std::string str = oss.str();
      m_what = new char[str.size()+1];
      std::strcpy(m_what, str.c_str());
    }

    return m_what;
  }
  catch (...) {
    return "my library error (description unavailable due to failure in 'what()' method)";
  }
}


int main()
{
  // Construct and throw a few examples.
  for (MyLibraryErrorCode code = ML_ERR_INTERNAL_ERROR;
       code < ML_NUM_ERROR_CODES;
       code = (MyLibraryErrorCode)(code+1)) {
    try {
      throw MyLibraryError(code, code+12340,
        code==ML_ERR_INTERNAL_ERROR? "buffer marker missing" :
        code==ML_ERR_API_MISUSE?     "cannot pop an empty stack" :
                                     NULL);
    }

    // One of the goals here is to allow a standard catch clause like
    // this one to work for generic error reporting, without having
    // specific knowledge of MyLibraryError.
    catch (std::exception &e) {
      std::cout << e.what() << std::endl;
    }
  }
  return 0;
}

The output from the above code is:

my library error: internal error: buffer marker missing
my library error: API misused: cannot pop an empty stack
my library error: error reading input source
my library error: no rule matches the input text (ruleCount=12344)
my library error (description unavailable due to failure in 'what()' method)

Summary of changes from the version in the previous question:

• I added an enumeration of error codes.

• Inheritance is now virtual.

• Added m_ruleCount.

• Most important: m_what is now a char* that uses explicit new[] and delete[]. It is only populated inside what(). These measures avoid problems with exceptions thrown by std::string.

Answer 1

We need to include <cstring> for std::strcpy.

I think that myLibraryErrorString() would be clearer in terms of switch:

char const *myLibraryErrorString(MyLibraryErrorCode code)
{
    switch (code) {
    case ML_ERR_NO_ERROR: return "no error";
    case ML_ERR_INTERNAL_ERROR: return "internal error";
    case ML_ERR_API_MISUSE: return "API misused";
    case ML_ERR_INPUT_ERROR: return "error reading input source";
    case ML_ERR_NO_RULE_MATCHES: return "no rule matches the input text";
    case ML_ERR_OUT_OF_MEMORY: return "out of memory";
    }
    return "invalid error code";
}

Of course, our compiler will now warn that we don't have a case for ML_NUM_ERROR_CODES, but I think that doesn't belong in the enum and should be removed.

We can then make MyLibraryErrorCode into a enum class, as we shouldn't be doing arithmetic on the values.

clearWhat() doesn't need to test for null pointers, as delete[] behaves reasonably anyway.

We could do with a move constructor that transfers m_what rather than nulling it (but that conflicts somewhat with C++03 compatibility).