> On 20. Aug 2018, at 16:12, Ville Voutilainen <[email protected]> > wrote: > > On 20 August 2018 at 16:58, Kevin Funk via Development > <[email protected]> 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/[email protected]/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 [email protected] http://qt.io Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
