[boost exception]complaint
In the course of investigating some issues with the serialization library I had occasion to investigate boost/throw exception.hpp . I understood the original purpose of this header was to provide support for throw for both compilers which implemented exceptions and those that didn't (e.g. some embedded systems environment). Looking through this header - it seems that its function has changed completely. It now drags in a lot of machinery that no one who was using it before will need. It seems that the whole purpose of the header has been changed. I did review the documentation for boost exception and indeed, it now seems that boost/throw_exception does implement a different purpose. Besides imposing the requirement for RTTI - I have no idea what else this might break. Should all users of throw_exception be advised to recode? Could we not restore the orginal boost/throw_exception and make a differently named file like boost/exception/???.hpp. This would provide all the benefits of the new library to those who want and require them without requiring changes all over the rest of boost. Robert Ramey
Robert Ramey wrote:
In the course of investigating some issues with the serialization library I had occasion to investigate boost/throw exception.hpp .
I understood the original purpose of this header was to provide support for throw for both compilers which implemented exceptions and those that didn't (e.g. some embedded systems environment).
Looking through this header - it seems that its function has changed completely. It now drags in a lot of machinery that no one who was using it before will need. It seems that the whole purpose of the header has been changed. I did review the documentation for boost exception and indeed, it now seems that boost/throw_exception does implement a different purpose.
Besides imposing the requirement for RTTI - I have no idea what else this might break. Should all users of throw_exception be advised to recode? Could we not restore the orginal boost/throw_exception and make a differently named file like boost/exception/???.hpp. This would provide all the benefits of the new library to those who want and require them without requiring changes all over the rest of boost.
As I understand it, the change of boost/throw_exception.hpp was proposed and discussed and finally - by apparent majority (as in, most people who spoke up were for it) - accepted before it was implemented. Also, I believe there was talk that there would be a simple preprocessor macro to revert to the old behaviour. Sebastian
Sebastian Redl wrote:
As I understand it, the change of boost/throw_exception.hpp was proposed and discussed and finally - by apparent majority (as in, most people who spoke up were for it) - accepted before it was implemented. Also, I believe there was talk that there would be a simple preprocessor macro to revert to the old behaviour.
If I had been aware of this discussion, I would have raised the issue at the time. I can't follow them all. Sorry about that. It looks like for some compilers, the preprocessor macros alter the behavior to disabled - even though those compilers actually can throw exceptions so the new behavior is even more complex and arbitrary than I thought. Its hard to believe that a majority thought this was a good idea. Not that that's relevant. It's still a bad idea. Robert Ramey
Quoting Robert Ramey <ramey@rrsd.com>:
It looks like for some compilers, the preprocessor macros alter the behavior to disabled - even though those compilers actually can throw exceptions so the new behavior is even more complex and arbitrary than I thought.
Are you sure? To make sure we are on the same page, I am looking at: http://svn.boost.org/trac/boost/browser/branches/release/boost/throw_excepti... An exception is not thrown if and only if BOOST_NO_EXCEPTIONS is defined. This has always been the case. It sounds like you are interpreting "BOOST_EXCEPTION_DISABLE defined" to mean "exception not thrown" but that's not the case. It merely disables the new functionality the Boost.Exception library.
Quoting Robert Ramey <ramey@rrsd.com>:
In the course of investigating some issues with the serialization library I had occasion to investigate boost/throw exception.hpp .
I understood the original purpose of this header was to provide support for throw for both compilers which implemented exceptions and those that didn't (e.g. some embedded systems environment).
Looking through this header - it seems that its function has changed completely. It now drags in a lot of machinery that no one who was using it before will need. It seems that the whole purpose of the header has been changed. I did review the documentation for boost exception and indeed, it now seems that boost/throw_exception does implement a different purpose.
Besides imposing the requirement for RTTI -
I'm not sure that is the right characterization. By default Boost works with standard C++ so if RTTI is turned off, then it is fair to require the BOOST_NO_TYPEID define. Changes such as this which potentially change "latent" bugs in client code/setup being turned into visible bugs have to be acceptable when changing Boost version, IMO.
I have no idea what else this might break.
It will break people using throw_exception but throwing things not derived from std::exception. This was an implicit requirement anyway, but users will have previously got away with it if they never actually exercised their code on a no-exceptions platform. I can't think of any other problems, though. What did you have in mind? IMO, the major issue with this change is the lack of documentation. Specifically, http://www.boost.org/doc/libs/1_36_0/libs/utility/throw_exception.html seems to refer to the 1.35 version of the code. I don't know if this a documentation bug or a documentation generation bug. Pete
2008/8/20 Peter Bartlett <pete@pcbartlett.com>:
IMO, the major issue with this change is the lack of documentation. Specifically,
http://www.boost.org/doc/libs/1_36_0/libs/utility/throw_exception.html
seems to refer to the 1.35 version of the code. I don't know if this a documentation bug or a documentation generation bug.
Probably a documentation bug. The 1.36 documentation is at: http://www.boost.org/doc/libs/1_36_0/libs/exception/doc/boost-exception.html I expect the utility documentation should be changed to note that exception handling is no longer part of it and link to the new documentation. Daniel
Peter Bartlett wrote:
Quoting Robert Ramey <ramey@rrsd.com>:
I'm not sure that is the right characterization. By default Boost works with standard C++ so if RTTI is turned off, then it is fair to require the BOOST_NO_TYPEID define. Changes such as this which potentially change "latent" bugs in client code/setup being turned into visible bugs have to be acceptable when changing Boost version, IMO.
My complaint is the imposition of a new requirement. This changes the "contract" between the library and its user. You can't change the contract and say that the code which used the old contract had "latent" bugs in it.
I have no idea what else this might break.
It will break people using throw_exception but throwing things not derived from std::exception. This was an implicit requirement anyway,
What is an "implicit requirement"? You mean that previous users of boost/throw_exception where remiss if they didn't derive from std::exception or if they didn't surround throw_exception with #if BOOST_NO_TYPEID. I like to think I'm a smart guy but I don't claim to be telepathic.
but users will have previously got away with it if they never actually exercised their code on a no-exceptions platform.
Hmmm - It looks to me that the previous code would work fine on a no-exeptions platform - all it did was call a user defined throw_exception function.
I can't think of any other problems, though. What did you have in mind?
LOL, That's the problem. I can't think of any problems because to do so would require investigating the new library in more detail would cost me more time than I have available right now. This was not a problem then throw_exception had a very limited and well defined purpose. The real problem is creating a new library with new functionality and requirements and assigning to an old name which heretofore was being used for something else. This kind of thing creates a lot of unnecessary work for innocent bystanders.
IMO, the major issue with this change is the lack of documentation. Specifically,
http://www.boost.org/doc/libs/1_36_0/libs/utility/throw_exception.html
seems to refer to the 1.35 version of the code. I don't know if this a documentation bug or a documentation generation bug.
well, there is that as well.
Robert Ramey:
What is an "implicit requirement"? You mean that previous users of boost/throw_exception where remiss if they didn't derive from std::exception
Yes, they were. This was often caused by silent bugs - throwing enums instead of the appropriate exception type, in one case.
or if they didn't surround throw_exception with #if BOOST_NO_TYPEID.
No, throw_exception is not supposed to be surrounded by #if BOOST_NO_TYPEID, and it would in any case be incorrect to do so. What it _is_ supposed to do is work and not cause problems for people unaware of Boost.Exception, and it will hopefully be fixed accordingly. For this to happen, Boost.Config needs to acquire a BOOST_NO_RTTI macro. Incidentally, g++ 4.3 now seems to have a predefined __GXX_RTTI macro that we can use to autodetect that BOOST_NO_TYPEID and BOOST_NO_RTTI need to be set (earlier g++ versions did not).
Hmmm - It looks to me that the previous code would work fine on a no-exeptions platform - all it did was call a user defined throw_exception function.
No, it would fail to compile. The user-defined function took an std::exception argument. A fundamental issue is that there are many configurations relevant to our users that we don't test. -Wshadow -Wundef is one, no-exceptions is another, no-RTTI is a third one. Speaking as a lib maintainer, I try to keep the libraries working under these conditions, but without test results, it's not easy to do so.
on Wed Aug 20 2008, "Peter Dimov" <pdimov-AT-pdimov.com> wrote:
A fundamental issue is that there are many configurations relevant to our users that we don't test. -Wshadow -Wundef is one, no-exceptions is another, no-RTTI is a third one. Speaking as a lib maintainer, I try to keep the libraries working under these conditions, but without test results, it's not easy to do so.
Hi Peter, I think that's an important point. Maybe you should open a ticket so it persists until we're brave enough to deal with it :-) -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams:
on Wed Aug 20 2008, "Peter Dimov" <pdimov-AT-pdimov.com> wrote:
A fundamental issue is that there are many configurations relevant to our users that we don't test. -Wshadow -Wundef is one, no-exceptions is another, no-RTTI is a third one. Speaking as a lib maintainer, I try to keep the libraries working under these conditions, but without test results, it's not easy to do so.
Hi Peter,
I think that's an important point. Maybe you should open a ticket so it persists until we're brave enough to deal with it :-)
Tickets, I find, are only effective if they are assigned to a specific person. ;-)
on Thu Aug 21 2008, "Peter Dimov" <pdimov-AT-pdimov.com> wrote:
David Abrahams:
on Wed Aug 20 2008, "Peter Dimov" <pdimov-AT-pdimov.com> wrote:
A fundamental issue is that there are many configurations relevant to our users that we don't test. -Wshadow -Wundef is one, no-exceptions is another, no-RTTI is a third one. Speaking as a lib maintainer, I try to keep the libraries working under these conditions, but without test results, it's not easy to do so.
Hi Peter,
I think that's an important point. Maybe you should open a ticket so it persists until we're brave enough to deal with it :-)
Tickets, I find, are only effective if they are assigned to a specific person. ;-)
If they they are assigned a component then they are automatically assigned to _somebody_ specific. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
Quoting Robert Ramey <ramey@rrsd.com>:
I have no idea what else this might break.
It will break people using throw_exception but throwing things not derived from std::exception. This was an implicit requirement anyway,
What is an "implicit requirement"? You mean that previous users of boost/throw_exception where remiss if they didn't derive from std::exception or if they didn't surround throw_exception with #if BOOST_NO_TYPEID. I like to think I'm a smart guy but I don't claim to be telepathic.
Why would you need to surround throw_exception with #if BOOST_NO_TYPEID? The template function works whether it is defined or not...
but users will have previously got away with it if they never actually exercised their code on a no-exceptions platform.
Hmmm - It looks to me that the previous code would work fine on a no-exeptions platform - all it did was call a user defined throw_exception function.
At the very least the fact that the declaration for that case in boost/throw_exception was throw_exception( std::exception ) was at least a hint that it was intended for use with classes deriving from std::exception. Admittedly the client could crack open the boost namespace and add another overload for their own exception type(s), and include that before boost/throw_exception.hpp. Perhaps this use case was not sufficiently thought about in prior discussions. For me though the benefits of enabling Boost.Exception functionality for all existing code that uses boost::throw_exception outweighs that cost.
On Wed, Aug 20, 2008 at 10:07 AM, Robert Ramey <ramey@rrsd.com> wrote:
Peter Bartlett wrote:
Quoting Robert Ramey <ramey@rrsd.com>:
I'm not sure that is the right characterization. By default Boost works with standard C++ so if RTTI is turned off, then it is fair to require the BOOST_NO_TYPEID define. Changes such as this which potentially change "latent" bugs in client code/setup being turned into visible bugs have to be acceptable when changing Boost version, IMO.
My complaint is the imposition of a new requirement. This changes the "contract" between the library and its user. You can't change the contract and say that the code which used the old contract had "latent" bugs in it.
I have no idea what else this might break.
It will break people using throw_exception but throwing things not derived from std::exception. This was an implicit requirement anyway,
What is an "implicit requirement"? You mean that previous users of boost/throw_exception where remiss if they didn't derive from std::exception or if they didn't surround throw_exception with #if BOOST_NO_TYPEID. I like to think I'm a smart guy but I don't claim to be telepathic.
The implicit requirement was that any exception passed to boost::throw_exception must derive from std::exception. This has always been a requirement but it wasn't enforced before. It is enforced now and this did catch a few violations of this requirement.
but users will have previously got away with it if they never actually exercised their code on a no-exceptions platform.
Hmmm - It looks to me that the previous code would work fine on a no-exeptions platform - all it did was call a user defined throw_exception function.
Which takes std::exception. That's why it is required to derive from std::exception.
I can't think of any other problems, though. What did you have in mind?
LOL, That's the problem. I can't think of any problems because to do so would require investigating the new library in more detail would cost me more time than I have available right now.
The reason why the library integrates with boost::throw_exception is to allow the user to take advantage of the new facilities without requiring changes in any library that throws exceptions. Granted, for something as central as throw_exception, this has to be done very carefully and I think your concern is that this wasn't the case. Note that we don't have test failures in any library due to this new behavior of boost::throw_exception. Of course, if you can think of any problems or potential problems I'm very much interested to know about them.
This was not a problem then throw_exception had a very limited and well defined purpose. The real problem is creating a new library with new functionality and requirements and assigning to an old name which heretofore was being used for something else. This kind of thing creates a lot of unnecessary work for innocent bystanders.
I'll ignore the unsubstantiated claims and I'll just point out that boost::throw_exception has the same purpose it did before: it throws the exception you pass to it. The only difference is that now any exception that emanates from Boost can be transported between threads and can transport any data to the catch site. Consider that without the new boost::throw_exception behavior, for a user to take advantage of the new Exception library, it would be required to change all Boost libraries that throw exceptions.
IMO, the major issue with this change is the lack of documentation. Specifically,
http://www.boost.org/doc/libs/1_36_0/libs/utility/throw_exception.html
seems to refer to the 1.35 version of the code. I don't know if this a documentation bug or a documentation generation bug.
well, there is that as well.
The function is well documented, see http://www.boost.org/doc/libs/1_36_0/libs/exception/doc/throw_exception.html. Even if the new documentation was not available, the old documentation is 100% accurate because the semantics of boost::throw_exception have not changed at all. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On Wed, Aug 20, 2008 at 10:07 AM, Robert Ramey <ramey@rrsd.com> wrote:
What is an "implicit requirement"? You mean that previous users of boost/throw_exception where remiss if they didn't derive from std::exception or if they didn't surround throw_exception with #if BOOST_NO_TYPEID.
The semantics of boost::throw_exception have not changed. Users are not required to surround a call to throw_exception with #if BOOST_NO_TYPEID. The user-reported problem was due to user error. They had violated the general Boost requirement to #define BOOST_NO_TYPEID if they don't want Boost libraries to use typeid. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Wed, Aug 20, 2008 at 10:07 AM, Robert Ramey <ramey@rrsd.com> wrote:
What is an "implicit requirement"? You mean that previous users of boost/throw_exception where remiss if they didn't derive from std::exception or if they didn't surround throw_exception with #if BOOST_NO_TYPEID.
The semantics of boost::throw_exception have not changed.
That's where we differ. It seems to me that on certain platforms things won't work as they had before. Of course if I'm wrong about that, the my complaint is ill founded.
Users are not required to surround a call to throw_exception with #if BOOST_NO_TYPEID.
The user-reported problem was due to user error. They had violated the general Boost requirement to #define BOOST_NO_TYPEID if they don't want Boost libraries to use typeid.
Hmm - I wasn't aware of any such requirement. I would expect something like that to be defined by the config library. Hmmm, so now we have "implicit requirements" and "general requirements" that we need to be cognisant of to use boost? This seems like a lot to ask. Robert Ramey
On Wed, Aug 20, 2008 at 12:46 PM, Robert Ramey <ramey@rrsd.com> wrote:
Emil Dotchevski wrote:
On Wed, Aug 20, 2008 at 10:07 AM, Robert Ramey <ramey@rrsd.com> wrote:
What is an "implicit requirement"? You mean that previous users of boost/throw_exception where remiss if they didn't derive from std::exception or if they didn't surround throw_exception with #if BOOST_NO_TYPEID.
The semantics of boost::throw_exception have not changed.
That's where we differ. It seems to me that on certain platforms things won't work as they had before.
If that's true, it's a bug and I'll fix it. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
participants (7)
-
Daniel James -
David Abrahams -
Emil Dotchevski -
Peter Bartlett -
Peter Dimov -
Robert Ramey -
Sebastian Redl