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
