[boost] [review] Exception

From: Pavel Vozenilek (pavel_vozenilek_at_[hidden])
Date: 2007-10-18 11:23:35

I think the library should be included in Boost because ... well, I use it 
and found it very convenient in my code.

-------------------------------
There are several situation where this library may come very handy (expected 
use cases should be mentioned in the docs, btw):

1. As a replacement of ad-hoc and messy network of exception classes with 
something less complicated (this is why I use it).  This, in turn, makes 
unit tests more stable and thus easier to write.

2. As a unified solution for large systems where user codes the lowest level 
bits, the middle level is provided as a framework and highest part is again 
coded by the user. Without solution like boost::exception one would need to 
accomodate with exceptions to the framework, not the other way around. Here, 
the framework would not dictate what to do.

3. The library has a potential to deal with exception handling in a 
structured way.

Say there's low level module A, middle level module B and high level module 
C. Module B uses A, C uses B. Some tool can be implemented that processes or 
converts all exceptions thrown from A *inside* B so module C will only see 
interface exposed from B and never ever anything from A.

This library would allow to implement such handling in a single place, to 
implement automated checking and would allow to output detailed information 
in case of programmer mistake. It should eliminate need for chaining 
exceptions which still exposes lower level details

It would be useful for this purpose to provide an ability to iterate through 
the values or ability to pass a visitor or both.

-------------------------------
Notes about the code:

1. exception/exception.hpp (btw, this file name can easily confuse one with 
the same filename one level down) - assert() may be replaced with 
BOOST_ASSERT or even better commented out. People may use their own assert 
implementation and this would make the exception module (something what 
should be really the core part) dependent on something else.

2. shared_ptr<> may be replaced by intrusive_ptr<> or home made equivalent. 
This (a) reduces dependency on other Boost part (shared_ptr does depend on 
quite few other libs) and (b) the atomic locking used by shared_ptr would be 
eliminated. This would help a little bit on multiprocessor systems - lock 
may be hundredths of cycles and during the time access to the memory bus is 
disabled.

3. The part of code in exceptions/exception.hpp that may be put into 
separate TU could be wrapped in
#ifdef BOOST_EXCEPTION_THIS_IS_INSERTED_INTO_SEPARATE_CPP_FILE
...
#endif

Someone may have Boost as read-only files and/or be using SVN version and 
unwilling to do it again and again.

4. The boost/exception.hpp may have

#if (defined _MSC_VER) && (_MSC_VER >= 1200)
#  pragma once
#endif

to help a tiny bit with compilation times.

-------------------------------
Notes about the docs:

1. Section Logging may provide examples how to implement pretty-printing of 
the user data.

2. The documentation may say whether it is possible to use multiple 
inheritance with boost::exception descendants and show how. I tried it some 
time ago, it didn't compile so I gave up.

3. The example using existing exception class hierarchy should be longer and 
show how one can catch such exception. It may be obvious but it's better 
documented explicitly.

4. The example and the tests from libs/exception may be linked with short 
narrative what one can see here. There's never enough of examples.

-------------------------------
Other possible features:

1. As mentioned above, the ability to iterate through values and/or pass a 
visitor.

2. Cloning of the exception. When I catch exception thrown in a worker 
thread I create clone of it and throw it in the thread which requested the 
task to be executed in separate thread. It looks like:

struct my_exc : public boost::exception
{
  virtual my_exc* clone() { return new *this; }
};

I'd created common abstract subclass because of it, perhaps it would make 
sense somehow to support the feature.

I see the http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2179.html 
now but it doesn't help me here and now, I think.

3. My code often looks like:

throw exc::some_exception() << boost::error_info<exc::tags::my_tag1>(errno) 
<< boost::error_info<exc::tags::my_tag2>("this") << 
boost::error_info<exc::tags::my_tag3>("that") ;

IOW, very long and hard to read lines. I would welcome some way to fill in 
the exception in more compact form. I, however, do not have any suggestion 
how to do it.

4. I would like the ability to collect traces generated by what() function 
in DEBUG mode, something as:

catch (my_low_level_exception& e) {
   my_high_level_exception e2;
   ... fill in e2
   e2.add_debug_trace(e);  // adds e.what() somewhere, no-op in release mode
   throw e2;
}

...

catch (my_high_level_exception& e) {
  cout << e.dump_collected_debug_trace(); // no-op in release
}

This would print complete debug information yet not expose any low level 
details.

-------------------------------

Some time ago (Aug 17, 2007, in an ancient thread "Review Request: Boost 
Exception") Daryle Walker asked few questions about the library:

1.  What happens if a "boost::error_info" tag is used multiple times on
the same object?  Is it an error (exception or assert?), do the
instances get combined like a multi-set, or do all instances besides the
first (or last) get ignored/dropped?  Shouldn't this be documented?

2.  Shouldn't all inheritance of "boost::exception," especially via
"enable_error_info," be of the virtual kind?  Catching a given class
type only works if it has exactly one connection to the actual exception
object, which means that:

 a. The type is the final type of the object
 b. The type appears exactly once in the inheritance hierarchy
 c. All inheritance appearances of the type are virtual

If a class type uses another type as a base multiple times, and at least
one appearance is _not_ virtual, then catches with that base will be
skipped when that first type is thrown.  Imagine what happens if
"enable_error_info" is applied multiple times during an exception
session and/or the original exception object used "boost::exception".

-------------------------------

About the compile safety of tags: I prefere to have them statically checked. 
Acesibility of the tags could be limited only for relevant parts of an 
application thus removing chance to depend on such internal details in 
urelated code. String based indentification, if used, should be an option, 
not mandatory.

-------------------------------

It would be ideal, if the library is accepted into Boost that all currently 
thrown exception from Boost libraries (e.g. archive_exception from 
Serialization) descend from boost::exception. (I am bot holding my breath, 
this is just an unreachable ideal.)

/Pavel