std::unique_ptr deleted function, initializer_list - driven allocation

Question

All,

When I instantiate a widgets array using an initializer-list format, a naked pointer that points to a member-variable widget instance compiles but after change to std::unique_ptr<> gcc gives a compilation error regarding a deleted function.

$ uname -a

Linux .. 3.5.0-21-generic #32-Ubuntu SMP Tue Dec 11 18:51:59 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

$ g++ --version

g++ (Ubuntu/Linaro 4.7.2-5ubuntu1) 4.7.2

This code gives the following compiler error:

#include <stdlib.h>
#include <memory>

class Widget
{
public:
    Widget() {}
};

class W1 : public Widget
{
public:
    W1() {}
};

class W2 : public Widget
{
public:
    W2() {}
};

class WFactory
{
public:
    WFactory(const int i)   : _w(new W1()) {}
    WFactory(const char* s) : _w(new W2()) {}

    ~WFactory() { _w.reset(nullptr); }
    // ~WFactory() { delete _w; }  <--- for naked ptr

private:
    // NOTE: does not compile
    std::unique_ptr<Widget>  _w; 
    // NOTE: does compile
    // Widget* _w;
};

int main()
{
    std::unique_ptr<Widget> a(new W1()); // <--- compiles fine

    WFactory wf[] { 4, "msg" };          // <--- compiler error using unique_ptr<>
}

error:

$ g++ -o unique_ptr  -std=c++11 -Wall  unique_ptr.cpp 
unique_ptr.cpp: In function ‘int main()’:
unique_ptr.cpp:36:30: error: use of deleted function ‘WFactory::WFactory(const WFactory&)’
unique_ptr.cpp:22:7: note: ‘WFactory::WFactory(const WFactory&)’ is implicitly deleted because the default definition would be ill-formed:
unique_ptr.cpp:22:7: error: use of deleted function ‘std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp, _Dp>&) [with _Tp = Widget; _Dp = std::default_delete<Widget>; std::unique_ptr<_Tp, _Dp> = std::unique_ptr<Widget>]’
In file included from /usr/include/c++/4.7/memory:86:0,
             from unique_ptr.cpp:2:
/usr/include/c++/4.7/bits/unique_ptr.h:262:7: error: declared here
unique_ptr.cpp:36:30: error: use of deleted function ‘WFactory::WFactory(const WFactory&)’
unique_ptr.cpp:36:14: warning: unused variable ‘wf’ [-Wunused-variable]

I'm at a loss as to either: the mechanics behind the scenes that yields a deleted fcxn; or more simply, why the expressiveness of std::unique_ptr<> appears to be restricted compared w/ a naked ptr.

My question is:

• pilot error?
• compiler error?
• can I get my intended code to work w/ some change?

Thank you.

Edit 1

Based on your answers, which I appreciate, I can make the following change to WFactory:

(flagged as immoral code)

class WFactory
{
public:
    WFactory(const WFactory& wf)
    {
        (const_cast<WFactory&>(wf)).moveto(_w);
    }

    WFactory(const int i)   : _w(new W1()) {}
    WFactory(const char* s) : _w(new W2()) {}

    ~WFactory() { _w.reset(nullptr); }

    void moveto(std::unique_ptr<Widget>& w)
    {
        w = std::move(_w);
    }
private:
    std::unique_ptr<Widget>  _w; 
};

and now the program compiles and runs. I appreciate that the standards folks wrote the specification for a reason, so I post my result as a good-faith specialization for my case at hand, where I really would like to emphasize the uniqueness of the ptr.

Edit 2

Based on Jonathan's replies, the following code does not suppress the implicit move ctor:

class WFactory
{
public:
    WFactory(const int i)   : _w(new W1()) {}
    WFactory(const char* s) : _w(new W2()) {}

private:
    std::unique_ptr<Widget>  _w; 
};

Note that there is no ~WFactory() {..} at all.

Perhaps there's ya-ans, but I have found that using a c++11-style iteration over wf[] in Main() brings back the no-copy-ctor-for-WFactory error. That is:

int Main()
..
    WFactory wf[] { 4, "msg" };

    for ( WFactory iwf : wf )    <---- compiler error again
        // ..

    for (unsigned i = 0; i < 2; ++i)  <--- gcc happy
        wf[i] //  ..
}

I guess it's self-evident that the new c++11-style iteration is doing an object copy.

Solution 1

According to Paragraph 8.5.1/2 of the C++11 Standard:

When an aggregate is initialized by an initializer list, as specified in 8.5.4, the elements of the initializer listare taken as initializers for the members of the aggregate, in increasing subscript or member order. Each member is copy-initialized from the corresponding initializer-clause. [...]

For each element, then, copy-initialization involves the creation of a temporary of the destination type, which is then use to copy-construct the element of the array.

However, your class contains a member whose type is an instance of unique_ptr, which is non-copyable. That makes your class non-copyable as well.

Moreover, although unique_ptr is moveable, your class is not, because the implicit generation of a move constructor by the compiler is suppressed by the presence of an explicitly defined destructor. If that were not the case (i.e., if you explicitly defined a move constructor for your class), copy-initialization would work (see 8.5/15).

