A request from the docs team: once the final decisions are made, please either let us know what those decisions are (use our doc request form: https://bugzilla.mozilla.org/form.doc) or update the coding style guide yourselves ( https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style). Either one wins you brownie points, but updating the doc yourselves will result in super-tasty brownie points.
On Thu, May 7, 2015 at 9:52 AM, Ehsan Akhgari <ehsan.akhg...@gmail.com> wrote: > On 2015-05-05 2:51 PM, Jeff Walden wrote: > >> 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.) >> > > At the risk of repeating myself and others here, let's not worry about > what C++ should do if it were being redesigned today, and let's focus on > what it actually does. > > 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.) >> > > This is basically what Trevor was arguing for. I don't think there is > much more to say on this as we're basically comparing what you and I are > used to read. > > * 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. >> > > That has similar verbosity issues. But yeah I agree on the broader point > that this macro situation can be solved in other ways, we just disagree > what those ways should be. :-) > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > -- Eric Shepherd Senior Technical Writer Mozilla Blog: http://www.bitstampede.com/ Twitter: http://twitter.com/sheppy _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
