tags:

views:

673

answers:

5

In an answer to http://stackoverflow.com/questions/700588/is-it-safe-to-store-objects-of-a-class-which-has-an-stdautoptr-as-its-member-v I stated that a class that contained an auto_ptr could be stored in a vector provided the class had a user-defined copy constructor.

There were several comment suggesting that this was not the case, so this question is an attempt to clear the issue up. Consider the following code:

#include <memory>
#include <vector>
using namespace std;

struct Z {};

struct A {

    A( Z z ) 
     : p( new Z(z) ) {} 

    A( const A & a ) 
     : p( a.p.get() ? new Z( *a.p.get()) : 0 ) {}

    // no assigment op or dtor defined by intent

    auto_ptr <Z> p;
};

int main() {
    vector <A> av;    
    Z z;     
    A a(z);
    av.push_back( a );  
    av.push_back( A(z) ); 
    av.clear();    
}

Please examine the above & in your reply indicate where undefined behaviour in the meaning of the C++ Standard could occur for this particular class used in this particular way. I am not interested whether the class is useful, well-behaved, sortable, or how it performs under exceptions.

Please also note that this is not a question about the validity of creating a vector of auto_ptrs - I am well aware of the issues regarding that.

Thanks all for your inputs on what in retrospect is probably a rather silly question. I guess I focussed too much on the copy ctor & forgot about assignment. The lucky winner of my acceptance points (and points mean prizes!) is litb for a typically exhaustive explanation (sorry earwicker)

A: 

Since the regular auto_ptr semantic could suggests that the ownership is passed during the copying, I would rather use here boost::scoped_ptr. Of course the assignment operator is missing.

oo_olo_oo
+4  A: 

I don't think it's necessarily the case that the above code will even compile. Surely the implementor of std::vector is at liberty to require an assignment operator to be available, from const A&?

And having just tried it, it doesn't compile on Visual Studio C++ 2008 Service Pack 1:

binary '=' : no operator found which takes a right-hand operand of type 'const A' (or there is no acceptable conversion)

My guess is that, on the guidance of Herb Sutter, the container classes in VC++ make every effort to impose the standard requirements on their type parameters, specifically to make it hard to use auto_ptr with them. They may have overstepped the boundaries set by the standard of course, but I seem to remember it mandating true assignment as well as true copy construction.

It does compile in g++ 3.4.5, however.

