how to reuse a copy constructor in an operator = function

J

JD

Hi,

My associate has written a copy constructor for a class. Now I need to add
an operator = to the class. Is there a way to do it without change her code
(copy constructor) at all? Your help is much appreciated.

JD
 
D

Daniel T.

JD said:
My associate has written a copy constructor for a class. Now I need to add
an operator = to the class. Is there a way to do it without change her code
(copy constructor) at all? Your help is much appreciated.

Yes. Make the attempt and I'm sure we can critique it for you.
 
K

Kai-Uwe Bux

JD said:
My associate has written a copy constructor for a class. Now I need to
add an operator = to the class. Is there a way to do it without change
her code (copy constructor) at all? Your help is much appreciated.

First off, why would adding an assignment operator affect the copy
constructor? In fact, you can very likely implement assignment in terms of
the copy-constructor. There are two methods:

(a) Copy-swap:

T& operator= ( T const & t ) {
T(t).swap( *this );
return ( *this );
}

You need to implement a swap() method for this one.


(b) Destruct-resurrect:

T& operator= ( T const & t ) {
if ( &t != this ) {
this->T::~T();
new (this) T (t);
}
return ( *this );
}

This method has gotchas and is not exception safe and is therefore not
applicable in all cases (but despite the harsh words that H. Sutter finds
about it, it can be justified sometimes).


Rule of thumb: generically (and always, when in doubt), go with (a).


Best

Kai-Uwe Bux
 
?

=?iso-8859-1?q?Elias_Salom=E3o_Helou_Neto?=

My associate has written a copy constructor for a class. Now I need to add
an operator = to the class. Is there a way to do it without change her code
(copy constructor) at all? Your help is much appreciated.

There is simpy no need to alter the copy constructor at all. Plain and
simply write an operator= to the class. But I realize, because of the
title of the post, that your question is not quite that. You actually
would like to reuse the copy constructor inside operator=.

I see no solution other than copying the constructor code (if you do
have acces to that) as a help to begin writing operator=. Whether this
falls under the "code reusing" category may be discussed.

Another solution I see, would be to provide an efficient swap() member
to your class and use the copy constructor like this:

class MyClass {

public:

MyClass( const MyClass& ); //Copy constructor

swap( MyClass& ); //Swap the content of two opbjects.
//This can be done rather efficiently by swapping only
//pointers and references to deep content.

MyClass& operator=( const MyClass& ); //See below.

};

MyClass& MyClass::eek:perator=( const MyClass& rhs )
{
MyClass temp( rhs ); //Reusing copy constructor code.
this->swap( temp );
return( *this );
}

Notice that this may be sub-optimal because some instructions in the
copy constructor will be much the same than some in the swap() method.
If one could assign to the this pointer, things could be made easier,
even without that swap() method.

This raises the question: why isn't the this pointer a valid lvalue?


Elias Salomão Helou neto.
 
J

James Kanze

First off, why would adding an assignment operator affect the
copy constructor? In fact, you can very likely implement
assignment in terms of the copy-constructor. There are two
methods:
(a) Copy-swap:
T& operator= ( T const & t ) {
T(t).swap( *this );
return ( *this );
}
You need to implement a swap() method for this one.
(b) Destruct-resurrect:
T& operator= ( T const & t ) {
if ( &t != this ) {
this->T::~T();
new (this) T (t);
}
return ( *this );
}
This method has gotchas and is not exception safe and is
therefore not applicable in all cases (but despite the harsh
words that H. Sutter finds about it, it can be justified
sometimes).

It also causes no end of problems if someone inherits from the
class.
Rule of thumb: generically (and always, when in doubt), go
with (a).

Only if you can implement a no-throw version of swap. This is
not generally the case, and if swap throws somewhere in the
middle, you may end up with an incoherent state. (Also, I
prefer the somewhat more explicit:

T&
T::eek:perator=( T const& other )
{
T tmp( other ) ;
swap( tmp ) ;
return *this ;
}

.. Somewhat clearer, IMHO.)

In general, of course, the rule for achieving the strong
exception safety guarantee (transactional integrity) is to do
everything that might raise an exception before modifying any
state. But you don't really need the strong guarantee that
often, and meeting it can be expensive.
 
J

