Seeing this a touch late, commenting on things not noted yet. On 04/27/2015 12:48 PM, Ehsan Akhgari wrote: > 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: > > * It is a more succinct, as |virtual void foo() override;| doesn't convey > more information than |void foo() override;|. > * 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.
All else equal, shorter is better. But this concision hurts readability, even past the non-obvious final/override => virtual implication others have noted. (And yes, C++ can/should permit final/override on non-virtuals. JSString and subclasses would be immediate users.) Requiring removal of "virtual" from the signature for final/override prevents simply examining a declaration's start to determine whether the function is virtual. You must read the entire declaration to know: a problem because final/override can "blend in". For longer (especially multiline) declarations this matters. Consider these SpiderMonkey declarations: > /* Standard internal methods. */ > virtual bool getOwnPropertyDescriptor(JSContext* cx, HandleObject proxy, > HandleId id, > MutableHandle<JSPropertyDescriptor> > desc) const override; > virtual bool defineProperty(JSContext* cx, HandleObject proxy, HandleId > id, > Handle<JSPropertyDescriptor> desc, > ObjectOpResult& result) const override; > virtual bool ownPropertyKeys(JSContext* cx, HandleObject proxy, > AutoIdVector& props) const override; > virtual bool delete_(JSContext* cx, HandleObject proxy, HandleId id, > ObjectOpResult& result) const override; "virtual" is extraordinarily clear in starting each declaration. "override" and "final" alone would be obscured at the end of a long string of text, especially when skimming. (I disagree with assuming syntax coloring penalizing non-IDE users.) I think ideal style requires as many of virtual/override/final as apply. Virtual for easy reading. Override to be clear when it occurs. And final when that's what you want. (Re dholbert's "subtle" point: |virtual void foo() final| loses strictness, but -Woverloaded-virtual gives it back.) > * It will allow us to remove NS_IMETHODIMP, and use NS_IMETHOD instead. A better alternative, that exposes standard C++ and macro-hides non-standard gunk, would be to use NS_IMETHOD for everything and directly use |virtual| in declarations. This point is no justification for changing style rules. Jeff _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
