canceling noncopyable feature

[Michal]

Hallo Group Members

The following simple program allows for copying non-copyable function.

#include <boost/noncopyable.hpp>
#include <iostream>

using namespace std;

class Original: boost::noncopyable {

public:
Original() {
cout << "Original constructor" << endl;
}
Original(const Original& rhs) {
// this definition is the reason!!!!
}
};


int main(int argc, char *argv[])
{
Original o;
Original o2 = o;
return 0;
}


Do You know how is it possible?

best regards
Michal

 

[Victor Bazarov]

The following simple program allows for copying non-copyable function.

Why did you say "function"?

#include <boost/noncopyable.hpp>
#include <iostream>

using namespace std;

class Original: boost::noncopyable {

public:
Original() {
cout << "Original constructor" << endl;
}
Original(const Original& rhs) {
// this definition is the reason!!!!
}
};


int main(int argc, char *argv[])
{
Original o;
Original o2 = o;
return 0;
}


Do You know how is it possible?

What does "boost::nocopyable" do? How is it defined? Is it something
like a class with a private copy-constructor and copy assignment op? I
will base my answer on that assumption.

The copy-constructor of your "Original" class makes no attempt to copy
the base class, and the compiler will just use the default constructor
of the base class ('boost::noncopyable') to create the base class subobject.

That is not the same if you don't define the copy constructor yourself.
The compiler-defined copy-constructor uses *copy-construction* for all
base class subobjects and members. If you don't override that
behaviour, it would make it impossible if the base class' copy
constructor is inaccessible.

V

 

[Fraser Ross]

Thats a big problem. noncopyable doesn't do its job as well as I
thought. I think copy_protected would be a better name. It's a term
that is used. Noncopyable isn't a word.

Fraser.

 

[Fabio Fracassi]

Hallo Group Members

The following simple program allows for copying non-copyable function.

#include <boost/noncopyable.hpp>
#include <iostream>

using namespace std;

class Original: boost::noncopyable {

public:
  Original() {
    cout << "Original constructor" << endl;
  }
  Original(const Original& rhs) {
    // this definition is the reason!!!!
  }

};

int main(int argc, char *argv[])
{
  Original o;
  Original o2 = o;
  return 0;

}

Do You know how is it possible?

best regards
Michal

In C++ it is, in this special case unfortunally, possible to change
the access protection of a member during overloading.
In C++03 there is no (portable) way of inhibiting the behaviour of the
derived code.
C++0x will (IIRC) have the [[final]] attribute which would flag
"Original"s declaration/definition of a copy-ctor as an error.

