Re: Warning from Boost.Exception

From: Alexander Grund (alexander.grund_at_[hidden])
Date: 2019-10-17 07:21:41

Am 16.10.19 um 19:38 schrieb Emil Dotchevski via Boost:
> The destructor is not virtual by design, not by mistake. The code is
> correct as written, so the warning is to be silenced.

I think we already agreed that the warning is a false positive in this case.
Only thing I'd complain about the current change: It silences the 
warning for the whole file, not only the class.

>>So I favour implementing BOOST_HAS_FINAL and using it in the code.
>>

>The problem is that this is not a solution for old code, since it may be
>used with old compiler versions, which requires the warning to be silenced
>anyway. Once the warning is silenced, adding final would not be needed.
>Can't hurt though.

Yes and no. The base class is "designed to not be deleted from outside" 
by making the dtor protected. All good here.

The derived class has a public dtor and is not final (and has no comment 
that one should not derive from it). This opens room for the bug the 
compiler (GCC/Clang) warns you about.
Solution a) Make the class final (the upcoming BOOST_FINAL also serves 
as a "comment" to programmers even though it has no effect on older 
compilers)
Solution b) Additionally make the destructor private (iff `release()` is 
the only valid way to delete the class)

Note that this is needed to avoid *future* bugs by documenting (in code) 
the expected usage in a way the programmer AND (newer) compilers do 
understand.

My argument for making the base class destructor virtual was this:
- Code is safe by default even after refactoring (e.g. for whatever 
reason:`void release(){ ...; error_container* c = this; ...; delete c; }` )
- Adding `final` will remove the virtual destructor call in current 
code,n making the code fast
- No warning suppression is needed
- Counter argument: On compilers not supporting "final" this will be a 
little bit slower on destruction