James Kanze

On Nov 2, 10:30 pm, Elias Salomão Helou Neto <[email protected]>
wrote:

[...]
MyClass& MyClass::eek:perator=( const MyClass& rhs )
{
MyClass temp( rhs ); //Reusing copy constructor code.
this->swap( temp );
return( *this );
}
Notice that this may be sub-optimal because some instructions
in the copy constructor will be much the same than some in the
swap() method.

It may be sub-optimal, because the normal implementation of swap
will invoke the assignment operator. Infinite recursion is
definitely sub-optimal. Obviously, you need a special
implementation of the swap function, which doesn't use the
assignment operator. In practice, this idiom is only useful if
all of the sub-elements support a no-throw swap. This is the
case for the containers in the standard library, and should be
the case for most new code, provided the authors have kept
themselves up to date, but there is an awful lot of code
floating around where it is not the case.
If one could assign to the this pointer, things could be made
easier, even without that swap() method.

You'll have to explain that one to me. Assigning to the this
pointer would be tandamount to moving the object somewhere else
in memory, no?
This raises the question: why isn't the this pointer a valid
lvalue?

Because it doesn't make sense to make it one. What would it
mean if you changed the value of the this pointer?
 
K

Kai-Uwe Bux

James said:
It also causes no end of problems if someone inherits from the
class.

(a) That's one of the gotchas, and

(b) the reason for using

this->T::~T();

instead of

this->~T();

That actually takes care of _many_ problems this method can cause in the
context of inheritance.

So far, I only used it in the implementation of a few smart-pointers. In
that case, the self-assignment test can be improved to test for equality of
values, the destructor is non-virtual, and the resulting assignment is
no-throw. Moreover, there is no reason to inherit from those
smart-pointers, ever.

Only if you can implement a no-throw version of swap.

Well, if swap() is no-throw, then (a) is exception safe. But even with a
possibly throwing swap(), it will still do an assignment. You just don't
get the exception safety for the assignment.

Exception safety is not the only reason to use (a) (or (b) for that matter).
The main reason for using one of the two methods above is code-reuse: if
the assignment operation is non-trivial, it is generally much easier to get
it right by defining it in terms of the copy-constructor. E.g., in the
context of reference-counted smart-pointers, it appears to be somewhat
difficult to get the order of count-adjustments right.

Given that method (b) requires too much care, I tend to use (a) in a first
implementation of the assignment operator. Any subsequent change is
considered an optimization.
This is not generally the case, and if swap throws somewhere in the
middle, you may end up with an incoherent state.

Right And the same is true for any other non-exception safe implementation
of the assignment operator.

[snipped: style issue]
In general, of course, the rule for achieving the strong
exception safety guarantee (transactional integrity) is to do
everything that might raise an exception before modifying any
state. But you don't really need the strong guarantee that
often, and meeting it can be expensive.

Actually, I think that your remark from above applies here, too: it is
simply not possible to implement an exception safe assignment operator in
all cases, and there are cases where it is possible but not feasible.

Moreover, I think, the scenarios are more or less the same as for the
no-throw swap() problem: you might need to inherit from a base class beyond
your control, say a library component, and that class does not provide the
necessary means. Also, I have yet to come across a case where an exception
safe assignment is feasible but a non-throwing swap() is not.


Best

Kai-Uwe Bux
 
?

=?iso-8859-1?q?Elias_Salom=E3o_Helou_Neto?=

On Nov 2, 10:30 pm, Elias Salomão Helou Neto <[email protected]>
wrote:

[...]
MyClass& MyClass::eek:perator=( const MyClass& rhs )
{
MyClass temp( rhs ); //Reusing copy constructor code.
this->swap( temp );
return( *this );
}
Notice that this may be sub-optimal because some instructions
in the copy constructor will be much the same than some in the
swap() method.

It may be sub-optimal, because the normal implementation of swap
will invoke the assignment operator. Infinite recursion is
definitely sub-optimal. Obviously, you need a special
implementation of the swap function, which doesn't use the
assignment operator. In practice, this idiom is only useful if
all of the sub-elements support a no-throw swap. This is the
case for the containers in the standard library, and should be
the case for most new code, provided the authors have kept
themselves up to date, but there is an awful lot of code
floating around where it is not the case.

