Re: C++ style question: virtual annotations on methods
On Wednesday 2013-09-04 14:28 +1000, Cameron McCormack wrote:
Bobby Holley wrote:
+1. EIBTI.
I agree, though MOZ_OVERRIDE does imply that the function is virtual
already, so it may not be so necessary there.
I also support repeating virtual as good documentation.
The introduction of MOZ_OVERRIDE (which is newer than most of our
existing code) perhaps offers a reason not to bother anymore,
though. But I think it's useful to have |virtual| be explicit.
There are many cases of member function declarations like:
/* virtual */ void theFunction();
I don't recall that convention for declarations, but what I do write
quite often is the same thing in function *definitions*, where
virtual (and static, for static methods) aren't allowed to be
repeated. In other words, I generally write:
class Foo {
virtual void do_something();
};
/* virtual */ void
Foo::do_something()
{
}
-David
--
턞 L. David Baron http://dbaron.org/ 턂
턢 Mozilla https://www.mozilla.org/ 턂
Before I built a wall I'd ask to know
What I was walling in or walling out,
And to whom I was like to give offense.
- Robert Frost, Mending Wall (1914)
signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
FWIW, I like to write both virtual and MOZ_OVERRIDE. I care a lot about always using MOZ_OVERRIDE when applicable. For virtual (when MOZ_OVERRIDE is present) I suppose it is more of a matter of tastes. Always explicitly writing both virtual and MOZ_OVERRIDE is a simpler rule than marking virtual only in the cases where needed. Plus, virtual is highlighted in our editors and people often (at least I) get used to looking for the answer to is it virtual? by glancing at the beginning of the line in the declaration. So I find always explicitly writing virtual to be easier to read. Cheers, Nical On Wed, Sep 4, 2013 at 6:45 AM, Chris Pearce cpea...@mozilla.com wrote: On 04-Sep-13 4:18 PM, Robert O'Callahan wrote: I like to put virtual on all methods that are virtual, even when it's not strictly necessary because the method overrides a virtual method of the parent class. Other people disagree, especially when the method has MOZ_OVERRIDE on it as well. virtual on non-overrides, MOZ_OVERRIDE on overrides. MOZ_OVERRIDE implies virtual, you get a compile error when you put MOZ_OVERRIDE on a non virtual, so it enforces that you're doing what you think you're doing, so we should use it. Chris P. __**_ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/**listinfo/dev-platformhttps://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
On 9/4/2013 12:45 AM, Chris Pearce wrote: On 04-Sep-13 4:18 PM, Robert O'Callahan wrote: I like to put virtual on all methods that are virtual, even when it's not strictly necessary because the method overrides a virtual method of the parent class. Other people disagree, especially when the method has MOZ_OVERRIDE on it as well. virtual on non-overrides, MOZ_OVERRIDE on overrides. I still agree with roc that virtual MOZ_OVERRIDE is better than just MOZ_OVERRIDE. MOZ_OVERRIDE implies virtual, you get a compile error when you put MOZ_OVERRIDE on a non virtual It does? That surprises me (it certainly wasn't the original intent of NS_OVERRIDE). There are certainly cases where we want to override non-virtual methods with an identical signature. --BDS ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
On 09/04/2013 05:24 AM, Benjamin Smedberg wrote:
MOZ_OVERRIDE implies virtual, you get a compile error when you put
MOZ_OVERRIDE on a non virtual
It does? That surprises me (it certainly wasn't the original intent of
NS_OVERRIDE). There are certainly cases where we want to override non-virtual
methods with an identical signature.
C++11 override functionality only works on virtual methods. Agreed it's
somewhat unfortunate it's only on virtual methods -- else I'd have used it on
all the deleted methods in js/src/vm/String.h -- but that's how the language
works. We could of course add a new macro, plugging into static analysis, if
we wanted inherited-signature-matching functionality that worked on
non-virtuals.
Making virtual mandatory, and MOZ_OVERRIDE mandatory when applicable (to
slightly expand this thread's scope), sounds good to me.
The only edge case is if you have a template class inheriting from a
template-selected base, and the template class has a method that's virtual
depending on the selected base class:
templateclass Base
struct Foo : public Base
{
void fun() {}
};
struct B1 { virtual void fun(); };
struct B2 { void fun(); };
FooB1().fun(); // virtual
FooB2().fun(); // non-virtual
There's probably a use case for this. SecurityWrapper is probably close but no
cigar; nothing closer springs immediately to mind. This case would preclude
adding virtual or MOZ_OVERRIDE to Foo::fun's declaration -- probably a bridge
to cross if we get to it, of course.
Jeff
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
On 2013-09-04 12:18 AM, Robert O'Callahan wrote: I like to put virtual on all methods that are virtual, even when it's not strictly necessary because the method overrides a virtual method of the parent class. Other people disagree, especially when the method has MOZ_OVERRIDE on it as well. Those people are wrong. :-) And it seems like everybody here agrees. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
+1 for always using virtual (useful documentation without having to check the super class), even with MOZ_OVERRIDE (just style). Also +1 for /* static */ on method definitions (when they are declared static) because that is useful information. /* virtual */ on definitions, I don't find useful because it is not necessary to understand the definition of the method, only when the method is called. On Wednesday, September 4, 2013 4:18:37 PM UTC+12, Robert O'Callahan wrote: I like to put virtual on all methods that are virtual, even when it's not strictly necessary because the method overrides a virtual method of the parent class. Other people disagree, especially when the method has MOZ_OVERRIDE on it as well. What say you all? Rob -- Jtehsauts tshaei dS,o n Wohfy Mdaon yhoaus eanuttehrotraiitny eovni le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o Whhei csha iids teoa stiheer :p atroa lsyazye,d 'mYaonu,r sGients uapr,e tfaokreg iyvoeunr, 'm aotr atnod sgaoy ,h o'mGee.t uTph eann dt hwea lmka'n? gBoutt uIp waanndt wyeonut thoo mken.o w * * ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
On Thu, Sep 5, 2013 at 7:40 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-09-04 12:18 AM, Robert O'Callahan wrote: I like to put virtual on all methods that are virtual, even when it's not strictly necessary because the method overrides a virtual method of the parent class. Other people disagree, especially when the method has MOZ_OVERRIDE on it as well. Those people are wrong. :-) And it seems like everybody here agrees. Chris P doesn't, bless him, but he's a clear minority so I'll update the style guide. Rob -- Jtehsauts tshaei dS,o n Wohfy Mdaon yhoaus eanuttehrotraiitny eovni le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o Whhei csha iids teoa stiheer :p atroa lsyazye,d 'mYaonu,r sGients uapr,e tfaokreg iyvoeunr, 'm aotr atnod sgaoy ,h o'mGee.t uTph eann dt hwea lmka'n? gBoutt uIp waanndt wyeonut thoo mken.o w * * ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
On Tue, Sep 3, 2013 at 9:18 PM, Robert O'Callahan rob...@ocallahan.orgwrote: I like to put virtual on all methods that are virtual, even when it's not strictly necessary because the method overrides a virtual method of the parent class. +1. EIBTI. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
Bobby Holley wrote: +1. EIBTI. I agree, though MOZ_OVERRIDE does imply that the function is virtual already, so it may not be so necessary there. There are many cases of member function declarations like: /* virtual */ void theFunction(); I'm assuming this is done because of some compiler warnings about using virtual when it wasn't strictly necessary? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ style question: virtual annotations on methods
On 04-Sep-13 4:18 PM, Robert O'Callahan wrote: I like to put virtual on all methods that are virtual, even when it's not strictly necessary because the method overrides a virtual method of the parent class. Other people disagree, especially when the method has MOZ_OVERRIDE on it as well. virtual on non-overrides, MOZ_OVERRIDE on overrides. MOZ_OVERRIDE implies virtual, you get a compile error when you put MOZ_OVERRIDE on a non virtual, so it enforces that you're doing what you think you're doing, so we should use it. Chris P. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
