Re: Warning from Boost.Exception

From: pbristow_at_[hidden]
Date: 2019-10-16 09:51:00

> -----Original Message-----
> From: Boost <boost-bounces_at_[hidden]> On Behalf Of Alexander Grund via
> Boost
> Sent: 16 October 2019 08:01
> To: boost_at_[hidden]
> Cc: Alexander Grund <alexander.grund_at_[hidden]>
> 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