Even if the assignement operator (for the actual class) is not used in
the swap member (and it cannot be, because it does not exist,
remeber?), this will be sub-optimal because many fields will be
assigned (with their assignement operator) in the copy constructor,
only to be reassigned then, via the swap member. With a
straightforward (without trying to reuse the copy constructor)
implementation of operator= this would not be needed. I was not even
thinking about infinite recursion.
You'll have to explain that one to me. Assigning to the this
pointer would be tandamount to moving the object somewhere else
in memory, no?

Somehow, yes.
Because it doesn't make sense to make it one. What would it
mean if you changed the value of the this pointer?

It does make sense: I could change the this pointer to point to
another object of the same (or even derived) classes, without the
placement new hassle. Placement new, as I see, is meant to be used to
ensure correct placement of the object when it is needed. In this case
it ended up being used as a workaround because this is not an lvalue
(not quite that, keep on reading). Would not the case (b) from Kai-Uwe
Bux be better if written like the following?

T& T::eek:perator= ( T const & t ) {
T *copy = new T( *this ); //Reuses copy ctr.
T *temp;
temp = this;
this = copy; //Not c++!
delete temp;
return ( *this );
}

The main issue here is that it would invalidate any pointer/reference
to the object, and it is definitely dangerous. If this is the case for
your class (and it will be to most classes), Kai-Uwe Buxs solution is
definitely better, even if not perfect.

The point is that in some, though rare, cases assigning to the this
pointer makes sense. It would, however, be extremely dangerous and
perhaps the standard is right about avoiding it.

Elias Salomão Helou Neto.
 
A

Andrew Koenig

(a) That's one of the gotchas, and
(b) the reason for using

this->T::~T();

instead of

this->~T();

That actually takes care of _many_ problems this method can cause in the
context of inheritance.

Well yes, but it misses the most important problem.

Suppose you derive from T, and write an assignment operator in the derived
class that calls the one from the base class:

class T1: public T {

// ...
T1& operator=(coust T1& x) {
this->T::eek:perator=(x);
// Do some other stuff here
return *this;
}

// ...
};

When T1::eek:perator= calls T::eek:perator=, T::eek:perator= will stealthily destroy
the object, which had dynamic type T1, and replace it with a new object that
has dynamic type T. Of course, the compiler won't know about this for the
purpose of static type checking.

The result will be chaos.
 
K

Kai-Uwe Bux

Andrew said:
Well yes, but it misses the most important problem.

Suppose you derive from T, and write an assignment operator in the derived
class that calls the one from the base class:

class T1: public T {

// ...
T1& operator=(coust T1& x) {
this->T::eek:perator=(x);
// Do some other stuff here
return *this;
}

// ...
};

When T1::eek:perator= calls T::eek:perator=, T::eek:perator= will stealthily
destroy the object, which had dynamic type T1, and replace it with a new
object that
has dynamic type T. Of course, the compiler won't know about this for the
purpose of static type checking.

The result will be chaos.

I think, that would happen if you did this->~T(). The version this->T::~T()
does not call the derived destructor:


#include <iostream>

struct X {

virtual
~ X ( void ) {
std::cout << "destroying X\n";
}

X & operator= ( X const & other ) {
if ( this != &other ) {
this->X::~X();
new (this) X (other);
}
return ( *this );
}

};

struct Y : public X {

~ Y ( void ) {
std::cout << "destroying Y\n";
}

Y & operator= ( Y const & other ) {
X::eek:perator=( other );
return (*this );
}

};


int main ( void ) {
Y* a_ptr = new Y;
Y* b_ptr = new Y;
*a_ptr = *b_ptr;
// intentionally leaking so that the Y-destructor remain silent
}


On my machine, this prints just "destroying X". Now, of course, if you are
right, then this program probably has undefined behavior; but I think, it's
well-defined and does not destroy any Y objects in the assignment.



Best

Kai-Uwe Bux
 
J

James Kanze

(a) That's one of the gotchas, and
(b) the reason for using

instead of

That actually takes care of _many_ problems this method can
cause in the context of inheritance.

