[Scummvm-devel] The Curious Case of NonCopyable
matt at use.net
Tue Apr 28 20:37:15 CEST 2009
In doing some cleanups of warnings reported by G++'s -Weffc++ and PC-Lint,
I came across this kind of warning:
lint-undeserved-173x.cpp:11: warning: 'class PluginManager' has pointer
lint-undeserved-173x.cpp:11: warning: but does not override
lint-undeserved-173x.cpp:11: warning: or 'operator=(const
PluginManager (indirectly) inherits from NonCopyable, which defines a
private copy ctor and assignment operator that do not have an
implementation. If anyone ever tried to use either, they'll get a link
error. This is the desired effect, and helps avoid a whole host of pointer
However, the checks in these tools basically say it doesn't count. This is
because the pointer member is in the subclass, which the NonCopyable
parent can't possibly know about and therefore handle the copy (deep or
Also, the indirect nature of this might be confusing to some
folks who would add a copy ctor or assignment operator in the subclass.
With -Wall and without -Wextra *and* -Weffc++, there is no warning that
something has gone wrong. (Unless, of course you explicitly enable the
individual warnings to catch these specific issues, which I don't think is
possible.) This means that these protections could be easily circumvented
by accident, leaving the safety net of code reviews and/or manual testing
to find the issue.
To remedy these issues, I suggest eliminating NonCopyable as a "mix-in"
type, and favoring a macro to be used directly in the subclasses that
should not be copyable. This takes the declarative approach closer to the
class is actually affects, making it more explicit and a little easier to
understand. It also eliminates these warnings and allows future warnings
to highlight real potential problems.
I suggest a macro that looks like:
void operator=(const klass&)
I can make this change locally and commit it as part of my warning
cleanup. If that is agreeable, where should the macro go?
tangled strands of DNA explain the way that I behave.
More information about the Scummvm-devel