So currently defining a copy-ctor in a class declared as (boost:
noncopyable is simply a mistake the compiler doesn't catch, so you
have to be careful during code reviews.

boost::noncopyable is more in the nature of documentation anyway, it
just concicely and conveniently declares your default copy-ctor/
operator private, giving it an explicit name which also documents the
reason for doing so. As a nice bonus you get somewhat better error
messages which contain noncopyable a lot, if your classes users do try
to copy it.

HTH

Fabio

 

[Vladimir Jovic]

Hallo Group Members

The following simple program allows for copying non-copyable function.

#include <boost/noncopyable.hpp>
#include <iostream>

using namespace std;

class Original: boost::noncopyable {

public:
Original() {
cout << "Original constructor" << endl;
}
Original(const Original& rhs) {
// this definition is the reason!!!!
}
};


int main(int argc, char *argv[])
{
Original o;
Original o2 = o;
return 0;
}


Do You know how is it possible?

best regards
Michal

Increase the warning level of the compiler. For g++ use next options:
-Wall -W

 

[Victor Bazarov]

Thats a big problem. noncopyable doesn't do its job as well as I
thought.

It doesn't necessarily means that it's not doing its job *as intended*,
and what you thought isn't necessarily what was intended. What do you
think it would do? Make any descendant class non-copyable? The whole
point is that the object of 'boost::noncopyable', well, can't be copied.
If you create your class based on this, and provide a way around
having to copy the base class, then here you are, your class is
copyable, but the base class is still not copied because it cannot be.

> I think copy_protected would be a better name. It's a term
that is used. Noncopyable isn't a word.

Why? The class isn't named 'providingalldescendantsfrombeingcopyable',
is it?

V

 

[mzdude]

It doesn't necessarily means that it's not doing its job *as intended*,
and what you thought isn't necessarily what was intended.  What do you
think it would do?  Make any descendant class non-copyable?

Yes. Perhaps changing the boost::noncopyable object to hide it's
default
ctor would help.

Then if you have a class derrived from it

class MyClass : boost::noncopyable
{
MyClass() : boost::noncopyable( some_trait ) {}

// then
MyClass( const MyClass &tocpy ) {}

would error because the compiler would subsitute the default
ctor for noncopyable which would be hidden.

I think that would provide a more intuitive interface.

It still wouldn't be perfect. You could intentionally get
around it by

MyClass::MyClass(const MyClass &toCopy )
: boost::noncopyable( some_trait )
{ }

but at least you would have to work at shooting
yourself.

 

[Victor Bazarov]

Yes. Perhaps changing the boost::noncopyable object to hide it's
default
ctor would help.

Then if you have a class derrived from it

class MyClass : boost::noncopyable
{
MyClass() : boost::noncopyable( some_trait ) {}

// then
MyClass( const MyClass &tocpy ) {}

would error because the compiler would subsitute the default
ctor for noncopyable which would be hidden.

I think that would provide a more intuitive interface.

It still wouldn't be perfect. You could intentionally get
around it by

MyClass::MyClass(const MyClass &toCopy )
: boost::noncopyable( some_trait )
{ }

but at least you would have to work at shooting
yourself.

Yes, and there will be somebody [more radical in their view] who'd come
and complain about that one. Wanna bet?

There is no way to satisfy everybody. Document what the class does and
what the intended use of it is, and let it go. 'boost::noncopyable' is
apparently intended to make a derived class such that the compiler
cannot produce a copy-ctor for it, which, provided the user does not try
to create a copy-ctor him-/herself, will make the derived class
non-copyalble as well.

There are always conditions. Nothing is absolute. Remember "null
references"?

V

 

[Noah Roberts]

Yes. Perhaps changing the boost::noncopyable object to hide it's
default
ctor would help.

Then if you have a class derrived from it

class MyClass : boost::noncopyable
{
MyClass() : boost::noncopyable( some_trait ) {}

// then
MyClass( const MyClass &tocpy ) {}

would error because the compiler would subsitute the default
ctor for noncopyable which would be hidden.

I don't think that noncopyable is meant to keep you from blowing your
brains out. It's meant to be a somewhat language enforced method of
documenting a known aspect of an interface. Anything that derives from
noncopyable is not meant to be copied, that's what the relationship is
documenting. The rest just keeps the compiler from self-generating the
copyable interface...I don't think it was ever meant to try rescuing
people from their own stupidity and I don't see how it even could.

 

[mzdude]

I don't think that noncopyable is meant to keep you from blowing your
brains out.  It's meant to be a somewhat language enforced method of
documenting a known aspect of an interface.  Anything that derives from
noncopyable is not meant to be copied, that's what the relationship is
documenting.  The rest just keeps the compiler from self-generating the
copyable interface...I don't think it was ever meant to try rescuing
people from their own stupidity and I don't see how it even could.

I know macros have a reputation for being *evil* but I think
the following documents the interface and doesn't bring along
any of the derived interface baggage.

#define NON_COPYABLE(x) private: \
##x(##x const & ); \
##x &operator=(##x const ref) const;

class MyClass
{
public:
MyClass();
NON_COPYABLE(MyClass)
};

 

[Victor Bazarov]

I know macros have a reputation for being *evil* but I think
the following documents the interface and doesn't bring along
any of the derived interface baggage.

#define NON_COPYABLE(x) private: \
##x(##x const & ); \
##x &operator=(##x const ref) const;

class MyClass
{
public:
MyClass();
NON_COPYABLE(MyClass)
};

I don't like the name. I think it should be different. Something like

#define DECLARE_NON_COPYABLE(x) ...

and if you leave out the closing semicolon, you will actually force the
user to write something similar to a class definition statement:

...
DECLARE_NON_COPYABLE(MyClass);

Of course, the problem is that the macro hides the change in the access
modifiers, everything that follows it will be 'private' unless you
change the modifier (which is sort of ugly). Besides, the class is
still copyable for friends and members...

When C++0x is out, we'll have the luxury of prohibiting copying for good
using the 'delete' syntax, even for members and friends...

V

 

[Noah Roberts]

I know macros have a reputation for being *evil* but I think
the following documents the interface and doesn't bring along
any of the derived interface baggage.

I don't think there's much baggage being brought along. The noncopyable
class is meant to be inherited from privately.

#define NON_COPYABLE(x) private: \
##x(##x const & ); \
##x &operator=(##x const ref) const;

class MyClass
{
public:
MyClass();
NON_COPYABLE(MyClass)
};

Except the error generated using the noncopyable class will have
"noncopyable" in it:


1>d:\dev_workspace\experimental\scratch\scratch\main.cpp(6) : error
C2248: 'boost::noncopyable_::noncopyable::noncopyable' : cannot access
private member declared in class 'boost::noncopyable_::noncopyable'
1> d:\boostvs39\include\boost\noncopyable.hpp(27) : see
declaration of 'boost::noncopyable_::noncopyable::noncopyable'
1> d:\boostvs39\include\boost\noncopyable.hpp(22) : see
declaration of 'boost::noncopyable_::noncopyable'
1> This diagnostic occurred in the compiler generated function
'myclass::myclass(const myclass &)'

your version:


1>d:\dev_workspace\experimental\scratch\scratch\main.cpp(15) : error
C2512: 'myclass' : no appropriate default constructor available
1>d:\dev_workspace\experimental\scratch\scratch\main.cpp(16) : error
C2248: 'myclass::myclass' : cannot access private member declared in
class 'myclass'
1> d:\dev_workspace\experimental\scratch\scratch\main.cpp(10) :
see declaration of 'myclass::myclass'
1> d:\dev_workspace\experimental\scratch\scratch\main.cpp(9) :
see declaration of 'myclass'

Also note the extra error regarding the default constructor with yours.
You can't let the default constructor be compiler generated in your
version.

The code:


#define NON_COPYABLE(x) private: \
##x(##x const&); \
##x &operator=(##x const ref) const;

struct myclass
{
NON_COPYABLE(myclass)
};

int main()
{
myclass a;
myclass b = a;
}

And with noncopyable myclass was defined as:

struct myclass : private boost::noncopyable
{
};

So the benefits of using boost::noncopyable over your macro would seem
to me to be:

1) the statement "noncopyable" in the diagnostic error.
2) the ability to use default constructors.

You say there's derived interface baggage but since you inherit
privately I don't think that even affects derived classes that might
also inherit from noncopyable. I don't see any pollution of the
interface since private inheritance is not an is-a type of relationship.

 

[mzdude]

I don't think there's much baggage being brought along.  The noncopyable
class is meant to be inherited from privately.

It would be interesting to see how many implementations
actually use the private. Refer to the OP.

So the benefits of using boost::noncopyable over your macro would seem
to me to be:

1) the statement "noncopyable" in the diagnostic error.
2) the ability to use default constructors.

