-----Original Message----- From: Boost
On Behalf Of Alexander Grund via Boost Sent: 16 October 2019 08:01 To: boost@lists.boost.org Cc: Alexander Grund Subject: Re: [boost] Warning from Boost.Exception Am 15.10.19 um 22:45 schrieb Emil Dotchevski via Boost:
The warning is not correct, and making the destructor virtual is wrong, because by design it is a bug to delete the base pointer type. I just disabled the warning for MSVC.
The warning is "Virtual class has non-virtual destructor so deleting it through a base class will lead to bugs", which is correct, isn't it? As mentioned: Neither the compiler nor a reviewer can confirm that a delete only ever happens through the most derived pointer because while error_info_container has a protected destructor (and hence cannot be deleted through a pointer to that class from outside), the error_info_container_impl has a public destructor and deletes itself through a this pointer. So if at any point in time someone derives from that class (which the missing final allows and there is no comment advising against that) you will have a bug.
Don't get me wrong: I agree that it is not a bug here and everything is working as intended *currently*. I'm just saying that the warning is valid, and the Clang/GCC(?) warning is even more helpful in suggesting to make this class error_info_container_impl which it should be according to the design.
While writing this: It seems MSVC already warns for the base class -.- Again this is a valid warning, but can be ignored for this specific class because the dtor is protected. I'm not a friend of globally disabling a warning (for the whole file) without an explanation. I'd expected it around error_info_container with a comment like `//Never deleted through base class pointer` as a note to future devs.
As the OC (Original Complainant) can I comment that * We are all agreed that this is a false positive warning. * Warning messages are bad - because of wasted efforts in producing them, and being confusing to the users. * Global disabling of warnings is a NONO. * Local quieting is tricky because so many compilers are involved, and their mechanisms are different. * Adding code comment on why, with links to this discussion is very helpful. * Making the code C++ Standard compliant is always best. Since the Standard seems to suggest to me, (ill-informed on this language lawyers delight) that using keyword final where implemented, that that is the Right Thing To Do long-term. So I favour implementing BOOST_HAS_FINAL and using it in the code. Paul Paul A. Bristow Prizet Farmhouse Kendal, Cumbria LA8 8AB UK