[Development] override keyword on destructors
Kevin Funk
kevin.funk at kdab.com
Mon Aug 20 15:58:35 CEST 2018
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.
[1] https://github.com/isocpp/CppCoreGuidelines/issues/721
[2] https://en.cppreference.com/w/cpp/language/override
My 2 cents.
Regards,
Kevin
> So clang-tidy is the one odd out.
>
> I'll update the coding conventions if nobody opposes.
>
>
>
> [1] -
> https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md
> #Rh-override
>
> Regards,
--
Kevin Funk | kevin.funk at kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5322 bytes
Desc: not available
URL: <http://lists.qt-project.org/pipermail/development/attachments/20180820/af8fe446/attachment.bin>
More information about the Development
mailing list