Question

I'm using std::unique_ptr to create some public members on a class and those members must be non copyable or movable. But std::unique_ptr is movable and I was wondering what would happen if someone would move the pointer contained by an std::unique_ptr and then I try to access the std::unique_ptr member of that class that was moved.

Example code:

#include <cstdlib>
#include <memory>
#include <string>
#include <iostream>

struct obj
{
    obj(std::string id)
        : id(id)
    {

    }

    void identif()
    {
        std::cout << "i am " << id << std::endl;
    }

    std::string id;
};

struct objs
{
    // They must stay here no mater what.
    std::unique_ptr< obj > obj_a;
    std::unique_ptr< obj > obj_b;
    std::unique_ptr< obj > obj_c;

    objs()
        : obj_a(std::unique_ptr< obj >( new obj("object a") ))
            ,obj_b(std::unique_ptr< obj >( new obj("object b") ))
            ,obj_c(std::unique_ptr< obj >( new obj("object c") ))
    {

    }

    void do_a()
    {
        std::cout << " member: " << obj_a->id << std::endl;
        std::cout << " method: ";
        obj_a->identif();
    }

    void do_b()
    {
        std::cout << " member: " << obj_b->id << std::endl;
        std::cout << " method: ";
        obj_b->identif();
    }

    void do_c()
    {
        std::cout << " member: " << obj_c->id << std::endl;
        std::cout << " method: ";
        obj_c->identif();
    }
};

int main(int argc, char** argv)
{

    objs obx;

    std::cout << " before move: " << std::endl;

    obx.do_a();
    obx.do_b();
    obx.do_c();

    std::cout << " after move: " << std::endl;

    std::unique_ptr< obj > newb(std::move(obx.obj_b));

    obx.do_a();
    obx.do_b();
    obx.do_c();

    return EXIT_SUCCESS;
}

The application crashes because of course which leads me to my question. How can I protect against that?

The objects must be public, not constant and unable to move or copy. The actual object in question uses variadic templates and I would prefer to not use the old way with a static member function create() in obj and then protect the constructors. Also I need to avoid geters and setters since the class uses variadic templates and makes the code ugly as hell.

All I need is to make std::unique_ptr non movable and to simply hold a unique pointer that can't be moved or copied ever from the class that owns it.

The actual code in question is the Nano::signal class from the Nano-signal-slot library. (in case someone needs the actual code)

EDIT: Adapted to use Nevin's method and made it to work. I've posted the code because Nevin implements it on the container and not the objects.

#include <cstdlib>
#include <memory>
#include <string>
#include <iostream>

struct obj
{
    obj(std::string id)
        : id(id)
    {

    }

    obj(obj const&) = delete;
    obj& operator=(obj const&) = delete;
    obj& operator=(obj&&) = delete;
    obj(obj&&) = delete;


    void identif()
    {
        std::cout << "i am " << id << std::endl;
    }

    std::string id;
};

struct objs
{
    const std::unique_ptr< obj > obj_a;
    const std::unique_ptr< obj > obj_b;
    const std::unique_ptr< obj > obj_c;

    objs()
        : obj_a(std::unique_ptr< obj >( new obj("object a") ))
            ,obj_b(std::unique_ptr< obj >( new obj("object b") ))
            ,obj_c(std::unique_ptr< obj >( new obj("object c") ))
    {

    }

    void do_a()
    {
        std::cout << " member: " << obj_a->id << std::endl;
        std::cout << " method: ";
        obj_a->identif();
    }

    void do_b()
    {
        std::cout << " member: " << obj_b->id << std::endl;
        std::cout << " method: ";
        obj_b->identif();
    }

    void do_c()
    {
        std::cout << " member: " << obj_c->id << std::endl;
        std::cout << " method: ";
        obj_c->identif();
    }
};

int main(int argc, char** argv)
{

    objs obx;

    std::cout << " before move: " << std::endl;

    obx.do_a();
    obx.do_b();
    obx.do_c();

    std::cout << " after move: " << std::endl;

    std::unique_ptr< obj > newb(std::move(obx.obj_b));

    obx.do_a();
    obx.do_b();
    obx.do_c();

    return EXIT_SUCCESS;
}

This should now generate:

main.cpp: In function 'int main(int, char**)':
main.cpp:77:53: error: use of deleted function 'std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp, _Dp>&) [with _Tp = obj; _Dp = std::default_delete<obj>]'
     std::unique_ptr< obj > newb(std::move(obx.obj_b));
                                                     ^
In file included from /usr/include/c++/4.8/memory:81:0,
                 from main.cpp:2:
/usr/include/c++/4.8/bits/unique_ptr.h:273:7: error: declared here
       unique_ptr(const unique_ptr&) = delete;
       ^
Was it helpful?

Solution

Just make objs non-copyable and non-moveable, as in:

struct objs
{
    objs(objs const&) = delete;
    objs& operator=(objs const&) = delete;
    objs& operator=(objs&&) = delete;
    objs(objs&&) = delete;

    std::unique_ptr< obj > obj_a;
    std::unique_ptr< obj > obj_b;
    std::unique_ptr< obj > obj_c;

    objs()
        : obj_a(std::unique_ptr< obj >( new obj("object a") ))
            ,obj_b(std::unique_ptr< obj >( new obj("object b") ))
            ,obj_c(std::unique_ptr< obj >( new obj("object c") ))
        {}

};

Plus, once you've done that, there is no need for pointers internally either. You can simplify the code as:

struct objs
{
    objs(objs const&) = delete;
    objs& operator=(objs const&) = delete;
    objs& operator=(objs&&) = delete;
    objs(objs&&) = delete;

    obj obj_a;
    obj obj_b;
    obj obj_c;

    objs()
        : obj_a("object a")
            ,obj_b("object b")
            ,obj_c("object c")
            {}

};

OTHER TIPS

newb(std::move(obx.obj_b))

This code is expected to leave obx in an unspecified state, so you should not try to access it again, rather try to access newb. If you don't want the contained unique_ptr's to be moved, then don't move their container obx. Or, if you want to define the semantics of (or disable) moving struct objs, then provide your own move constructor (assignment, etc.) or delete the default one(s), depending on what you want to do.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top