[Development] override keyword on destructors

[Ville Voutilainen]

On 20 August 2018 at 16:58, Kevin Funk via Development
<development at qt-project.org> wrote:
> On Monday, 20 August 2018 14:08:36 CEST Sérgio Martins via Development wrote:
>> Hi,
>>
>>
>> Looks like some 'override' keywords crept into a few destructors. This
>> is probably because clang-tidy warns about it (and now QtCreator).
>>
>> IMO we should avoid it, as it's misleading. Dtors are a special case and
>> have completely different semantics. They don't replace their base class
>> dtors. They're chained instead.
>>
>> This is not 100% consensual, some people like to use it.
>>
>> But it's discouraged by the Cpp Core Guidelines [1]; gcc's
>> -Wsuggest-override doesn't suggest it for dtors and neither does clang's
>> -Winconsistent-missing-override.
>
> Heya,
>
> Just as a side note: There's still a warning which will warn for inconsistent
> overrides on destructors in Clang:
>   https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg50989.html
>
> (off by default though, it was noted it's too noisy for existing code bases)
>
>
> To be honest, I find the explanation on the Cpp Core Guidelines wiki pretty
> poor, if not to say non-existent. If you read through the discussion of
> amending of rule C.128 [1] it becomes pretty clear there's no consensus at
> all, but rather one person (gdr-at-ms) trying to push through his preference
> and shutting down the discussion.
>
> I think the destructors-are-chained argument is a bit weak; to me 'override'
> rather translates to "I expect there to exist a base class version of this
> function which is declared virtual" (also cf. [2]), which applies very well to
> destructors. IMO, it would also be nice to get a compiler warning/error if a
> base class' destructor is changed from virtual to non-virtual which may cause
> subtle behavioral changes such as memory leaks.

Right; that's the rationale from Google for marking destructor
overrides. If you have

struct B {virtual ~B();};

struct D : B {~D() override;};

changing the destructor of B to be non-virtual will cause a compiler error in D.
If 'override' is not present in D, that change would not cause a
compiler error, and would
very probably cause a silent change in meaning and a run-time error.
The question
becomes whether such changes are likely; I don't think they are,
whereas for functions
other than a destructor such changes are somewhat likely.