I don't think so. First, if the destructor of the T is
non-trivial, you've probably got undefined behavior---I don't
see what else it could be if you destruct a base class without
destructing the derived class. (This is also true if the
destructor of T isn't virtual, of course.) And of course, After
the new, you have an object of the base class type; any use of
it as the derived class it originally was is undefined behavoir
as well.
So far, I only used it in the implementation of a few
smart-pointers. In that case, the self-assignment test can be
improved to test for equality of values, the destructor is
non-virtual, and the resulting assignment is no-throw.
Moreover, there is no reason to inherit from those
smart-pointers, ever.

If 1) you can be sure that no one will ever derive from the
class, 2) you can be sure that the copy constructor cannot
throw, and 3) you can be sure that anyone modifying the code is
aware of these constraints, and will modify the assignment
operator if they cease to hold, then it's safe. I've never
really seen a case where I feel safe concerning 3, and 1 and 2
generally imply a class so simple that it's not worth the effort
using a dubious idiom.
Well, if swap() is no-throw, then (a) is exception safe. But
even with a possibly throwing swap(), it will still do an
assignment. You just don't get the exception safety for the
assignment.

Nor any other benefits.
Exception safety is not the only reason to use (a) (or (b) for
that matter). The main reason for using one of the two
methods above is code-reuse: if the assignment operation is
non-trivial, it is generally much easier to get it right by
defining it in terms of the copy-constructor. E.g., in the
context of reference-counted smart-pointers, it appears to be
somewhat difficult to get the order of count-adjustments
right.

It depends. If I'm writing such a pointer today, I'd define a
nothrow swap, and use it. But long before I was aware of the
idiom, I knew enough to increment the pointer being copied
first, and decrement the one being overwritten second.
Alternatively, you could just check whether the two pointers
pointed to the same thing first.
Given that method (b) requires too much care, I tend to use
(a) in a first implementation of the assignment operator. Any
subsequent change is considered an optimization.

If I'm more or less in control of all of the classes of the
sub-objects, or the sub-objects are not class types, I'll
generally go with (a) too, unless the assignment operator is
really, really trivial. (I generally won't bother, for example,
if the object only contains arithmetic types: integers, floating
point and enums.) I generally won't use either (a) or (b) if
the object contains class type sub-objects which do not
themselves implement a no throw swap. In such cases, I'm
dealing with legacy code, or third party software, and I have to
actually think about what I want to do.
Right And the same is true for any other non-exception safe
implementation of the assignment operator.

Not necessarily.
 
J

James Kanze

On Nov 2, 10:30 pm, Elias Salomão Helou Neto <[email protected]>
wrote:
[...]
MyClass& MyClass::eek:perator=( const MyClass& rhs )
{
MyClass temp( rhs ); //Reusing copy constructor code.
this->swap( temp );
return( *this );
}
Notice that this may be sub-optimal because some instructions
in the copy constructor will be much the same than some in the
swap() method.
It may be sub-optimal, because the normal implementation of swap
will invoke the assignment operator. Infinite recursion is
definitely sub-optimal. Obviously, you need a special
implementation of the swap function, which doesn't use the
assignment operator. In practice, this idiom is only useful if
all of the sub-elements support a no-throw swap. This is the
case for the containers in the standard library, and should be
the case for most new code, provided the authors have kept
themselves up to date, but there is an awful lot of code
floating around where it is not the case.
Even if the assignement operator (for the actual class) is not
used in the swap member (and it cannot be, because it does not
exist, remeber?),

Well, the thing that worries me most is someone implementing a
member swap function:

void
MyClass::MyClass( MyClass& other )
{
std::swap( *this, other ) ;
}

If the only goal is to provide the class with a swap function,
this is the obvious way to do it. Use this function in the
assignment operator, however, and you're going to have a real
performance problem---the operator will only stop when it runs
out of stack.
this will be sub-optimal because many fields will be assigned
(with their assignement operator) in the copy constructor,
only to be reassigned then, via the swap member. With a
straightforward (without trying to reuse the copy constructor)
implementation of operator= this would not be needed. I was
not even thinking about infinite recursion.

Even without infinite recursion, I agree. In general, I will
take the risk that if the class type of a sub-element provides a
member swap, it will be more efficient to use it; my rule is to
use the swap idiom only if the class consists only of class
types with a member swap function or non-class types.
Somehow, yes.
It does make sense: I could change the this pointer to point
to another object of the same (or even derived) classes,
without the placement new hassle.

