[Development] override keyword on destructors
Eike Ziller
Eike.Ziller at qt.io
Mon Aug 20 16:40:31 CEST 2018
> On 20. Aug 2018, at 16:12, Ville Voutilainen <ville.voutilainen at gmail.com> wrote:
>
> 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.
For example when removing QObject as a base class in a class hierarchy.
We regularly do these kind of changes in Qt Creator, when we find out that we don’t need the baggage of QObject after all for something.
Br, Eike
--
Eike Ziller
Principal Software Engineer
The Qt Company GmbH
Rudower Chaussee 13
D-12489 Berlin
eike.ziller at qt.io
http://qt.io
Geschäftsführer: Mika Pälsi,
Juha Varelius, Mika Harjuaho
Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B
More information about the Development
mailing list