noncopyable EBO problem?
Hi, as sparent@adobe.com pointed out in <http://svn.boost.org/trac/boost/ticket/979>, the EBO is not possible in some cases. I fixed the operators library, but now I wonder if we should fix boost::noncopyable as well. Consider: class A : boost::noncopyable {}; class B : boost::noncopyable { A a; }; now sizeof(B)==2 instead of the expected 1. To fix it, we have to change the interface (or at least allow an alternative interface). Example with the interface I'd like to suggest: class A : boost::noncopyable_<A> {}; class B : boost::noncopyable_<B> { A a; }; now sizeof(B)==1, as expected. The crucial point of such a change is IMHO the interface. Breaking noncopyable is bad, so I'd like to add noncopyable_<T> and make noncopyable a typedef to noncopyable_<void>. This should provide backwards compatibility and provides users with a better, but less brief alternative in case they need the additional EBO opportunity. Comments? Since it's so damn short, here's the suggested implementation: namespace boost { namespace noncopyable_impl // prevent unintended ADL { template< typename > class noncopyable_ { protected: noncopyable_() {} ~noncopyable_() {} private: noncopyable_( const noncopyable_& ); const noncopyable_& operator=( const noncopyable_& ); }; typedef noncopyable_<void> noncopyable; } using namespace noncopyable_impl; } Regards, Daniel
On Thursday 31 May 2007 20:09:37 Gmane wrote:
as sparent@adobe.com pointed out in <http://svn.boost.org/trac/boost/ticket/979>, the EBO is not possible in some cases. I fixed the operators library, but now I wonder if we should fix boost::noncopyable as well.
Consider:
class A : boost::noncopyable {}; class B : boost::noncopyable { A a; };
now sizeof(B)==2 instead of the expected 1. To fix it, we have to change the interface (or at least allow an alternative interface). Example with the interface I'd like to suggest:
class A : boost::noncopyable_<A> {}; class B : boost::noncopyable_<B> { A a; };
now sizeof(B)==1, as expected.
The crucial point of such a change is IMHO the interface. Breaking noncopyable is bad, so I'd like to add noncopyable_<T> and make noncopyable a typedef to noncopyable_<void>. This should provide backwards compatibility and provides users with a better, but less brief alternative in case they need the additional EBO opportunity. Comments?
I added a comment inline which I hope explains the rationale for this class, because it is far from obvious IMHO. If the reasons there are wrong, please correct me. Other than that, I'd suggest naming it noncopyable_base<T>.
namespace noncopyable_impl // prevent unintended ADL { /* baseclass for noncopyable objects
Usage: // A is noncopyable and non-assignable now class A: noncopyable_<A> {}; Note: the reason this is a template and not a plain class like it used to be is that it improves EBO. If an (otherwise empty) object inherits noncopyable and includes (via a member) noncopyable, too, the compiler is required to assign each a unique address (TODO: insert standard reference), thus the resulting size of the object must be at least 2. If the two non-copyable subobjects are of different types (as they are when templated) they are allowed to have the same address and the compiler can fully exploit EBO. */
template< typename > class noncopyable_ { protected: noncopyable_() {} ~noncopyable_() {} private: noncopyable_( const noncopyable_& ); const noncopyable_& operator=( const noncopyable_& ); }; typedef noncopyable_<void> noncopyable; }
using namespace noncopyable_impl;
Why this? I guess it is intended to prevent ADL, right? Can you point me to a rationale for this? thanks Uli
Ulrich Eckhardt wrote:
I added a comment inline which I hope explains the rationale for this class, because it is far from obvious IMHO. If the reasons there are wrong, please correct me. Other than that, I'd suggest naming it noncopyable_base<T>.
This should IMHO go to the documentation, maybe a small comment in the code could be added as well. My implementation wasn't meant to go the CVS, it was just a complete example of what the technical part of the change should be. Of course the real change would also include documentation changes and maybe even a testcase... For the name, noncopyable_base is too much typing for my taste, YMMV. Ideally, it would have been noncopyable<T> from the very beginning, but it's too late for that, so I think adding a simple _ is the least intrusive change. Anyway, it's a valid idea to use _base, let's see what others think.
namespace noncopyable_impl // prevent unintended ADL { template< typename > class noncopyable_ { // omitted ... }; typedef noncopyable_<void> noncopyable; }
using namespace noncopyable_impl;
Why this? I guess it is intended to prevent ADL, right? Can you point me to a rationale for this?
For example: <http://lists.boost.org/boost-users/2006/06/20110.php> Regards, Daniel
Daniel Frey wrote:
For the name, noncopyable_base is too much typing for my taste, YMMV. Ideally, it would have been noncopyable<T> from the very beginning, but it's too late for that, so I think adding a simple _ is the least intrusive change. Anyway, it's a valid idea to use _base, let's see what others think.
If it has to be changed anyways I'd recommend uncopyable<T>. As Scott Meyers says in Effect C++ "That class is named noncopyable. It's a fine class, I just find the name a bit un-, er, nonnatural." - Michael Marcin
on Fri Jun 01 2007, "Michael Marcin" <mmarcin-AT-method-solutions.com> wrote:
Daniel Frey wrote:
For the name, noncopyable_base is too much typing for my taste, YMMV. Ideally, it would have been noncopyable<T> from the very beginning, but it's too late for that, so I think adding a simple _ is the least intrusive change. Anyway, it's a valid idea to use _base, let's see what others think.
If it has to be changed anyways I'd recommend uncopyable<T>.
As Scott Meyers says in Effect C++
"That class is named noncopyable. It's a fine class, I just find the name a bit un-, er, nonnatural."
Yes, Scott complained about that to me in private before publishing his opinion, but I don't agree with him. "Noncopyable" is less ambiguous and no less correct. Can something be uncopied like it can be unfolded? -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
David Abrahams wrote:
on Fri Jun 01 2007, "Michael Marcin" <mmarcin-AT-method-solutions.com> wrote:
As Scott Meyers says in Effect C++
"That class is named noncopyable. It's a fine class, I just find the name a bit un-, er, nonnatural."
Yes, Scott complained about that to me in private before publishing his opinion, but I don't agree with him. "Noncopyable" is less ambiguous and no less correct. Can something be uncopied like it can be unfolded?
I agree with your reasons for chosing "noncopyable" and I'd like to point out to Michael that it's existing practice for some years now, so yet another argument to stick with it. Dave, any wisdom on the technical part? And as I believe you are the maintainer (and given we can agree that the change should be made and that a good naming scheme is chosen): Would you like to do the change yourself, would you like to receive a patch or shall I go ahead and commit changes to CVS? Regards, Daniel
on Mon Jun 04 2007, Daniel Frey <d.frey-AT-gmx.de> wrote:
David Abrahams wrote:
on Fri Jun 01 2007, "Michael Marcin" <mmarcin-AT-method-solutions.com> wrote:
As Scott Meyers says in Effect C++
"That class is named noncopyable. It's a fine class, I just find the name a bit un-, er, nonnatural."
Yes, Scott complained about that to me in private before publishing his opinion, but I don't agree with him. "Noncopyable" is less ambiguous and no less correct. Can something be uncopied like it can be unfolded?
I agree with your reasons for chosing "noncopyable" and I'd like to point out to Michael that it's existing practice for some years now, so yet another argument to stick with it.
Dave, any wisdom on the technical part? And as I believe you are the maintainer (and given we can agree that the change should be made
I don't think I want to see a backwards-incompatible change. An extension might be OK.
and that a good naming scheme is chosen): Would you like to do the change yourself, would you like to receive a patch or shall I go ahead and commit changes to CVS?
If you have a patch with docs and tests, I'd like to review it before you commit. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
participants (5)
-
Daniel Frey -
David Abrahams -
Gmane
-
Michael Marcin -
Ulrich Eckhardt