views:

307

answers:

2

I have an auto pointer implementation:

template <typename T, bool Arr = false>
class GAutoPtr
{
    T *Ptr;

public:
    typedef GAutoPtr<T, Arr> &AutoPtrRef;

    GAutoPtr(T *ptr = 0)
    {
        Ptr = ptr;
    }

    GAutoPtr(AutoPtrRef p)
    {
        Ptr = p.Release();
    }

    ~GAutoPtr() { Empty(); }
    operator T*() { return Ptr; }
    T *Get() { return Ptr; }
    T *operator->() const { LgiAssert(Ptr); return Ptr; }

    inline void Empty()
    {
        if (Arr)
            delete [] Ptr;
        else
            delete Ptr;
        Ptr = 0;
    }

    AutoPtrRef operator =(GAutoPtr<T> p)
    {
        Empty();
        Ptr = p.Ptr;
        p.Ptr = 0;
        return *this;
    }

    void Reset(T *p)
    {
        if (p != Ptr)
        {
            Empty();
            Ptr = p;
        }
    }

    T *Release()
    {
        T *p = Ptr;
        Ptr = 0;
        return p;
    }
};

typedef GAutoPtr<char, true> GAutoString;
typedef GAutoPtr<char16, true> GAutoWString;

And that works fine in Visual C++ 6. However in Visual C++ 2005 or 2008 I can't return an auto pointer from a function without things going horribly wrong.

e.g.

GAutoString Func()
{
    char *s = new char[4];
    strcpy(s, "asd");
    return s;
}

int main()
{
    GAutoString a = Func();
    /// a.Ptr is now garbage
}

What happens is that the compiler creates a temporary GAutoString to hold the return value for the function, and then in passing that to the variable 'a' on the stack calles the operator T*() of the temp variable, and then the GAutoPtr(T *ptr = 0) constructor, instead of just using the copy constructor: GAutoPtr(AutoPtrRef p)

This results in the temp auto ptr deleting the memory and 'a' holding a pointer to the freed memory.

However in VC6 it DOES call the right constructor. Now in saying all this I also use gcc on Linux and Mac, so whatever code I write needs to work there too. VC2008 prevents you from using a non-const by value variable in the copy constructor. Also I don't want "const" anyway, because the copy constructor takes ownership of the memory block which removes ownership from the object being copied... thus modifying it.

How can I make this work in VC 2005/2008?

+1  A: 

You could mark the constructor that takes the T* argument with the "explicit" keyword. I've verified that this works and prevents the creation of the spurious temporary object too, a performance enhancement. Anyone using that constructor could no longer rely on compiler type conversion rules, though. For example, you function would change to this:

GAutoString Func()
{
    char *s = new char[4];
    strcpy(s, "asd");
    return GAutoString(s);
}

It's annoying, but I don't think that is necessarily a bad thing in this case as automatic type conversions can be confusing.

David Gladfelter
Yeah that works. I do have to clean up a new automatic convertsions like you said... but I'd rather that than apps the crash ;)
fret
+1  A: 

Did this even compile under a recent g++? I tried it on my MacBook Pro and:

xxx.cpp: In function ‘AutoString func()’:
xxx.cpp:52: error: no matching function for call to ‘AutoPtr<char, true>::AutoPtr(AutoString)’
xxx.cpp:12: note: candidates are: AutoPtr<T, isArray>::AutoPtr(AutoPtr<T, isArray>&) [with T = char, bool isArray = true]
xxx.cpp:9: note:                 AutoPtr<T, isArray>::AutoPtr(T*) [with T = char, bool isArray = true]
xxx.cpp:52: error:   initializing temporary from result of ‘AutoPtr<T, isArray>::AutoPtr(T*) [with T = char, bool isArray = true]’

The only way that I can get this to compile is by making the copy constructor take a const reference which is what I had suspected. It looks like Herb Sutter posted about something very similar to this in his most recent GotW. I wouldn't try to replicate the ownership transfer semantic of std::auto_ptr without a very good reason. You might want to look at the various goodies in Boost.SmartPtr. If possible, use them instead.

If you can't Boost for whatever reason, then read your favorite std::auto_ptr implementation and pay particular attention to the std::auto_ptr_ref class. Once you understand why it is there and exactly how it does what it does, then go back and write an auto-ptr class. This class exists to get around the problem that you are seeing. IIRC, this is discussed in some detail in Josuttis': The Standard C++ Library. I think that is where I really understood it for the first time.

D.Shawley
Ending up needing more than the 'explicit' keyword, so I've distilled a solution from STL using the auto_ptr_ref class. So far so good.
fret