But what does this do for you. The object (in memory) I called
the function on isn't going anywhere, and all of the users of
the object will still look for it in the old place.
Placement new, as I see, is meant to be used to
ensure correct placement of the object when it is needed.

For example. The important point about placement new is that
the address is being supplied by the caller. He's putting the
object where he expects it. You can't change where the caller
expects to find the object from inside the function.
In this case it ended up being used as a workaround because
this is not an lvalue (not quite that, keep on reading). Would
not the case (b) from Kai-Uwe Bux be better if written like
the following?
T& T::eek:perator= ( T const & t ) {
T *copy = new T( *this ); //Reuses copy ctr.
T *temp;
temp = this;
this = copy; //Not c++!
delete temp;
return ( *this );
}
The main issue here is that it would invalidate any pointer/reference
to the object,

And since to call the operator, you definitly had such a
pointer, or at least knew its address, and since you wouldn't
bother calling such a function unless you were going to use the
object again...

Think of what happens in the case of:

T anObject ;
anObject = someOtherT ;
and it is definitely dangerous. If this is the case for your
class (and it will be to most classes), Kai-Uwe Buxs solution
is definitely better, even if not perfect.

Kai-Uwe's solution doesn't really work either.
The point is that in some, though rare, cases assigning to the
this pointer makes sense. It would, however, be extremely
dangerous and perhaps the standard is right about avoiding it.

It doesn't make any sense with the C++ object model. It's hard
to see how the object model could be rewritten so that it would
make sense. In the C++ object model, an object is identified by
its address. Change the address, and you have a different
object.
 
K

Kai-Uwe Bux

James said:
I don't think so. First, if the destructor of the T is
non-trivial, you've probably got undefined behavior---I don't
see what else it could be if you destruct a base class without
destructing the derived class.

I don't know; but it could be true. [3.8/7] comes to mind.

In practice, of course, you will not see a problem; and one can argue that
[3.8/7] is overspecified. The last item in the list seems to be written
with this->~T() in mind. (The example that follows this provision in the
current draft n2461 seems to support that interpretation.)
(This is also true if the
destructor of T isn't virtual, of course.) And of course, After
the new, you have an object of the base class type; any use of
it as the derived class it originally was is undefined behavoir
as well.

Huh? I am not following. The derived object is still there and can be used
as an object of the derived type. I don't see how its type (static or
dynamic) could have changed in the process. Here you argue as if the
destructor call had torn down the ambient derived object, whereas above you
worried about destructing the subobject while not destroying the ambient
object.

If 1) you can be sure that no one will ever derive from the
class, 2) you can be sure that the copy constructor cannot
throw, and 3) you can be sure that anyone modifying the code is
aware of these constraints, and will modify the assignment
operator if they cease to hold, then it's safe. I've never
really seen a case where I feel safe concerning 3, and 1 and 2
generally imply a class so simple that it's not worth the effort
using a dubious idiom.



Nor any other benefits.

What other benefits do you expect from an assignment operator?

It depends. If I'm writing such a pointer today, I'd define a
nothrow swap, and use it. But long before I was aware of the
idiom, I knew enough to increment the pointer being copied
first, and decrement the one being overwritten second.
Alternatively, you could just check whether the two pointers
pointed to the same thing first.

It's easy to have more complicated situations. When the destructor and the
copy-constructor are getting longish, it is usually a pain to write an
assignment operator from scratch. In those cases, it increases
maintainability to use the copy-swap thing.


[snip]


Best

Kai-Uwe Bux
 
J

James Kanze

I don't know; but it could be true. [3.8/7] comes to mind.
In practice, of course, you will not see a problem; and one
can argue that [3.8/7] is overspecified. The last item in the
list seems to be written with this->~T() in mind. (The example
that follows this provision in the current draft n2461 seems
to support that interpretation.)

I'm not sure that the problem is 3.8/7; I think that that
concerns complete objects. The problem here is that you are
destructing a subobject, without notifying the object it
contains. In practice, it would probably work with members, but
I don't see how a compiler could make it work for base classes;
the usual object model involves base and derived classes sharing
some (or all) of the vptr, for example.

