On Mon, Apr 27, 2015 at 03:48:48PM -0400, Ehsan Akhgari wrote: > Right now, our coding style requires that both the virtual and override > keywords to be specified for overridden virtual functions. A few things > have changed since we decided that a number of years ago: > > 1. The override and final keywords are now available on all of the > compilers that we build with, and we have stopped supporting compilers that > do not support these features. > 2. We have very recently gained the ability to run clang-based mass source > code transformations over our code that would let us enforce the coding > style [1]. > > I would like to propose a change to our coding style, and run a mass > transformation over our code in order to enforce it. I think we should > change it to require the usage of exactly one of these keywords per > *overridden* function: virtual, override, and final. Here are the > advantages:
I believe we have some cases in the tree where a virtual function doesn't override but is final so you need to write virtual final. I believe one of those cases may be so a function can be called indirectly from outside libxul, and another might be to prevent people using some cc macros incorrectly. > * It is a more succinct, as |virtual void foo() override;| doesn't convey > more information than |void foo() override;|. personally I think it takes significantly more mental effort to read void foo() final; to mean it overrides something and is virtual than if its explicit as in virtual void foo() override final; and actually I'd also like it if C++ changed to allow override and final on non virtual functions in which case this would be a loss of information. > * It can be easily enforced using clang-tidy across the entire code base. That doesn't really seem like an argument for choosing this particular style we could as easily check that virtual functions are always marked virtual, and marked override if possible. > * It makes it easier to determine what kind of function you are looking at > by just looking at its declaration. |virtual void foo();| means a virtual > function that is not overridden, |void foo() override;| means an overridden > virtual function, and |void foo() final;| means an overridden virtual > function that cannot be further overridden. this seems to be more an advantage of any enforced rule. That is if we just enforced that any function that overrides is marked override then you would know that virtual void foo(); doesn't override, and otherwise override would always be present which would make it clear it does override in addition to possibly being final. > * It allows us to be in sync with the Google C++ Style on this issue [2]. I don't personally care about that much. > * It will allow us to remove NS_IMETHODIMP, and use NS_IMETHOD instead. That doesn't really seem like much of an improvement, and of course really we should just get rid of both but that's more work. Trev > > Please let me know what you think! > > [1] Facilitated by bug 904572. Also see bug 1158776 for my first attempt > at this. > [2] > http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Inheritance > > Cheers, > -- > Ehsan > _______________________________________________ > dev-platform mailing list > [email protected] > https://lists.mozilla.org/listinfo/dev-platform _______________________________________________ dev-platform mailing list [email protected] https://lists.mozilla.org/listinfo/dev-platform