Try changing the definition of WFactory as follows to see that:

class WFactory
{
public:
    WFactory(const int i)   : _w(new W1()) {}
    WFactory(const char* s) : _w(new W2()) {}
    WFactory(WFactory&& f) : _w(std::move(f._w)) {}
    ~WFactory() { _w.reset(nullptr); }
private:
    std::unique_ptr<Widget> _w;
};

int main()
{
    std::unique_ptr<Widget> a(new W1());
    WFactory wf[] { 4, "msg" };          // OK
}

Solution 2

the mechanics behind the scenes that yields a deleted fcxn;

Arrays can only be initialized like that if the type is copyable or movable, and unique_ptr is not copyable, so a class with a unique_ptr member is not copyable by default, and your type has a user-defined destructor, which inhibits the implicit move constructor, so your type is not movable either.

or more simply, why the expressiveness of std::unique_ptr<> appears to be restricted compared w/ a naked ptr.

unique_ptr is saving you from a serious bug. With the naked pointer your type is massively unsafe and will result in undefined behaviour, because you don't have a copy constructor so the pointer gets copied and then deleted twice by two different objects. Boom, your program has undefined behaviour. unique_ptr fixes your class by preventing it from being copied, which is safe and correct.

You can make it work in several ways, the easiest is to remove the user-defined destructor, which makes your class movable, and the array initialization will compile.

Or if you need a user-defined destructor for some other reason, you can still make it work by writing a user-defined move constructor to explicitly move the _w member.

If for some reason that isn't possible, you can make it work by adding a default constructor, so the array elements can be default constructed, and then move-assigning to them:

class WFactory
{
public:
    WFactory() = default;
    WFactory(const int i)   : _w(new W1()) {}
    WFactory(const char* s) : _w(new W2()) {}

private:
    std::unique_ptr<Widget>  _w; 
};

int main()
{  
    WFactory wf[2];
    wf[0] = WFactory(4);
    wf[1] = WFactory("msg");
}

Your edited version is immoral and highly dubious, you should not cast away const like that and you should not move from an lvalue, especially not a const lvalue. Do not go there. Instead change how you use the class to avoid needing to copy it, or write a valid copy constructor that does a deep copy of the owned object:

class Widget
{
public:
    Widget() {}
    virtual std::unique_ptr<Widget> clone() const = 0;
};

class W1 : public Widget
{
public:
    W1() {}
    virtual std::unique_ptr<Widget> clone() const
    { return std::unique_ptr<Widget>(new W1(*this)); }
};

class W2 : public Widget
{
public:
    W2() {}
    virtual std::unique_ptr<Widget> clone() const
    { return std::unique_ptr<Widget>(new W2(*this)); }
};

class WFactory
{
public:
    WFactory(const int i)   : _w(new W1()) {}
    WFactory(const char* s) : _w(new W2()) {}
    WFactory(const WFactory& w) : _w(w._w->clone()) {}
    // ...

The best approach is to make the class movable, and a good way to do that is to follow the rule of zero

Comments

[JayInNyc]

I like that my edit is immoral ;-) That put a smile on my face (really).

[JayInNyc]

"With the naked pointer your type is massively unsafe and will result in undefined behaviour, because you don't have a copy constructor so the pointer gets copied and then deleted twice by two different objects. Boom, your program has undefined behaviour. unique_ptr fixes your class by preventing it from being copied, which is safe and correct." -- so we agree. This is the inspiration for my original question.

[JayInNyc]

FYI my application cannot serially call wf[0] = ..., wf[1] = ... wf[] {..} is the mechanism I (currently) require.

[JayInNyc]

The clone() approach for the deep copy requires that each widget is allocated twice. Just off the top of my head that seems a steep price. As an alternative (which isn't available), why doesn't the standard read: Each member is copy-initialized or move-initialized from the corresponding initializer-clause? The (initial) code I've written has clear intent but it's the internal mechanism wf[] {} that prevents me from either being pointer-safe or managing a copy ctor w/out a deep copy. What would the drawbacks of a move-ctor be?

[Jonathan Wakely]

The term copy-initialization pre-dates move semantics, and a copy-initialization is actually allowed to move instead of copying. The reason your code didn't allow moving is because the user-defined destructor suppresses the implicit move constructor. Get rid of the pointless destructor that redundantly calls reset and it should work. I've updated my answer to correct my previous incorrect statements

[Jonathan Wakely]

[dcl.init]/15 "[ Note: Copy-initialization may invoke a move (12.8). — end note ]"

[JayInNyc]

@AndyProwl Andy, my edits are asynch to yours -- just saw your change. Interesting... works like a charm. To me this version is the most expressive -- I have a uptr, I have my wf[] {..} style initialization, and I clearly std::move when I'm asked to (even though hidden away). Thank you.

[Andy Prowl]

@JayInNyc: Glad it helped :-)

[JayInNyc]

Andy, Jonathan, thank you both very much. This has been a very enlightening discussion.