Discussion:
[Audacity-devel] final, const, and override
Paul Licameli
2016-08-10 22:48:09 UTC
Permalink
James lately commented in a commit:


(Cosmetic) added 'final' to class definition.

Putting the word 'final' is a bit like adding a comment that nothing is
derived from this effect.

My understanding is that final can cause very slightly more efficient code
to be generated. If Y derives from X, and the inheritance is final, and
class Y has a virtual function, and you call that virtual function through
a pointer-to-Y variable, then the compiler can deduce that it does not need
to generate the code to do the virtual function dispatch, but can jump to
Y's override directly.

Not the compelling reason to use final, but it does make final a little
more than a comment. Yet still I think final is mostly useful as a comment
on the programmer's intention.

Whereas const and override are more than mere comments: they help the
compiler to help you, by restricting what you can legally write. Pass
compilation checks with const and override in place, and you have more
confidence that the code is correct. override in paticular, a new C++11
thing, prevents you from getting the argument or return types wrong in an
intended virtual function override -- which would otherwise quietly compile
without override, but not do what you intended.

PRL
James Crook
2016-08-11 10:11:45 UTC
Permalink
Post by Paul Licameli
(Cosmetic) added 'final' to class definition.
Putting the word 'final' is a bit like adding a comment that nothing is
derived from this effect.
My understanding is that final can cause very slightly more efficient code
to be generated. If Y derives from X, and the inheritance is final, and
class Y has a virtual function, and you call that virtual function through
a pointer-to-Y variable, then the compiler can deduce that it does not need
to generate the code to do the virtual function dispatch, but can jump to
Y's override directly.
Not the compelling reason to use final, but it does make final a little
more than a comment. Yet still I think final is mostly useful as a comment
on the programmer's intention.
What I wrote in Bugzilla was 99% for QA - so they had an idea what the
change was about. Yes indeed 'final' is slightly beyond a comment, in
that it (should) stop future developers deriving EffectBassMidTreble
from EffectBassTreble, whereas a comment wouldn't. The potential
efficiency difference is so tiny that I wouldn't use it as even partial
justification in this case.

FWIW some more background on why I made the change - I'd been spending
some time investigating
http://bugzilla.audacityteam.org/show_bug.cgi?id=1460 and I was trying
to make EffectBassTreble 'more like' the other RTP effects - in an
attempt to find out why it sometimes is disabled in the menu list.

Changing
#define BASSTREBLE_PLUGIN_SYMBOL XO("Bass and Treble")
to
#define BASSTREBLE_PLUGIN_SYMBOL XO("XBass and Treble")

so that the effect appears later in the menu 'fixed' the problem of the
effect being greyed out for me. I thought the problem might always be
with the first RTP effect in the list rather than be specific to 'Bass
and Treble'. I'd hoped 'Distortion' would start being disabled instead,
but that didn't happen. When I changed the Bass and Treble define back
the problem had gone moonphase on me and I no longer saw it at all. So
I have moved on, and am no longer investigating this P2. I guess if it
comes back for me I will pick it up again.
Post by Paul Licameli
Whereas const and override are more than mere comments: they help the
compiler to help you, by restricting what you can legally write. Pass
compilation checks with const and override in place, and you have more
confidence that the code is correct. override in paticular, a new C++11
thing, prevents you from getting the argument or return types wrong in an
intended virtual function override -- which would otherwise quietly compile
without override, but not do what you intended.
If I recall correctly I have in the past been caught by virtual
functions with and without const in them. This can lead to a virtual
function NOT being called because the signatures don't match, and no
warning, when the only difference is in 'const'. So const actually
created a problem in the category of problems that override was invented
to solve!

--James.
Steve the Fiddle
2016-08-11 10:56:10 UTC
Permalink
Post by James Crook
Post by Paul Licameli
(Cosmetic) added 'final' to class definition.
Putting the word 'final' is a bit like adding a comment that nothing is
derived from this effect.
My understanding is that final can cause very slightly more efficient code
to be generated. If Y derives from X, and the inheritance is final, and
class Y has a virtual function, and you call that virtual function through
a pointer-to-Y variable, then the compiler can deduce that it does not need
to generate the code to do the virtual function dispatch, but can jump to
Y's override directly.
Not the compelling reason to use final, but it does make final a little
more than a comment. Yet still I think final is mostly useful as a comment
on the programmer's intention.
What I wrote in Bugzilla was 99% for QA - so they had an idea what the
change was about. Yes indeed 'final' is slightly beyond a comment, in
that it (should) stop future developers deriving EffectBassMidTreble
from EffectBassTreble, whereas a comment wouldn't. The potential
efficiency difference is so tiny that I wouldn't use it as even partial
justification in this case.
FWIW some more background on why I made the change - I'd been spending
some time investigating
http://bugzilla.audacityteam.org/show_bug.cgi?id=1460 and I was trying
to make EffectBassTreble 'more like' the other RTP effects - in an
attempt to find out why it sometimes is disabled in the menu list.
Changing
#define BASSTREBLE_PLUGIN_SYMBOL XO("Bass and Treble")
to
#define BASSTREBLE_PLUGIN_SYMBOL XO("XBass and Treble")
so that the effect appears later in the menu 'fixed' the problem of the
effect being greyed out for me. I thought the problem might always be
with the first RTP effect in the list rather than be specific to 'Bass
and Treble'. I'd hoped 'Distortion' would start being disabled instead,
but that didn't happen. When I changed the Bass and Treble define back
the problem had gone moonphase on me and I no longer saw it at all. So
I have moved on, and am no longer investigating this P2. I guess if it
comes back for me I will pick it up again.
Interesting. Another difference is that "Bass and Treble" has spaces
whereas the other RTP effects don't. I don't see why that should make
a difference, but if anyone can reliably reproduce the problem it may
be worth trying:
#define BASSTREBLE_PLUGIN_SYMBOL XO("BassAndTreble")

Steve
Post by James Crook
Post by Paul Licameli
Whereas const and override are more than mere comments: they help the
compiler to help you, by restricting what you can legally write. Pass
compilation checks with const and override in place, and you have more
confidence that the code is correct. override in paticular, a new C++11
thing, prevents you from getting the argument or return types wrong in an
intended virtual function override -- which would otherwise quietly compile
without override, but not do what you intended.
If I recall correctly I have in the past been caught by virtual
functions with and without const in them. This can lead to a virtual
function NOT being called because the signatures don't match, and no
warning, when the only difference is in 'const'. So const actually
created a problem in the category of problems that override was invented
to solve!
--James.
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity
planning reports. http://sdm.link/zohodev2dev
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Loading...