[boost] [review] Review of Exception

From: Chris Newbold (Chris.Newbold_at_[hidden])
Date: 2007-10-09 18:31:22

On Thu, 2007-09-27 at 17:43 -0400, Tobias Schwinger wrote:

> The formal review of the Exception library, proposed by Emil
> Dotchevski, begins today.
> 
> The library provides an exception class that can transport arbitrary
> values, thus freeing the user from designing custom exception classes.

Here's my review of the proposed Exception library; apologies for not
being in the "standard" format. I'm just now reading through the other
Exception review feedback, so some of my concerns and questions may
already have been addressed...

====

My over-all impression of the Exception library is very favorable. It
provides an elegant solution to the problem of collecting "supporting
details" in generic fashion as an exception propagates; it would be
immediately useful to us.

My evaluation consisted of a detailed reading of the documentation and a
casual inspection of the code.

Some areas of concern and specific suggestions:

Thread safety. The documentation does not specify what guarantees (or
lack there of) the library makes about threading or re-entrancy. I think
this is crucial for a library like Exceptions. I think a reasonable
level of thread-safety would be to support the concurrent
creation/throwing/catching/manipulating distinct instances of
boost::exception from multiple threads. Identifier registration should
also be thread-safe. Concurrent operations on a single instance of
boost::exception need not be thread safe. On my casual inspection of the
code, I did not see anything which would preclude supporting such a
guarantee.

Introspection. boost::error_info<> allows me to extract a specific piece
of data presuming I know the corresponding identifier.
boost::exception::what() has some support for enumerating all of the
contained values in a generic manner. I would like to have an explicit
interface which I could use to enumerate the values contained within a
boost::exception without prior knowledge of their types. Some sort of
iterator-style interface, perhaps leveraging boost::any to return the
values, for example.

Duplicate identifiers. It was not clear from reading the documentation
what the behavior of the Exception library is when an attempt is made to
store a value into boost::exception using an identifier for which a
value already exists. Reading the code reveals that the original value
will be silently overwritten. It might be beneficial to have a richer
API here, perhaps with "checked" and "unchecked" insertion to allow
clients to catch cases where a value will be overwritten.

std::exception. I think the rationale for not deriving from
std::exception is sound, but it's worth pointing out that the concerns
about duplicate std::exception instances could be resolved by having
boost::exception derive from std::exception using virtual inheritance.

Support for polymorphic cloning/rethrowing. We often have the need to
clone and rethrow polymorphic exceptions; this is useful, for example,
to "transport" exceptions from one thread to another. There are a number
of competing proposals before the ANSI C++ standards committee in this
area, including N2229, "Cloning and Throwing Dynamically Typed
Exceptions" (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2229.html). It would be nice to see some level of support for this sort of functionality in the Exceptions library.

Chaining. Sometimes there is a need to catch one exception type and
throw a new type instead; this can often happen at the boundaries
between different libraries, for example. Though a new exception type
must be thrown, you'd rather not lose the information contained in the
original exception: this can be captured by chaining the exceptions
together. "This exception was caused by that exception." Chaining can
certainly be expressed with the proposed Exception library by creating
the appropriate identifier and storing the root cause inside the new
exception. But I'd be interested in some discussion about explicit
support for chaining.

-Chris