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