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