[C++11] If you have an old class with a throwing destructor...
I just got reminded of a breaking change in C++11 regarding this. Destructors without any exception specification in C++11 get `noexcept(true)` added to them. This means that pre-C++11 classes with throwing destructors turn into `std::terminate` bombs when imported to a C++11 project! I have two^H^H^Hthree such classes in Boost, so I just reported myself. (It's bug #9088, if you're curious.) Any other maintainers as stu^H^H^Henlightened as me should review their classes and add `noexcept(false)` when compiling in C++11 mode. If there are other libraries affected, maybe someone should make an en-masse bug ticket. Daryle W.
On 7 September 2013 08:22, Daryle Walker
I just got reminded of a breaking change in C++11 regarding this. Destructors without any exception specification in C++11 get `noexcept(true)` added to them.
That isn't the rule. It is actually: n3290 12.4p3 Destructors: A declaration of a destructor that does not have an *exception-specification *is implicitly considered to have the same *exception-specification *as an implicit declaration n3290 15.4p14 Exception specifications: If f is an implicitly declared default constructor, copy constructor, move constructor, destructor, copy assignment operator, or move assignment operator, its implicit *exception-specification *specifies the *type-id *T if and only if T is allowed by the *exception-specification *of a function directly invoked by f’s implicit definition; f shall allow all exceptions if any function it directly invokes allows all exceptions, and f shall allow no exceptions if every function it directly invokes allows no exceptions. -- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404
Your analysis is correct, but in practice the problem described by Daryle
still exists. Authors of classes that throw in destructors need to take
some action if they have written their libraries in C++03: either mark
destructors as noexcept(false) or embed a sub-object (member or base class)
that has a noexcept(false) destructor.
Regards,
&rzej
2013/9/9 Nevin Liber
On 7 September 2013 08:22, Daryle Walker
wrote: I just got reminded of a breaking change in C++11 regarding this. Destructors without any exception specification in C++11 get `noexcept(true)` added to them.
That isn't the rule. It is actually:
n3290 12.4p3 Destructors:
A declaration of a destructor that does not have an *exception-specification *is implicitly considered to have
the same *exception-specification *as an implicit declaration
n3290 15.4p14 Exception specifications:
If f is
an implicitly declared default constructor, copy constructor, move constructor, destructor, copy assignment
operator, or move assignment operator, its implicit *exception-specification *specifies the *type-id *T if and only
if T is allowed by the *exception-specification *of a function directly invoked by f’s implicit definition; f shall
allow all exceptions if any function it directly invokes allows all exceptions, and f shall allow no exceptions
if every function it directly invokes allows no exceptions.
-- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On 9 Sep 2013 at 18:41, Andrzej Krzemienski wrote:
Your analysis is correct, but in practice the problem described by Daryle still exists. Authors of classes that throw in destructors need to take some action if they have written their libraries in C++03: either mark destructors as noexcept(false) or embed a sub-object (member or base class) that has a noexcept(false) destructor.
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause. Now it's somewhat mandatory. As an aside, we could really do with some new compiler warnings on this actually. I already get caught enough times forgetting to mark move constructors as noexcept, without which some STL implementations will silently disable move construction completely. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On 9 September 2013 11:52, Niall Douglas
On 9 Sep 2013 at 18:41, Andrzej Krzemienski wrote:
Your analysis is correct, but in practice the problem described by Daryle still exists. Authors of classes that throw in destructors need to take some action if they have written their libraries in C++03: either mark destructors as noexcept(false) or embed a sub-object (member or base class) that has a noexcept(false) destructor.
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
As an aside, we could really do with some new compiler warnings on this actually.
I'm not sure what the warning would be, as move constructors are allowed to throw.
I already get caught enough times forgetting to mark move constructors as noexcept, without which some STL implementations will silently disable move construction completely.
That seems buggy, other than the well-defined cases such as vector using the equivalent of move_if_noexcept under the covers when growing a vector. -- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant: destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } } Indeed I once had some python executed by my build script to auto-insert those destructor wrappers because I kept forgetting to do it. The problem is that unless some code is specifically marked as noexcept, you have to assume it can throw, and as destructors ought to be noexcept, that means lots of try...catch verbiage.
As an aside, we could really do with some new compiler warnings on this actually.
I'm not sure what the warning would be, as move constructors are allowed to throw.
They are, but equally if you think it through an exception throwing move constructor is about as useful as an exception throwing destructor (i.e. somewhat so, but it is wise to choose a different route where possible). I'm willing to bet US$1 that C++ 2x will make move constructors noexcept if everything they call is noexcept ... :) Anyway, it's more a question of good style. GCC has a class of warnings for style, and clang's static analyser is another case example. Those would be ideal for this kind of warning.
I already get caught enough times forgetting to mark move constructors as noexcept, without which some STL implementations will silently disable move construction completely.
That seems buggy, other than the well-defined cases such as vector using the equivalent of move_if_noexcept under the covers when growing a vector.
I think it's buggy if they silently disable move construction and don't tell you (older versions of libstdc++ I'm looking at you). libc++ at least is kind enough to refuse to compile, perhaps even a bit too anally so. Ideally a STL should issue a compiler warning if it refuses to use you move constructor for some reason. It is, after all, a quality of implementation issue. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On 2013-09-09 5:57 PM, Niall Douglas wrote:
Indeed I once had some python executed by my build script to auto-insert those destructor wrappers because I kept forgetting to do it.
This doesn't sound right. You should be more deliberate than that IMO. Can you describe the situation that required this kind of pattern?
On 13-09-09 02:57 PM, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Can you elaborate on what "something useful" is in this context? -- Eric Niebler Boost.org
On 13-09-09 02:57 PM, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Can you elaborate on what "something useful" is in this context?
Log the error, perhaps? Regards, Nate
On Sun, Sep 15, 2013 at 11:20 PM, Nathan Ridge
On 13-09-09 02:57 PM, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Can you elaborate on what "something useful" is in this context?
Log the error, perhaps?
I'd log it in /dev/null. :) -- Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On 9/16/2013 2:20 AM, Nathan Ridge wrote:
On 13-09-09 02:57 PM, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Can you elaborate on what "something useful" is in this context?
Log the error, perhaps?
Sure. And then? Don't say, "Quietly swallow the exception." Something Very Bad has happened. Ignoring it inevitably leads to Something Worse (like data corruption, security vulnerabilities, etc). The only sensible option is to terminate, which is why the language semantics are what they are. The OP's suggestion to litter destructors with try/catch is misguided. That's no way to build reliable systems. -- Eric Niebler Boost.org http://www.boost.org
On Mon, Sep 16, 2013 at 1:47 PM, Eric Niebler
On 9/16/2013 2:20 AM, Nathan Ridge wrote:
On 13-09-09 02:57 PM, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Can you elaborate on what "something useful" is in this context?
Log the error, perhaps?
Sure. And then? Don't say, "Quietly swallow the exception." Something Very Bad has happened.
Nothing bad has happened. If something can fail "badly" you shouldn't do it in the destructor. -- Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Mon, Sep 16, 2013 at 1:47 PM, Eric Niebler
wrote: On 9/16/2013 2:20 AM, Nathan Ridge wrote:
On 13-09-09 02:57 PM, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
> In C++03 it was always best practice to *always* wrap your > destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Can you elaborate on what "something useful" is in this context?
Log the error, perhaps?
Sure. And then? Don't say, "Quietly swallow the exception." Something Very Bad has happened.
Nothing bad has happened. If something can fail "badly" you shouldn't do it in the destructor.
Or at all. "Very Bad" means "no exception safety guarantee", which is another way of saying "do not use". Stated differently, if the ... throwing is Very Bad, it would still be Very Bad if used outside of a destructor. So you shouldn't ever use it.
On 16 Sep 2013 at 16:47, Eric Niebler wrote:
Sure. And then? Don't say, "Quietly swallow the exception." Something Very Bad has happened. Ignoring it inevitably leads to Something Worse (like data corruption, security vulnerabilities, etc). The only sensible option is to terminate, which is why the language semantics are what they are. The OP's suggestion to litter destructors with try/catch is misguided. That's no way to build reliable systems.
I agree that if your program has entered undefined state, you need to fatal exit. Fatal exit != immediate std::terminate() though. There's plenty of very useful things you can do before you die such as flush buffers, kick out memory maps, dump a trace file etc. Regarding the other comments about "keeping destructors simple enough they never throw", sure I agree that if a C++ expert designed and wrote all the code then throws from destructors are likely to not occur. However in the real world destructors call third party code and who knows what they could throw. The truth is that in real world code written by non experts you have to assume that every single line of C++ can *always* throw, unless it is a function marked noexcept or it is a function marked extern "C" (and even then some compilers allow C++ exceptions to traverse C code). I therefore return to my original statement that wrapping destructors in try...catch is both useful and wise. Note I did not state that fatal exit isn't the wisest choice if a destructor does throw. Unless you're bothering to universally write restartable destructors, and far more importantly to bear the maintenance burden of doing so and training in staff to do so, fatal exit on destructor throw is the best course of action. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On 15 September 2013 20:25, Eric Niebler
On 13-09-09 02:57 PM, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Can you elaborate on what "something useful" is in this context?
As well what happens if something in that block throws. -- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404
On 15 Sep 2013 at 18:25, Eric Niebler wrote:
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Can you elaborate on what "something useful" is in this context?
Sure. A bit more than a decade ago I took two years out to implement a next-gen software platform. It conceptualised GUI components as a distributed, adhoc, semantically generated graph of data stream processors, so think of something like Boost.iostreams spread out over disparate compute units and whose relationships were described by a triplestore, and you've got a good idea (if it sounds like Plan 9, you're not wrong). One of the big problems in the client-server relationship was exception propagation. If I ask for an instantiation of an editing processor for a Word file, and I am not on a Microsoft Windows machine, the framework automatically found a nearby machine with the requisite Word COM objects and simply plugged together the needed bits code from the semantic graph database. If the Word COM object threw a COM exception, that got converted into a C++ exception and propagated over the asynchronous RPC channel such that it pops out locally. Because it's asynchronous, it pops out only at certain points i.e. when you next talk to the relevent item (Boost.AFIO does exactly the same BTW). The problem is what happens when exceptions cascade. In the above situation, an exception from COM could cause a chain of bits of intermediate processing to also throw exceptions, and they had a nasty habit of "popping out" via propagation during object destruction as often client code hadn't done anything in between to otherwise retrieve the asynchronous exception state. In this situation, we really *want* exception throws during object destruction to NOT call std::terminate() because no unsafe and ill defined program state has occurred. And hence the solution I mentioned earlier which was a script to add try...catch to all destructors, and if an exception was already in flight, it appended the new exception to the existing one as a nested exception and it retried the destructor. Later on, my additions to the C++ runtime would spot the situation and figure out if the nested exception throws created indeterminate program state - in which case now we fatal exit - or if the destructor retry was successful, and the party can continue. Most would say a better solution would have been the make the throwing destructors private and force people to use a virtual Destroy() function. On the hand, that's not very C++-ish, and my solution did work very nicely so long as you didn't mess up retryable destructors. I also hooked management of the Python GIL into the same framework, so one could traverse between C++ and Python and back into C++ and so on. It worked well. Lots more info on nested exception handling at http://tnfox.sourceforge.net/TnFOX-svn/html/class_f_x_1_1_f_x_exceptio n.html for those really interested. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On 9 September 2013 22:57, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Indeed I once had some python executed by my build script to auto-insert those destructor wrappers because I kept forgetting to do it.
The problem is that unless some code is specifically marked as noexcept, you have to assume it can throw, and as destructors ought to be noexcept, that means lots of try...catch verbiage.
I disagree that littering destructors with try-catch is best practice.
I already get caught enough times forgetting to mark move constructors as noexcept, without which some STL implementations will silently disable move construction completely.
That seems buggy, other than the well-defined cases such as vector using the equivalent of move_if_noexcept under the covers when growing a vector.
I think it's buggy if they silently disable move construction and don't tell you (older versions of libstdc++ I'm looking at you).
You mean pre-C++11 versions? It seems a bit picky to complain about non-conformance in an experimental mode shipped before the standard was finished.
libc++ at least is kind enough to refuse to compile, perhaps even a bit too anally so.
Could you clarify what you're talking about? You're making a lot of pretty vague statements, could you show some code? I would be quite surprised if valid code that compiles with libstdc++ doesn't compile with libc++.
Ideally a STL should issue a compiler warning if it refuses to use you move constructor for some reason. It is, after all, a quality of implementation issue.
No, it's required behaviour, the relevant members of std::vector say "If an exception is thrown other than by the move constructor of a non-CopyInsertable T there are no effects." In order to meet that guarantee a throwing move constructor must not be used if the type is CopyInsertable, i.e. it must "refuse" to use the move constructor, to avoid un-recoverable data loss. Apart from being a bogus warning, it would be rather hard for the library to issue a compiler warning in that situation anyway.
On 16 September 2013 13:12, Jonathan Wakely
On 9 September 2013 22:57, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Indeed I once had some python executed by my build script to auto-insert those destructor wrappers because I kept forgetting to do it.
The problem is that unless some code is specifically marked as noexcept, you have to assume it can throw, and as destructors ought to be noexcept, that means lots of try...catch verbiage.
I disagree that littering destructors with try-catch is best practice.
I already get caught enough times forgetting to mark move constructors as noexcept, without which some STL implementations will silently disable move construction completely.
That seems buggy, other than the well-defined cases such as vector using the equivalent of move_if_noexcept under the covers when growing a vector.
I think it's buggy if they silently disable move construction and don't tell you (older versions of libstdc++ I'm looking at you).
You mean pre-C++11 versions? It seems a bit picky to complain about non-conformance in an experimental mode shipped before the standard was finished.
There were some problems caused by the little-known fact that is_nothrow_move_constructible depends on a noexcept destructor as well as nothrow move constructor and until G++ implemented the implicit noexcept on destructors you needed to explicitly mark your types' destructors as noexcept to make the library do the right thing. That was actually a problem in the G++ front-end, libstdc++ was doing the right thing based on the value of is_nothrow_move_constructible. If the compiler causes the value of is_nothrow_move_constructible to be wrong there's not much the library can do.
On Mon, Sep 16, 2013 at 1:28 PM, Jonathan Wakely
On 16 September 2013 13:12, Jonathan Wakely
wrote: On 9 September 2013 22:57, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote:
In C++03 it was always best practice to *always* wrap your destructors in a try...catch clause.
No; the best practice is not to throw from a destructor.
Sorry, my unclear phrasing again ... by wrapping destructors I specifically meant:
destructor::~destructor() { try { ... } catch(...) { /* do something useful */ } }
Indeed I once had some python executed by my build script to auto-insert those destructor wrappers because I kept forgetting to do it.
The problem is that unless some code is specifically marked as noexcept, you have to assume it can throw, and as destructors ought to be noexcept, that means lots of try...catch verbiage.
I disagree that littering destructors with try-catch is best practice.
The use of an explicit try-catch block is rarely the optimal solution in a destructor. I don't think we should consider recommending this practice. I am open to exploring the potential solutions involving decorating the destructor declarations/definitions to improve behaviour across compiler versions. I am unclear about how clever the C++11 has_trivial_destructor meta-functions are. Do they generally work when an explicitly defined noexcept destructor is declared? My opinion is that we should provide the support for broken compiler versions if and only if the additional verbiage doesn't cause a performance regression for correct compilers.
I already get caught enough times forgetting to mark move constructors as noexcept, without which some STL implementations will silently disable move construction completely.
That seems buggy, other than the well-defined cases such as vector using the equivalent of move_if_noexcept under the covers when growing a vector.
I think it's buggy if they silently disable move construction and don't tell you (older versions of libstdc++ I'm looking at you).
You mean pre-C++11 versions? It seems a bit picky to complain about non-conformance in an experimental mode shipped before the standard was finished.
There were some problems caused by the little-known fact that is_nothrow_move_constructible depends on a noexcept destructor as well as nothrow move constructor and until G++ implemented the implicit noexcept on destructors you needed to explicitly mark your types' destructors as noexcept to make the library do the right thing. That was actually a problem in the G++ front-end, libstdc++ was doing the right thing based on the value of is_nothrow_move_constructible. If the compiler causes the value of is_nothrow_move_constructible to be wrong there's not much the library can do.
Additionally it doesn't seem that there is much that we Boost developers can do for the broken versions without deteriorating the results for working versions. I therefore vote for doing nothing! Neil Groves
On 16 September 2013 14:54, Neil Groves wrote:
The use of an explicit try-catch block is rarely the optimal solution in a destructor. I don't think we should consider recommending this practice. I am open to exploring the potential solutions involving decorating the destructor declarations/definitions to improve behaviour across compiler versions. I am unclear about how clever the C++11 has_trivial_destructor meta-functions are. Do they generally work when an explicitly defined noexcept destructor is declared?
I don't know about in general, but libstdc++'s does, yes. It's
basically implemented in terms of noexcept(std::declval
On Mon, Sep 16, 2013 at 5:12 AM, Jonathan Wakely
On 9 September 2013 22:57, Niall Douglas wrote:
On 9 Sep 2013 at 13:11, Nevin Liber wrote: The problem is that unless some code is specifically marked as noexcept, you have to assume it can throw, and as destructors ought to be noexcept, that means lots of try...catch verbiage. I disagree that littering destructors with try-catch is best practice.
Littering your code with throwing destructors is not the best practice, either. The simple rule to follow is: don't ever throw in destructors. In practice this means that destructors should free resources and do nothing else. That way you don't litter destructors with try catch(...), you use this only as a last resort means to prevent a destructor from throwing. -- Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
participants (11)
-
Andrzej Krzemienski
-
Daryle Walker
-
Emil Dotchevski
-
Eric Niebler
-
Jonathan Wakely
-
Nathan Ridge
-
Neil Groves
-
Nevin Liber
-
Niall Douglas
-
Peter Dimov
-
Sohail Somani