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.