[boost::noncopyable] compiler warnings with the -Weffc++ flags
Hi, Deriving from the boost::noncopyable class generates warnings when I compile with the -Weffc++ (effective C++) flags. There are two types of warnings: 1) Non-virtual destructor warning (base class 'class boost::noncopyable_::noncopyable' has a non-virtual destructor). Here's a testcase: #include <boost/noncopyable.hpp> #include <iostream> namespace { class DontTreadOnMe : private boost::noncopyable { public: DontTreadOnMe() { std::cout << "defanged!" << std::endl; } }; // DontTreadOnMe } // unnamed namespace main() { DontTreadOnMe object1; } g++ -I/vobs/3rdparty/BOOST/boost_1_38_0/boost_inc -Weffc++ main.cpp main.cpp:10: warning: base class 'class boost::noncopyable_::noncopyable' has a non-virtual destructor 2) I have disabled the copy constructor/assignment operator in a class that has pointer data members by deriving from boost::noncopyable. However, the compiler generates the following warning: warning: 'class<unnamed>::DontTreadOnMe' has pointer data members main.cpp:10: warning: but does not override '<unnamed>::DontTreadOnMe(const <unnamed>::DontTreadOnMe&)' This kind of forces me to explicitly override the copy constructor and assignment operator instead of deriving from noncopyable. Here's the full test case: #include <boost/noncopyable.hpp> #include <iostream> namespace { class DontTreadOnMe : private boost::noncopyable { public: DontTreadOnMe() { std::cout << "defanged!" << std::endl; } private: char* buf; }; // DontTreadOnMe } // unnamed namespace main() { DontTreadOnMe object1; } g++ -I/vobs/3rdparty/BOOST/boost_1_38_0/boost_inc -Weffc++ main.cpp main.cpp:10: warning: base class 'class boost::noncopyable_::noncopyable' has a non-virtual destructor main.cpp:10: warning: 'class<unnamed>::DontTreadOnMe' has pointer data members main.cpp:10: warning: but does not override '<unnamed>::DontTreadOnMe(const <unnamed>::DontTreadOnMe&)' main.cpp:10: warning: or 'operator=(const <unnamed>::DontTreadOnMe&)' Is there a workaround and/or fix? I am using gcc 4.0.1. Thanks, Anand
AMDG Anand Lakshminath wrote:
Deriving from the boost::noncopyable class generates warnings when I compile with the -Weffc++ (effective C++) flags.
There are two types of warnings:
<snip> g++ -I/vobs/3rdparty/BOOST/boost_1_38_0/boost_inc -Weffc++ main.cpp main.cpp:10: warning: base class 'class boost::noncopyable_::noncopyable' has a non-virtual destructor
If you're planning on using Boost at all, you should disable this warning. Many Boost libraries use inheritance without virtual destructors heavily and gcc doesn't make it easy to selectively disable spurious warnings.
main.cpp:10: warning: 'class<unnamed>::DontTreadOnMe' has pointer data members main.cpp:10: warning: but does not override '<unnamed>::DontTreadOnMe(const <unnamed>::DontTreadOnMe&)' main.cpp:10: warning: or 'operator=(const <unnamed>::DontTreadOnMe&)'
Is there a workaround and/or fix? I am using gcc 4.0.1.
Um. Don't use -Weffc++? Seriously, the code is not broken and there's nothing we can do to prevent these warnings. In Christ, Steven Watanabe
on Thu Jun 11 2009, Steven Watanabe <watanabesj-AT-gmail.com> wrote:
AMDG
Anand Lakshminath wrote:
Deriving from the boost::noncopyable class generates warnings when I compile with the -Weffc++ (effective C++) flags.
There are two types of warnings:
<snip> g++ -I/vobs/3rdparty/BOOST/boost_1_38_0/boost_inc -Weffc++ main.cpp main.cpp:10: warning: base class 'class boost::noncopyable_::noncopyable' has a non-virtual destructor
If you're planning on using Boost at all, you should disable this warning. Many Boost libraries use inheritance without virtual destructors heavily and gcc doesn't make it easy to selectively disable spurious warnings.
main.cpp:10: warning: 'class<unnamed>::DontTreadOnMe' has pointer data members main.cpp:10: warning: but does not override '<unnamed>::DontTreadOnMe(const <unnamed>::DontTreadOnMe&)' main.cpp:10: warning: or 'operator=(const <unnamed>::DontTreadOnMe&)'
Is there a workaround and/or fix? I am using gcc 4.0.1.
Um. Don't use -Weffc++? Seriously, the code is not broken and there's nothing we can do to prevent these warnings.
Yeah, that's a dumb warning unless you're doing strictly traditional OO and nothing else (and not using any libraries that do anything else). At the *very least* using private derivation should suppress it. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
Perhaps slightly off topic, but I seem to remember a couple of years ago Sean Parent pointing out that noncopyable might be better implemented with CRTP. Has this been forgotten? I think the rationale was that if you have one child which inherits from two or more bases that inherit from the current noncopyable, the empty base optimization can't be used since each of the noncopyable bases have to have different addresses, so your type's size grows with each additional base that inherits from noncopyable. As well, I believe that certain compilers were producing spurious warnings if you use such multiple inheritance, suggesting to the programmer that noncopyable may be better as a virtual base (which it clearly would not). Making a noncopyable template, perhaps called noncopyable_ so that the two can coexist, that is implemented the same way but whose intended use is to be instantiated with the child type fixes these problems by making certain that all noncopyable bases in a hierarchy have different types, meaning that EBO may be used and there are no duplicate noncopyable bases. -- -Matt Calabrese
Hi Matt, Sean reported the same underlying problem against the operators library as <https://svn.boost.org/trac/boost/ticket/979>. I applied the fix as it was transparent for the users and also suggested a similar change for noncopyable on the mailing list - but there wasn't much interest IIRC. The problem with the approach you described is IMHO that if you allow noncopyable_<T> as an *alternative*, most people will likely ignore it. OTOH, a breaking change - turning the classic noncopyable into a template - might be too intrusive. Personally, I would not mind it, since the compiler will catch all uses of noncopyable without a template parameter and the required change is a no-brainer, so +1 for the change from my side. Let's see if more people like the idea this time :) Regards, Daniel On 13.06.2009, at 18:11, Matt Calabrese wrote:
Perhaps slightly off topic, but I seem to remember a couple of years ago Sean Parent pointing out that noncopyable might be better implemented with CRTP. Has this been forgotten? I think the rationale was that if you have one child which inherits from two or more bases that inherit from the current noncopyable, the empty base optimization can't be used since each of the noncopyable bases have to have different addresses, so your type's size grows with each additional base that inherits from noncopyable. As well, I believe that certain compilers were producing spurious warnings if you use such multiple inheritance, suggesting to the programmer that noncopyable may be better as a virtual base (which it clearly would not). Making a noncopyable template, perhaps called noncopyable_ so that the two can coexist, that is implemented the same way but whose intended use is to be instantiated with the child type fixes these problems by making certain that all noncopyable bases in a hierarchy have different types, meaning that EBO may be used and there are no duplicate noncopyable bases. -- -Matt Calabrese _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
AMDG Daniel Frey wrote:
Sean reported the same underlying problem against the operators library as <https://svn.boost.org/trac/boost/ticket/979>. I applied the fix as it was transparent for the users and also suggested a similar change for noncopyable on the mailing list - but there wasn't much interest IIRC.
The problem with the approach you described is IMHO that if you allow noncopyable_<T> as an *alternative*, most people will likely ignore it. OTOH, a breaking change - turning the classic noncopyable into a template - might be too intrusive. Personally, I would not mind it, since the compiler will catch all uses of noncopyable without a template parameter and the required change is a no-brainer, so +1 for the change from my side. Let's see if more people like the idea this time :)
I don't think this is sufficiently important to warrant a breaking change no matter how obvious the update is. In Christ, Steven Watanabe
On Sat, Jun 13, 2009 at 12:48 PM, Daniel Frey <d.frey@gmx.de> wrote:
The problem with the approach you described is IMHO that if you allow noncopyable_<T> as an *alternative*, most people will likely ignore it.
I'm not convinced that is a problem. If people ignore it their code will still be correct, it just might make certain objects larger than they need to be, which is no different from the present situation. Those who are aware of the change could now use it instead of either just living with noncopyable as it currently is or handrolling their own noncopyable template. As well, those new to boost who are just being introduced to noncopyable would be able to use the template version from the start, since that's what the documentation would recommend. The added noncopyable_ template wouldn't hurt anyone IMO. On Sat, Jun 13, 2009 at 12:59 PM, Steven Watanabe <watanabesj@gmail.com>wrote:
I don't think this is sufficiently important to warrant a breaking change no matter how obvious the update is.
I'm with Steven on this, I don't think it's important enough for a sudden breaking change, so if a template version were added, the current version should continue to exist at least for a few versions. As well, the current noncopyable is used in at least one other library for a different purpose where there is no need for a template argument, and one would probably just make things confusing (I.E. Boost.Python's class_ template uses it to signal the implementation to suppress registration of to_python conversions, see http://www.boost.org/doc/libs/1_39_0/libs/python/doc/v2/class.html ). I'm fine with just having a template version documented and leaving the non-template version in undocumented (or with a note that it is for legacy code), but if people think it would be necessary to have either the template or the nontemplate, but not both, you could always spread the change across a few versions to make things easier for users and avoid a very sudden breaking change. For instance, you could add the noncopyable_ template for a few versions, deprecating noncopyable but leaving it in. Then, remove the noncopyable spelling completely and have both noncopyable_ and noncopyable be templates, with the underscored version deprecated. Finally, remove the underscore version entirely and be left with noncopyable as a template. Boost.Python would have to be updated accordingly but that wouldn't be much of a problem -- it could either switch to using its own noncopyable tag type or use noncopyable<>, where noncopyable would have an unspecified default. -- -Matt Calabrese
On 13.06.2009, at 20:27, Matt Calabrese wrote:
or the nontemplate, but not both, you could always spread the change across a few versions to make things easier for users and avoid a very sudden breaking change. For instance, you could add the noncopyable_ template for a few versions, deprecating noncopyable but leaving it in. Then, remove the noncopyable spelling completely and have both noncopyable_ and noncopyable be templates, with the underscored version deprecated. Finally, remove the underscore version entirely and be left with noncopyable as a template.
Sounds like a reasonable plan, but just adding noncopyable_<T> as an alternative and leaving noncopyable as it is is also fine with me. I don't have any strong feelings on this, I just tried to express that I personally wouldn't mind a breaking change.
Boost.Python would have to be updated accordingly but that wouldn't be much of a problem -- it could either switch to using its own noncopyable tag type or use noncopyable<>, where noncopyable would have an unspecified default.
I haven't looked at it, but as you describe it, it sound as if they should just create their own tag type anyway as they are simply misusing noncopyable. What might work technically is still wrong if it communicates the wrong thing. Regards, Daniel
On Sat, Jun 13, 2009 at 7:25 PM, Daniel Frey <d.frey@gmx.de> wrote:
On 13.06.2009, at 20:27, Matt Calabrese wrote: I haven't looked at it, but as you describe it, it sound as if they should just create their own tag type anyway as they are simply misusing noncopyable.
I think I agree in calling it a misuse, but I am always afraid to make such a statement, especially given that I'm pretty sure Dave created both noncopyable and the class_ interface. IMO, boost::noncopyable should not be the tag used with class_, but realistically it doesn't matter too much unless noncopyable is changed in the way described. -- -Matt Calabrese
on Sun Jun 14 2009, Matt Calabrese <rivorus-AT-gmail.com> wrote:
On Sat, Jun 13, 2009 at 7:25 PM, Daniel Frey <d.frey@gmx.de> wrote:
On 13.06.2009, at 20:27, Matt Calabrese wrote: I haven't looked at it, but as you describe it, it sound as if they should just create their own tag type anyway as they are simply misusing noncopyable.
I think I agree in calling it a misuse, but I am always afraid to make such a statement, especially given that I'm pretty sure Dave created both noncopyable and the class_ interface.
I did.
IMO, boost::noncopyable should not be the tag used with class_,
I'm not really sure there's a better name for what it does. Why should a different tag be used? -- Dave Abrahams BoostPro Computing http://www.boostpro.com
on Sat Jun 13 2009, Matt Calabrese <rivorus-AT-gmail.com> wrote:
Perhaps slightly off topic, but I seem to remember a couple of years ago Sean Parent pointing out that noncopyable might be better implemented with CRTP.
Ironic, since in Sean's world, all types are copyable :-)
Has this been forgotten? I think the rationale was that if you have one child which inherits from two or more bases that inherit from the current noncopyable, the empty base optimization can't be used since each of the noncopyable bases have to have different addresses, so your type's size grows with each additional base that inherits from noncopyable. As well, I believe that certain compilers were producing spurious warnings if you use such multiple inheritance, suggesting to the programmer that noncopyable may be better as a virtual base (which it clearly would not). Making a noncopyable template, perhaps called noncopyable_ so that the two can coexist, that is implemented the same way but whose intended use is to be instantiated with the child type fixes these problems by making certain that all noncopyable bases in a hierarchy have different types, meaning that EBO may be used and there are no duplicate noncopyable bases.
Yeah, it's an issue, but I'm not sure it's worth improving. I view noncopyable as a quick fixup for people doing traditional OO-style programming. I'm open to arguments, though. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
On Thu, Jun 18, 2009 at 9:01 PM, David Abrahams <dave@boostpro.com> wrote:
Ironic, since in Sean's world, all types are copyable :-)
Ha, yes, and I even tend to agree. I look at noncopyable as more of a utility to prevent people from copying either because the operations just haven't yet been implemented or because there is currently no practical use-case for it to be called. In a perfect world, every type would be regular and it would take no time or effort to make them such, but the world is sadly not perfect. On Thu, Jun 18, 2009 at 9:50 PM, David Abrahams <dave@boostpro.com> wrote:
I'm not really sure there's a better name for what it does. Why should a different tag be used?
It's not that important to me and is more subjective than anything else. I don't see any issue with the name, more that it's reusing a type intended for a technically different purpose _because_ of that name (a noncopyable tag type with the same name in ::boost::python for instance would be fine if not potentially confusing). The use of noncopyable in Boost.Python does not directly relate to its documented purpose if you read the documentation of noncopyable. Again, it's really just used because of the name rather than the type's actual purpose. If noncopyable were a more complicated type or if its use were vastly different from its use in class_, I'd see it as an actual problem, but given that it's just a simple little utility type that doesn't even really depend on anything, there is no practical reason why it shouldn't be used. I doubt it even causes any confusion so it's just more of a philosophical issue at that point and I wouldn't use that alone as a rationale for a change. The only real issue as I see it is that if noncopyable becomes a template, it is a little bit less clear as to what exactly should be done for class_ to reflect the change. -- -Matt Calabrese
participants (5)
-
Anand Lakshminath -
Daniel Frey -
David Abrahams -
Matt Calabrese -
Steven Watanabe