Daniel Earwicker
Yes, now you remind me I remember that too - I guess that answers my question :-(
anon
Well where my's green tick for being a clever boy then? :)
Daniel Earwicker
All good things come to he who waits.
anon
Oh man. The anticipation is almost unbearable!
Daniel Earwicker
that is exactly what i told him on his answer on the other question. you have to be able to assign / copy from "const T" because the requirements state it. not because it might be useful or anything like that. +1 indeed
Johannes Schaub - litb
here is the output of gcc 4.1: http://codepad.org/P0uxxqxH
Johannes Schaub - litb
+1  A: 

What about the following?

cout << av[ 0 ] << endl;

Also, conceptually, a copy should leave the item copied from unchanged. This is being violated in your implementation.

(It is quite another thing that your original code compiles fine with g++ -pedantic ... and Comeau but not VS2005.)

dirkgently
"Also, conceptually, a copy should leave the item copied from unchanged." - try telling that to auto_ptr!
Daniel Earwicker
My question wasn't about the usefulness of the class - obviously it is completely broken, but only about UB. But as Earwicker pointed out I think VC++ may be right for once. Interesting about Comeau though...
anon
@Earwicker: That was my point about auto_ptrs.
dirkgently
@Neil Butterworth: You are only looking at part of the class and a special construct that does not invoke UB. The point of my example.
dirkgently
+4  A: 

Objects stored in containers are required to be "CopyConstructable" as well as "Assignable" (C++2008 23.1/3).

Your class tries to deal with the CopyConstructable requirement (though I'd argue it still doesn't meet it - I edited that argument out since it's not required and because it's arguable I suppose), but it doesn't deal with the Assignable requirement. To be Assignable (C++2008 23.1/4), the following must be true where t is a value of T and u is a value of (possibly const) T:

t = u returns a T& and t is equivalent to u

The standard also says in a note (20.4.5/3): "auto_ptr does not meet the CopyConstructible and Assignable requirements for Standard Library container elements and thus instantiating a Standard Library container with an auto_ptr results in undefined behavior."

Since you don't declare or define an assignment operator, an implicit one will be provided that uses the auto_ptr's assignment operator, which definitely makes t not equivalent to u, not to mention that it won't work at all for "const T u" values (which is what Earwicker's answer points out - I'm just pointing out the exact portion(s) of the standard).

Michael Burr
+3  A: 

Trying to put the list of places together that makes the example undefined behavior.

#include <memory>
#include <vector>
using namespace std;

struct Z {};

struct A {

    A( Z z ) 
        : p( new Z(z) ) {} 

    A( const A & a ) 
        : p( a.p.get() ? new Z( *a.p.get()) : 0 ) {}

    // no assigment op or dtor defined by intent

    auto_ptr <Z> p;
};

int main() {
    vector <A> av;  
    ...
}

I will examine the lines up to the one where you instantiate the vector with your type A. The Standard has to say

In 23.1/3:

The type of objects stored in these components must meet the requirements of CopyConstructible types (20.1.3), and the additional requirements of Assignable types.

In 23.1/4 (emphasis mine):

In Table 64, T is the type used to instantiate the container, t is a value of T, and u is a value of (possibly const) T.

+-----------+---------------+---------------------+
|expression |return type    |postcondition        |
+-----------+---------------+---------------------+
|t = u      |T&             |t is equivalent to u |
+-----------+---------------+---------------------+

Table 64

In 12.8/10:

If the class definition does not explicitly declare a copy assignment operator, one is declared implicitly. The implicitly-declared copy assignment operator for a class X will have the form

X& X::operator=(const X&)

if

  • each direct base class B of X has a copy assignment operator whose parameter is of type const B&, const volatile B& or B, and
  • for all the nonstatic data members of X that are of a class type M (or array thereof), each such class type has a copy assignment operator whose parameter is of type const M&, const volatile M& or M.

Otherwise, the implicitly declared copy assignment operator will have the form

X& X::operator=(X&)

(Note the last and second last sentence)

In 17.4.3.6/1 and /2:

In certain cases (replacement functions, handler functions, operations on types used to instantiate standard library template components), the C++ Standard Library depends on components supplied by a C++ program. If these components do not meet their requirements, the Standard places no requirements on the implementation.

In particular, the effects are undefined in the following cases:

  • for types used as template arguments when instantiating a template component, if the operations on the type do not implement the semantics of the applicable Requirements subclause (20.1.5, 23.1, 24.1, 26.1). Operations on such types can report a failure by throwing an exception unless otherwise specified.

Now, if you look at the specification of auto_ptr you will note it has a copy-assignment operator that takes a non-const auto_ptr. Thus, the implicitly declared copy assignment operator of your class will also take a non-const type as its parameter. If you read the above places carefully, you will see how it says that instantiating a vector with your type as written is undefined behavior.

Johannes Schaub - litb
But my class has an _explicitly_ declared copy constructor, so I don't see how this applies.
anon
it does not apply to that at all. it's the copy assignment operator that is missing - not the copy constructor. i would say as defined, your copy constructor is all fine.
Johannes Schaub - litb
oops - my misread - sorry
anon
Neil, c++98 standard had a typo that said "copy constructor" at one particular place (and i take my quotes from c++98 - only have that). in a revisions list i read c++03 fixed that. maybe it was this that made you think of a copy constructor :) (i already fixed it a hour ago)
Johannes Schaub - litb