Does using std::auto_ptr data members invoke UB?

Question

As a followup to Class containing auto_ptr stored in vector, I believe the unstated conclusion there is that it is OK to use classes with an auto_ptr member as container elements, so long as both copy construction and copy assignment are user-defined (so as not to invoke the auto_ptrs copy constructor and copy assignment). Is there anything about this that violates the Standard Library requirements?

Please tell me if there is anything wrong with the following, as I'd like to start doing it idiomatically:

#include <memory>
#include <algorithm>

class Y { /* ... */ };
class Z : public Y { /* ... */ };

class X {
    public:
        X() : ap(new Z()) {}
        X(const X& other) : ap(other.ap->clone()) {}
        X& operator=(X other) { swap(other); return *this; } // any problem with swap?
        void swap(X& other) { std::swap(ap, other.ap); }
        // note no destructor necessary
    private:
        std::auto_ptr<Y> ap; // Y provides clone()
};

Note that one thing I've realized is that with this approach, Y's class definition must be in scope for X's definition (as opposed to just forward-declared when a "raw" pointer to Y is used). This is because when the auto_ptr is instantiated, its destructor must call delete on a Y, and so Y cannot be an incomplete type. I suppose this has to be weighed against the value of RAII in the case that class X managed several resources (has several auto_ptr members). Any thoughts on this?

I think this technique is great overall, to eliminate a lot of tricky code for classes that may have to construct/destruct/copy potentially several resources (single responsibility principal). I'm just wondering if auto_ptr serves the purpose, or if it doesn't work here because of some subtle language/Standard Library requirement.

Please note that I'm aware of other types of smart pointers, but I'm implementing a library interface, and I don't want to impose any requirements for Boost or TR1 on my clients. Any lecture on the use of auto_ptr in general will be downvoted!

Answer 1

I think you've implemented a cloning smart pointer on top of std::auto_ptr. I'm not sure this is worth the hassle, as you're not very far from implementing one on top of a dumb pointer.

But as far as I see there's no error in this. (Of course, saying so will make someone point out an obvious flaw 5mins after I post this...)

Answer 2

Using auto_ptr when the destructor is out of scope does cause undefined behavior. MSVC warns about it. GCC doesn't.

You can make it work, but only if the destructor is not defined as an inline function.

The unique_ptr and scoped_ptr fix this problem. At least with those you are prevented from writing undefined code by accident.

Answer 3

What are you really trying to do? Seems like alot of unnecessary work if you just want to have a bunch of objects in a container. creating a temp object and having the object in the container take over ownership isn't a problem in my thinking

operator =() does not make sense to me, it is not = it is swap.

X a,b;
a=b
assert(a==b) // fail!

Your realisation is incorrect

Note that one thing I've realized is that with this approach, Y's class definition must be in scope for X's definition (as opposed to just forward-declared when a "raw" pointer to Y is used). This is because when the auto_ptr is instantiated, its destructor must call delete on a Y, and so Y cannot be an incomplete type.

if you define the destructor for X and implement in a CPP file you can quite legally do

header

class Y;
class X{
  virtual ~X();
  std::auto_ptr<Y> ap; 
};

body

class Y{};
~X(){}