You say there's derived interface baggage but since you inherit
privately I don't think that even affects derived classes that might
also inherit from noncopyable.  I don't see any pollution of the
interface since private inheritance is not an is-a type of relationship

As with engineering, there are always tradeoffs. The macro
solves the OP's problem. I never claimed it was a perfect
solution.

 

[Noah Roberts]

It would be interesting to see how many implementations
actually use the private. Refer to the OP.

The OP is inheriting privately. This is why I use the 'struct' keyword
rather than 'class'. Instinct is that you are inheriting publicly when
you inherit but it's actually private unless specified otherwise with
'class'.

But the question still stands of course and I don't know the answer.

 

[Balog Pal]

class Original: boost::noncopyable {

public:
Original() {
cout << "Original constructor" << endl;
}
Original(const Original& rhs) {
// this definition is the reason!!!!
}
};

The reason indeed.

int main(int argc, char *argv[])
{
Original o;
Original o2 = o;
return 0;
}
Do You know how is it possible?

How what is possible? You did define a copy ctor for Original, didn't you?
So it is working.
Whay your problem really is?

(note: the base class noncopyable is NOT copied in your copy ctor, it is
just default-inited like in the other ctor....)

 

[Fraser Ross]

Will you be suggesting this change to the Boost administrators?

Fraser.

 

[James Kanze]

The following simple program allows for copying non-copyable
function.

#include <boost/noncopyable.hpp>
#include <iostream>

using namespace std;

class Original: boost::noncopyable {

public:
Original() {
cout << "Original constructor" << endl;
}
Original(const Original& rhs) {
// this definition is the reason!!!!
}
};

int main(int argc, char *argv[])
{
Original o;
Original o2 = o;
return 0;
}

Do You know how is it possible?

You lied to the compiler (and to the reader of your code).
First, you said that the object wasn't copiable, then you said
it was. I may be of the old school, but I find lying
despicable.

The simple solution to this problem is to fire the person who
wrote the code.

 

[James Kanze]

I don't think that noncopyable is meant to keep you from
blowing your brains out. It's meant to be a somewhat language
enforced method of documenting a known aspect of an interface.
Anything that derives from noncopyable is not meant to be
copied, that's what the relationship is documenting.

I'd agree if you restrict it to directly deriving. When you
derive from boost::noncopyable, you're saying that copying is
*not* allowed. According to the LSP, however, a class which
derives from MyClass, above, is (and should be) allowed to
reactivate copy, since it is a loosening of the
"pre-conditions". (I know, the LSP generally applies to
functions, and not to objects. But I think you get the idea---a
derived class may add functionality, as long as all of the
functionality of the base class still works.)

FWIW, it's not totally inexistant in my derived classes to add a
private copy constructor, to support cloning, even though the
base class forbids copying (by deriving from
Gabi::Util::UncopiableObject---which is probably identical to
boost::noncopyable, but a bit older).

The rest just keeps the compiler from self-generating the
copyable interface...I don't think it was ever meant to try
rescuing people from their own stupidity and I don't see how
it even could.

The problem with trying to make anything idiot proof is that
nature keeps coming up with better idiots. No matter what you
do, it will fail in the end.

 

[Victor Bazarov]

Will you be suggesting this change to the Boost administrators?

What change do you mean? Please quote enough of the replied-to article,
otherwise the "train of thought" is off the tracks, so to speak.

V

 

[Fraser Ross]

"Victor Bazarov"

What change do you mean? Please quote enough of the replied-to
article, otherwise the "train of thought" is off the tracks, so to
speak.

class noncopyable {
protected:
explicit noncopyable (int) {}
~noncopyable () {}

private:
noncopyable ();
noncopyable (noncopyable const &);
noncopyable & operator=(noncopyable const &);
};


That is the suggestion. It prevents the OP's code compiling at the
expense of having to specifically initialise the base noncopyable.

Fraser.