Re: Warning from Boost.Exception

From: Alexander Grund (alexander.grund_at_[hidden])
Date: 2019-10-15 09:42:55

Am 15.10.19 um 11:14 schrieb Andrey Semashev via Boost:
> On 2019-10-15 12:06, Alexander Grund via Boost wrote:
>> Then it looks like the warning is indeed correct: 
>> https://github.com/boostorg/throw_exception/blob/e2e802e5085e10db7d5ee5ded4c80f663591e976/include/boost/exception/exception.hpp#L168 
>>
>>
>> `error_info_container` has a non-virtual dtor although it does have 
>> other virtual methods and the derived class has a `delete this` but 
>> is not final. Hence the compiler can't be sure that there isn't 
>> another derived class.
>>
>> Any reason to NOT make the dtor virtual? Performance degrade can be 
>> resolved by making the derived class final (where supported which 
>> should be almost everywhere nowadays)
>
> The warning is bogus because the object is never destroyed through the 
> base class. Marking the destructor virtual would suggest that that is 
> possible. And add a virtual call where none is needed.
The warning is actually correct as one *could* have a class derived 
from`error_info_container_impl` which would then be deleted through a 
pointer to `error_info_container_impl` only.
The destructor of `error_info_container` is protected, which is good. 
However the one of `error_info_container_impl` is not. It should 
probably be even private (if all deletes happen through its `release()` 
function)

So in the current form one (the compiler and a human) cannot judge if 
there are derived objects and how they are destroyed.

Of course you are right that this warning is a false-positive in this 
case, but it's hard/impossible to tell. That's why I suggested a virtual 
dtor, which avoids the problem, and a final class, which "devirtualizes" 
the call again.

> Marking error_info_container_impl final should help, but we currently 
> don't have a macro for that.
This would make the code correct (to tell that it is correct). Shame 
that there is no macro yet, so thanks for the PR.