On 2018-02-16 1:07 PM, L. David Baron wrote:
virtual void Bad1() final
I think there might be some legitimate use cases for this one, when
a function needs to be virtual because it's required for the calling
convention (as part of a binary plugin API or binary embedding API,
for example), but it's also not to be overridden, and it's also not
overriding a virtual function on a base class. While we've moved
away from binary plugin interfaces, I could imagine it the
definition of an API for embedding Gecko or some part of it.
That's an interesting point and there are some possible workarounds (below).
btw, I found four such non-overriding virtual/final declarations in
mozilla-central (links below) while writing this lint script:
1. NOT_INHERITED_CANT_OVERRIDE is a debug macro that declares a
non-overriding final virtual function (BaseCycleCollectable) as a
compile-time check of some properties of CC-able classes.
2. AccessibleNode::GetParentObject(). GetParentObject() is a common
virtual function in many DOM classes, but AccessibleNode does not derive
from any base classes the define GetParentObject(). I think this might
be some code that was copy/pasted. It doesn't need to be virtual.
3. WebCryptoTask::CalculateResult() and CallCallback() mirror virtual
function names in the CryptoTask class, though WebCryptoTask does not
actually derive from CryptoTask. These classes used to share code but
were decoupled (in bug 1001691) so WebCryptoTask could be used on worker
threads. These functions don't need to be virtual.
4. nsWindowBase::GetWindowHandle(), which does not override any base
class functions. The only other function named GetWindowHandle() is
MouseScrollHandler::EventInfo::GetWindowHandle() in an unrelated class.
I think it's reasonable to warn for it since the above case should
be pretty rare, but I'd be a little concerned about forbidding it
completely.
My lint check is currently written to run on checkins, so its warning
would be treated as an error on Treeherder. Possible workarounds:
* Individual files or directories to be whitelisted in the lint script.
This is easy.
* The virtual and final keywords could be moved to different lines. My
lint is just a complicated regular expression and can't analyze virtual
function declarations that span multiple lines.
[1]
https://searchfox.org/mozilla-central/rev/a5abf843f8fac8530aa5de6fb40e16547bb4a47a/xpcom/base/nsCycleCollectionParticipant.h#619-629
[2]
https://searchfox.org/mozilla-central/rev/a5abf843f8fac8530aa5de6fb40e16547bb4a47a/accessible/aom/AccessibleNode.h#37
[3]
https://searchfox.org/mozilla-central/rev/a5abf843f8fac8530aa5de6fb40e16547bb4a47a/dom/crypto/WebCryptoTask.h#182,184
[4]
https://searchfox.org/mozilla-central/rev/cac28623a15ace458a8f4526e107a71db1519daf/widget/windows/WinMouseScrollHandler.h#191
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
