std::unique_ptr issues

Started by
13 comments, last by mpratscher 9 years, 7 months ago

Hello,

I'm just in the process of replacing objects that owned and deleted a pure pointer with std::unique_ptr. However, I'm running into some issues:

- I've been heavily using forward declaration to speed up compile times, like here:


class Trigger;

class Instance
{
    typedef std::unordered_map<std::wstring, Trigger*> TriggerMap;
public:

    ~Instance(void)
    {
        for(auto& trigger : m_mTrigger)
        {
            delete trigger.second;
        }
    }
    
private:

    TriggerMap m_mTrigger;
}

However, I keep getting "warning C2027" in visual studio, in that an incomplete type can't be deleted when replacing the pure pointer with std::unique_ptr. Some of the times leaving the empty dtor fixes it, however in most cases, especially containers, I have to replace the forward-declaration with a direct include in the header. Is there some way around this? It really bugs me having to do this on so many occasions, could make recompile times significantly larger (already kind of a problem since I'm heavily using templates)...


class Trigger;

class Instance
{
	typedef std::unordered_map<std::wstring, std::unique_ptr<Trigger>> TriggerMap;
public:

	~Instance(void); // defined in cpp-file
	
private:

	TriggerMap m_mTrigger;
}

- I also keep getting compilation-errors with this kind of container-of-pointer setup when the class has a default copy ctor/operator. So the above example wouldn't even compile unless I put:


class Trigger;

class Instance
{
	typedef std::unordered_map<std::wstring, std::unique_ptr<Trigger>> TriggerMap;
public:

        Instance(void) = default;
        Instance(const Instance&) = delete;
	~Instance(void); // defined in cpp-file

        void operator=(const Instance&) = delete;
	
private:

	TriggerMap m_mTrigger;
}

Now I don't completely mind it, in fact I have been very sloppy about doing this before, since a class the won't compile that way shouldn't have been copyable before. Still, I wonder whether there is any shorthand to this? E.g. adding a std::unique_ptr member will automatically disable copy ctor/operator for the class, I wonder if thats possibly within a container class too?

Advertisement
I don't understand the problem you describe. Could you post a minimal complete program that shows the C2027 error?

I don't understand the problem you describe. Could you post a minimal complete program that shows the C2027 error?

Of course. Minimal program to show this would be:


#include <unordered_map>
#include <memory>

struct Bar;

struct Foo
{
	std::unordered_map<std::wstring, std::unique_ptr<Bar>> m_mPointer;
};

int main(){

	Foo foo;

	return -1;
}

Doesn't matter whether I put Foo.h in seperate file, and whether "Bar" exists in the translation unit and/or is included to Foo.cpp or not. The error is given by a static assert in the unique_ptr's default destructor:


	void operator()(_Ty *_Ptr) const _NOEXCEPT
		{	// delete a pointer
		static_assert(0 < sizeof (_Ty),
			"can't delete an incomplete type");
		delete _Ptr;
		}

Its basically just a safety-check for the warning that would be otherwise issues (C4150) and the resulting memory leak/undefined behaviour. I've actually written my own unique_ptr implementation, which doesn't do the static_assertion to confirm this.

Now I know that you technically CANT delete a class whose definition you don't have. I'm asking this since I've read that this can work if you make an empty dtor for the class and define it in the cpp-file that includes the forward declared class. This doesn't seem to work for me, is there still a way to use the unique_ptr without having to include the full class definiton to the header?

Is it understandable now?

You can't delete pointers to Bar if you don't have the definition of Bar. There is nothing special about that in std::unique_ptr. This compiles just fine:

#include <unordered_map>
#include <memory>
#include <string>

struct Bar;

struct Foo {
  std::unordered_map<std::wstring, std::unique_ptr<Bar>> m_mPointer;
};

struct Bar {
};

int main(){
  Foo foo;
}

You can't delete pointers to Bar if you don't have the definition of Bar. There is nothing special about that in std::unique_ptr. This compiles just fine:

Hm, I kind of was afraid that was the case. This is unfortunate, because otherwise it would have been like this:


// Foo.h

struct Bar;

struct Foo
{
    ~Foo(void);

    Bar* m_pBar;
}

// Foo.cpp
#include "Bar.h"

Foo::~Foo(void)
{
    delete m_bar;
}

While now I have to do


// Foo.h

#include "Bar.h"
#include <memory>

struct Foo
{
     std::unique_ptr<Bar> m_bar;
};

Which really makes me rethink the decision of applying those pointer everwhere. I liked the idea of automatic memory management without overhead, but not at the cost of losing forward declaration wherever I need it... :/


This is unfortunate, because otherwise it would have been like this:

// Foo.h

struct Bar;

struct Foo
{
~Foo(void);

Bar* m_pBar;
}

// Foo.cpp
#include "Bar.h"

Foo::~Foo(void)
{
delete m_bar;
}

Not sure why you are saying that. Which compiler are you using?

This works with VS2013 update 3


//////Test.h

#include <memory>

struct Bar;
struct Foo
{
	~Foo(void);
	std::unique_ptr<Bar> m_pBar;
};

/////Test.cpp
#include "test.h"

struct Bar
{

};

int main()
{
	Foo f;
}

I actually use unique_ptr with forward declarations a lot. Works great.

The C++ standard says that "the template parameter T of unique_ptr may be an incomplete type".
Your problem seems to be something causing std::unique_ptr to instantiate the destructor before Bar is defined. Pay close attention to the warning to see if it indicates another file in your compilation unit. This will usually happen if you have code in your header file that is trying to manipulate the unique_ptr, even if the manipulation is "invisible" (i.e. constructor, destructor, accessing, etc).

The examples provided above by Alvaro, Juliean and N.I.B. work because, by the time a Foo is instantiated (in main) the definition of Bar is available, whereas in your example Bar is never defined.

Also, unique_ptr is not copyable, so anything that stores them can not be copied. This includes containers, which is why compilation failed with the auto-generated copy constructor and copy assignment operator. Since unique_ptr indicates single-ownership, this is desired, and you should delete the copy constructor and assignment operator as you did above. If you want to allow someone to move ownership, then supply your own move constructor and operator which should work just fine.

I suspect all your problems would disappear if you used a pimpl. Hide your private parts.

Stephen M. Webb
Professional Free Software Developer

There's a proposed paper to add support for containers of smart pointers to incomplete types. No standard library implements it yet.

In other cases, remember that the compiler generates some implicit functions that invoke member destructors, like the destructor and copy operators. Explicitly declaring these functions in the declaration and then defining the functions as =default in the source tile avoids some problems, but had trade-offs.

edit: fixed some mangling my phone did when originally writing this up.

Sean Middleditch – Game Systems Engineer – Join my team!

This topic is closed to new replies.

Advertisement