In practice, it doesn't work with g++, Sun CC or VC++. Try the
following:

class B
{
public:
virtual ~B() {}
virtual void f() ;
B& operator=( B const& other ) ;
} ;

class D : public B
{
public:
virtual void f() ;
} ;

B&
B::eek:perator=(
B const& other )
{
if ( this != &other ) {
this->B::~B() ;
new (this ) B( other ) ;
}
return *this ;
}

void
B::f()
{
std::cout << "B::f()" << std::endl ;
}

void
D::f()
{
std::cout << "D::f()" << std::endl ;
}

int main()
{
D aD ;
D* pD = &aD ;
std::cout << typeid( aD ).name() << std::endl ;
std::cout << "from obj.: " ;
aD.f() ;
std::cout << "from ptr.: " ;
pD->f() ;
aD = D() ;
std::cout << typeid( aD ).name() << std::endl ;
std::cout << "from obj.: " ;
aD.f() ;
std::cout << "from ptr.: " ;
pD->f() ;
return 0 ;
}

Not in particular the output of the last two lines, and that
you're calling f() on the *same* object in them. (If the code
were correct, of course, you'd get D::f() everywhere.)
Huh? I am not following. The derived object is still there and
can be used as an object of the derived type.

No it can't. That's the problem. The derived object and its
base sub-objects are intimely linked; in all of the
implementations I know, for example, they share a vptr (and this
is definitly allowed by the standard).

The situation would be even worse if virtual inheritance were
involved, because the position of the virtual base class in the
complete object depends on the actual type---and is usually
determined by more or less the same mechanism as virtual
functions.
I don't see how its type (static or dynamic) could have
changed in the process.

Because the dynamic type of an object is determined by the last
constructor called on the memory which contains the object.
Calling "new (this) Base" means that the object at that address
now has the dynamic type Base.
Here you argue as if the destructor call had torn down the
ambient derived object, whereas above you worried about
destructing the subobject while not destroying the ambient
object.

The destructor to the base will destroy part of the derived
class, conceptually, and in many cases really. When you execute
the destructor, the dynamic type of the object "isA" base. The
compiler will normally modify any RTTI (vptr, etc.) to reflect
this; if it doesn't in simple cases, it is only because the
compiler is able to determine that the vptr would not actually
be used after the change.

Similarly, the constructor constructs an object of complete type
B. After construction, the memory no longer contains a D; it
contains a B. §3.8/4 is certainly relevant here: by calling the
constructor of the base type, you have reused the memory which
previously contained a D. And we can see the motivation for
§3.8/7 in the above code; the compiler knows that the object at
the address &aD has type D, so whenever is dealing with an
object that it knows to be at this address (the object itself,
but not the contents of the pointer), it can treat it as having
type D.
What other benefits do you expect from an assignment operator?

