1

I'm looking to replace some old raw pointer handling for objects (unparented QObjects) that in practice are treated as optional objects. E.g:

if(m_file)
   delete m_file;
m_file = new QFile(...);`

Both std::unique_ptr and std::optional seem to be suitable for my needs.

I decided to do some testing first, and came across some strange behaviour with std::optional: The T::dtor appears to be called twice when exiting scope.

https://godbolt.org/z/Yeq887YPh.

struct Loud
{
    Loud(std::string name) : m_name{ name } { print("Creating"); }
    Loud(const Loud& other) : m_name{ other.m_name } { print("Copying"); }
    Loud(Loud&& other) : m_name{ other.m_name }{ print("Moving"); }
    ~Loud() { print("Destroying"); }
    Loud& operator=(const Loud& other){ this->m_name = other.m_name; print("Copy="); return *this;}
    Loud& operator=(Loud&& other){ this->m_name = other.m_name; print("Move=");; return *this;}

    std::string m_name;
    void print(std::string operation) { std::cout << operation << " " << m_name << "\n"; }
};

void optionalTest()
{
    std::optional<Loud> opt;
    opt = Loud("opt 1");
    opt = Loud("opt 2");
}

void uniqueTest()
{
    std::unique_ptr<Loud> unique;
    unique = std::make_unique<Loud>("unique 1");
    unique =  std::make_unique<Loud>("unique 2");
}

int main()
{
    optionalTest();
    std::cout << "\n";
    uniqueTest();
}

Which produces the output

Creating opt 1
Moving opt 1
Destroying opt 1
Creating opt 2
Move= opt 2
Destroying opt 2
Destroying opt 2    <-- Whyy??

Creating unique 1
Creating unique 2
Destroying unique 1
Destroying unique 2

I'm a bit worried that the double dtor might cause issues with deallocating resources, so I'm currently more inclined to opt for the unique_ptr.


Edit:

Only the last destruction is the actual std::optional<Loud> opt object being destroyed. The rest of the destructors are from temporaries.

Its illustrated better by changing the other.m_name in the move operators (https://godbolt.org/z/3vWfPPY7b).

Output:


Creating opt 1
Moving opt 1
Destroying opt 1 (mvd from)
Creating opt 2
Move= opt 2
Destroying opt 2 (mvd from)
Destroying opt 2

3 Answers3

3

Everything is fine. You have 3 objects being worked on:

a) in std::optional,
b) temporary opt 1 and
c) temporary opt2.

Now, referring those to your output:

Creating opt 1   // create b
Moving opt 1     // create a using b
Destroying opt 1 // destroy b
Creating opt 2   // create c
Move= opt 2      // update a using c
Destroying opt 2 // destroy c
Destroying opt 2 // destroy a

std::unique_ptr doesn't store its own object, only a pointer, so there's no 3rd object in this case.

Yksisarvinen
  • 18,008
  • 2
  • 24
  • 52
1
Creating opt 2
Move= opt 2
Destroying opt 2
Destroying opt 2    <-- Whyy??

Because you created one opt 2 object, and then move-assigned it to another one. It says so, right there.

Move assigning (or constructing) does not destroy the moved-from objects as part of the move. The moved-from object still exists, and your move operation still has both objects tagged as "opt 2". You now have two opt 2 objects in existence. Each one has to get destroyed, at some point.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • Yeah, it makes sense by its own, but when I extend the code with `opt = Loud("opt 3");`, I get the following output: `.... Creating opt 2, Move= opt 2, Destroying opt 2, Creating opt 3, Move= opt 3, Destroying opt 3, Destroying opt 3` I would in this case expect "opt 2" to still be destroyed twice. Once for the temporary, and once when the optional container releases "opt 2" to be assigned "opt 3". – Jens Joberg Mar 10 '23 at 12:16
  • "once when the optional container releases "opt 2" to be assigned "opt 3." The optional container does not release opt2. It overwrites it with opt3 via the Move= operator. Update your print() method to print the value of "this" to see better what's going on. – Raymond Chen Mar 10 '23 at 12:26
  • @JensJoberg `std::optional` has overloaded `operator=` to forward the call directly to the held object (avoiding destruction and creation of held object as a result). Basically, with `Loud a, b; std::optional opt {a};` ,`a = b;` and `opt = b;` do the same thing. – Yksisarvinen Mar 10 '23 at 12:29
  • I guess that is where my mental model of `optional` was wrong. I've thought of it as something like a `unique_ptr`, but on the stack. Obviously there is more to it than that. I guess its an ok assumption for POD's, but might get a bit more tricky with RAII types. I would then think its safer to replace the old `if(m_var) delete m_var; m_var = new ...` mess with `unique_ptr` as thats more in line with the old behaviour. – Jens Joberg Mar 10 '23 at 12:36
  • @JensJoberg Right. The equivalent to an updating `optional` assignment would be something like `*unique = Loud{"unique 2"};`. The `unique` holds the same object, but the object has been assigned from another object. – Raymond Chen Mar 10 '23 at 16:43
0

In the std::optional case there are three Loud objects in the std::unique_ptr<Loud> case there are only two Loud objects.

For std::optional:

void optionalTest()
{
    std::optional<Loud> opt;
    //A std::optional to hold a <Loud> object is created

    opt = Loud("opt 1");
    //An Object <Loud> with "opt 1" is created
    //The Object <Loud> with "opt 1" is move assigned to the <Loud> object in std::optional:
    // => m_name in <Loud> with "opt 1" is still "opt 1"
    // => m_name in <Loud> in the std::optional is now "opt 1"
    //The Object <Loud> with "opt 1" goes out of scope and is destroyed

    opt = Loud("opt 2");
    //An Object <Loud> with "opt 2" is created
    //The Object <Loud> with "opt 2" is move assigned to the <Loud> object in std::optional:
    // => m_name in the <Loud> with "opt 2" is still "opt 2"
    // => m_name in <Loud> in the std::optional is now "opt 2"
    //The Object <Loud> with "opt 2" goes out of scope and is destroyed

    //The std::optional goes out of scope, and gets destroyed alongside its own <Loud> Object, which has a m_name with "opt 2"
}

And for std::unique_ptr:

void uniqueTest()
{
    std::unique_ptr<Loud> unique;
    //A std::unique_ptr to hold a pointer to a <Loud> object is created

    unique = std::make_unique<Loud>("unique 1");
    //An Object <Loud> with "opt 1" is created on the Heap
    //The pointer to the <Loud> with "opt 1" is assigned to the std::unique_ptr
    
    unique = std::make_unique<Loud>("unique 2");
    //An Object <Loud> with "opt 2" is created on the Heap
    //The pointer to the <Loud> with "opt 2" is assigned to the std::unique_ptr
    //The std::unique_ptr automatically deletes the <Loud> with "opt 1"

    //The std::unique_ptr goes out of scope and is destryoed, automatically deleting the <Loud> with "opt 2"
}

I think the confusion might come from the fact that the move-assignment operator does not actually move the object itself, but rather the contents of the object (or at least it should); or, as the cppreferences puts it

Move assignment operators typically "steal" the resources held by the argument (e.g. pointers to dynamically-allocated objects, file descriptors, TCP sockets, I/O streams, running threads, etc.), rather than make copies of them, and leave the argument in some valid but otherwise indeterminate state.

CharonX
  • 2,130
  • 11
  • 33