I'm talking about the swap idiom: if I use a particular idiom,
it's because I expect some benefit from doing so. If all of the
sub-objects have a no-throw, relativly optimized swap, that's no
problem with the swap idiom because the benefits greatly
outweigh the cost. But if some subclass doesn't implement it,
then I'm not really implementing the swap idiom; I'm
implementing something that looks like it, but that isn't. I'm
confusing the reader (who recognizes the idiom, and assumes
something that isn't true), and I'm likely paying an unnecessary
performance penalty.
 
K

Kai-Uwe Bux

[lots of very enlightening techincal information and explanation snipped]
Because the dynamic type of an object is determined by the last
constructor called on the memory which contains the object.
Calling "new (this) Base" means that the object at that address
now has the dynamic type Base.

Thank you very much for the detailed explanation. I wasn't aware of that.
Now I see that you were right from the beginning: the destroy-resurrect
thingy can only be done for final classes (of which there are virtually
none.)

The main point of destroy-resurrect is code-reuse. I hope, the following
variation achieves that effect legally and without undefined behavior:

class X {

void release ( void ) {
// release all resources;
}

public:

X & operator= ( X const & other ) {
if ( this != &other ) {
release();
// allocate resources and reassign members
}
return ( *this );
}

X ( X const & other ) {
this->operator= ( other );
}

~X ( void ) {
release();
}

};

It has the disadvantage that the copy-constructor will now use assignment
instead of initialization, which means that the members will have to be
default-constructed. Alas.


[snip]
I'm talking about the swap idiom: if I use a particular idiom,
it's because I expect some benefit from doing so. If all of the
sub-objects have a no-throw, relativly optimized swap, that's no
problem with the swap idiom because the benefits greatly
outweigh the cost. But if some subclass doesn't implement it,
then I'm not really implementing the swap idiom; I'm
implementing something that looks like it, but that isn't. I'm
confusing the reader (who recognizes the idiom, and assumes
something that isn't true), and I'm likely paying an unnecessary
performance penalty.

You'r engaged in premature optimization :)

I guess it's a matter of how copy-swap is advertised. To me, exception
safety is only one of the reasons to go with copy-swap, and it does not
always apply. I see that copy-swap by and in itself does not make the
assignment operator exception safe. What it does is propagating exception
safety from members and base classes to the class under consideration. If
one of the subobjects breaks the chain, all exception safety is lost. (I am
probably as guilty as others of not stressing that point clear when talking
about copy-swap).

To me, however, the main benefit of copy-swap is that it is simpler to get a
swap() right than an assignment operator (since you only have to move
resources around, no release of resources is required). Also, a swap
requires usually less code than an assignment operator (which sort of
combines lines from the destructor and from the copy-constructor). Whenever
I want a swap method (and I usually do since I alway have classes that
represent values), I have the strong inclination of writing the assignment
operator in 3 lines and be done with it. There is nothing wrong in saving
brain cycles. The CPU can easily make up for it, and if it can't, then I
optimize what is needed.

With regard to the "confusing the reader" point: the copy-swap idiom will
only trick those into false expectations who think that exception safety is
its main point. Whether that applies to a certain community, is a cultural
thing. Of course, if your teammates have that opinion, you are well-advised
to code accordingly. And you maybe right that in most places the
expectation would be that copy-swap indicates exception safety.


Best

Kai-Uwe Bux
 
P

Pete Becker

I think, that would happen if you did this->~T(). The version this->T::~T()
does not call the derived destructor:


#include <iostream>

struct X {

virtual
~ X ( void ) {
std::cout << "destroying X\n";
}

X & operator= ( X const & other ) {
if ( this != &other ) {
this->X::~X();
new (this) X (other);
}
return ( *this );
}

};

struct Y : public X {

~ Y ( void ) {
std::cout << "destroying Y\n";
}

Y & operator= ( Y const & other ) {
X::eek:perator=( other );
return (*this );
}

};


int main ( void ) {
Y* a_ptr = new Y;
Y* b_ptr = new Y;
*a_ptr = *b_ptr;
// intentionally leaking so that the Y-destructor remain silent
}


On my machine, this prints just "destroying X". Now, of course, if you are
right, then this program probably has undefined behavior; but I think, it's
well-defined and does not destroy any Y objects in the assignment.

Now finish the experiment: what does delete a_ptr do? I haven't tried
it, but generally, it will destroy the X subobject and not the Y part,
because new(this) X(other) jams in X's vtable in place of Y's.
 
K

Kai-Uwe Bux

Pete said:
Now finish the experiment: what does delete a_ptr do? I haven't tried
it, but generally, it will destroy the X subobject and not the Y part,
because new(this) X(other) jams in X's vtable in place of Y's.

Jup.

int main ( void ) {
Y* a_ptr = new Y;
Y* b_ptr = new Y;
*a_ptr = *b_ptr;
delete a_ptr;
}

destroying X
destroying X


James Kanze has explained that in detail elsethread.


Thanks a lot

Kai-Uwe Bux
 
J

James Kanze

[...]
Thank you very much for the detailed explanation. I wasn't
aware of that. Now I see that you were right from the
beginning: the destroy-resurrect thingy can only be done for
final classes (of which there are virtually none.)

The real rule is that once it appears in a hierarchy, *all*
further derived classes must use it exclusively. Which means
that even the most trivially derived class requires a user
defined assignment operator. Document it as you will, some one
is going to forget.
The main point of destroy-resurrect is code-reuse.

I understand that. I stumbled on the same idea something like
15 years ago. Got taken down by Steve Clamage, and had to admit
that his arguments were justified. (And that was before
exceptions.) In my case, the goal was to ensure correct
behavior in the case of virtual inheritance (where the
intermediate classes can't call the base's operator= directly
without risking it being called several times). So I couldn't
even use the argument that inheritance didn't make sense for the
types I was using it for.
I hope, the following variation achieves that effect legally
and without undefined behavior:
class X {
void release ( void ) {
// release all resources;
}
public:
X & operator= ( X const & other ) {
if ( this != &other ) {
release();
// allocate resources and reassign members
}
return ( *this );
}
X ( X const & other ) {
this->operator= ( other );
}
~X ( void ) {
release();
}
};
It has the disadvantage that the copy-constructor will now use
assignment instead of initialization, which means that the
members will have to be default-constructed. Alas.

G++ used something similar in their older versions of
std::string; always construct an empty string, and go on from
there using assignment. Depending on the actual class, it could
be a reasonable solution. (If the class involves dynamic
memory, and the constructors all set the pointer to null first,
for example.)
[snip]
I'm talking about the swap idiom: if I use a particular idiom,
it's because I expect some benefit from doing so. If all of the
sub-objects have a no-throw, relativly optimized swap, that's no
problem with the swap idiom because the benefits greatly
outweigh the cost. But if some subclass doesn't implement it,
then I'm not really implementing the swap idiom; I'm
implementing something that looks like it, but that isn't. I'm
confusing the reader (who recognizes the idiom, and assumes
something that isn't true), and I'm likely paying an unnecessary
performance penalty.
You'r engaged in premature optimization :)

I was afraid someone was going to say that.

If copy/swap were the established idiom for *all* assignment
operators, then there would be some truth to it, as well. The
real argument is that I just don't expect to see it unless it is
guaranteeing exception safety.

And of course, if you do end up using std::swap on one of the
sub-elements, you've copied it an awful lot of times (3 or 4,
rather than just one).
I guess it's a matter of how copy-swap is advertised. To me,
exception safety is only one of the reasons to go with
copy-swap, and it does not always apply. I see that copy-swap
by and in itself does not make the assignment operator
exception safe. What it does is propagating exception safety
from members and base classes to the class under
consideration. If one of the subobjects breaks the chain, all
exception safety is lost. (I am probably as guilty as others
of not stressing that point clear when talking about
copy-swap).

The important point (with regards to exception safety) is that
none of the swap functions can throw. In particular, *if* you
have a member of class type which does not have a swap member
function, you cannot expect the exception safety guarantees if
you use std::swap to swap it; std::swap will use the copy
constructor and the assignment operator.
To me, however, the main benefit of copy-swap is that it is
simpler to get a swap() right than an assignment operator
(since you only have to move resources around, no release of
resources is required). Also, a swap requires usually less
code than an assignment operator (which sort of combines lines
from the destructor and from the copy-constructor). Whenever I
want a swap method (and I usually do since I alway have
classes that represent values), I have the strong inclination
of writing the assignment operator in 3 lines and be done with
it. There is nothing wrong in saving brain cycles. The CPU can
easily make up for it, and if it can't, then I optimize what
is needed.

*If* I can furnish an effective swap function, then I certainly
do use it in the assignment operator. My criticism is more
along the lines that unless all of your class type sub-objects
have furnished an effective swap function, you generally can't
either.
With regard to the "confusing the reader" point: the copy-swap
idiom will only trick those into false expectations who think
that exception safety is its main point. Whether that applies
to a certain community, is a cultural thing. Of course, if
your teammates have that opinion, you are well-advised to code
accordingly. And you maybe right that in most places the
expectation would be that copy-swap indicates exception
safety.

Well, it applies to me. Perhaps because I was writing
complicated assignment operators long before the swap idiom
became current, or because I still have to deal with a lot of
code which doesn't support it.

But seriously, if you were implementing something like
std::complex, would you use the swap idiom?
 
K

Kai-Uwe Bux

James Kanze wrote:

[agreement snipped]
But seriously, if you were implementing something like
std::complex, would you use the swap idiom?

In that case, I would trust that the compiler generates the right
copy-constructor, assignment operator, and destructor. The copy-swap idiom
(as any other way of defining the crucial three) only applies if there is
something non-trivial to do anyway.


Best

Kai-Uwe Bux
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,483
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top