[Interest] What's the recommended way to put QObjects in smart pointers?

[Nikos Chantziaras]

I've been removing every trance of 'new' and 'delete' from my code and
switching to something like:

auto obj = std::make_unique<Type>(ctor args);

(Or std::make_shared, depending on the situation.)

But for QObjects with a parent, that can go horribly wrong. So I have to
do annoying stuff like:

auto dialog = new QDialog(parent);
// work...
delete dialog;

Yes, 'parent' will eventually delete 'dialog', but that doesn't help
much when 'parent' is a long lived object (like the main application
window.)

Is there a Qt-specific idiom where I can manage QObjects that have a
parent with a smart pointer and not have to worry about double-deletion?

Or, to phrase it in another way, is there a way to eliminate the 'new'
keyword completely from my code-base when using QObjects with a parent?

[Sean Harmer]

Post by Nikos Chantziaras
I've been removing every trance of 'new' and 'delete' from my code and
auto obj = std::make_unique<Type>(ctor args);
(Or std::make_shared, depending on the situation.)
But for QObjects with a parent, that can go horribly wrong. So I have to
auto dialog = new QDialog(parent);
// work...
delete dialog;
Yes, 'parent' will eventually delete 'dialog', but that doesn't help
much when 'parent' is a long lived object (like the main application
window.)
Is there a Qt-specific idiom where I can manage QObjects that have a
parent with a smart pointer and not have to worry about double-deletion?
Or, to phrase it in another way, is there a way to eliminate the 'new'
keyword completely from my code-base when using QObjects with a parent?

Use smart pointers to only manage the root objects of your QObject hierarchies
and rely upon QObject to manage the child objects.

Cheers,

Sean

[Nikos Chantziaras]

Post by Sean Harmer

Post by Nikos Chantziaras
Is there a Qt-specific idiom where I can manage QObjects that have a
parent with a smart pointer and not have to worry about double-deletion?
Or, to phrase it in another way, is there a way to eliminate the 'new'
keyword completely from my code-base when using QObjects with a parent?

Use smart pointers to only manage the root objects of your QObject hierarchies
and rely upon QObject to manage the child objects.

That's not good enough. I need to delete objects way before their parent
is deleted. Otherwise, for all intents and purposes, I'm leaking memory.

[Nye]

Post by Nikos Chantziaras
I've been removing every trance of 'new' and 'delete' from my code and
auto obj = std::make_unique<Type>(ctor args);
(Or std::make_shared, depending on the situation.)
But for QObjects with a parent, that can go horribly wrong. So I have to
auto dialog = new QDialog(parent);
// work...
delete dialog;
Yes, 'parent' will eventually delete 'dialog', but that doesn't help
much when 'parent' is a long lived object (like the main application
window.)
Is there a Qt-specific idiom where I can manage QObjects that have a
parent with a smart pointer and not have to worry about double-deletion?

The parent will be notified of the child's deletion, so it's generally safe
to delete the child directly (with a few somewhat rare exceptions). So most
of the time QScopedPointer or it's STL equivalent work just fine. If you
need a guarded pointer for when Qt might delete the object while you still
hold a reference, you can use QPointer. I personally detest the unnecessary
new, and just create everything with auto storage where it's possible. So
that's another way you can eliminate new almost completely.

Kind regards.

[Nikos Chantziaras]

Post by Nye

Post by Nikos Chantziaras
[...]
Is there a Qt-specific idiom where I can manage QObjects that have a
parent with a smart pointer and not have to worry about double-deletion?

The parent will be notified of the child's deletion, so it's generally
safe to delete the child directly (with a few somewhat rare exceptions).
So most of the time QScopedPointer or it's STL equivalent work just
fine. If you need a guarded pointer for when Qt might delete the object
while you still hold a reference, you can use QPointer. I personally
detest the unnecessary new, and just create everything with auto storage
where it's possible. So that's another way you can eliminate new almost
completely.

From what I can tell, QPointer does not delete the object, so that's a
no go.

Putting the objects on the stack doesn't work either, because the Qt
object model does not support that (it will call 'delete' on the object
even if it's on the stack.)

[Thiago Macieira]

Post by Nikos Chantziaras
From what I can tell, QPointer does not delete the object, so that's a
no go.
Putting the objects on the stack doesn't work either, because the Qt
object model does not support that (it will call 'delete' on the object
even if it's on the stack.)

So what are you requirements? Do you want to a container that does delete the
object it has, unless the parent has already deleted it?

Use a custom deleter for unique_ptr that carries a QPointer so you can tell
whether it's already been deleted.

Or if you want to risk private API, see
QtSharedPointer::ExternalRefCountData::getAndRef(). Since it's used by inline
code, it can't disappear before Qt 6.

[Nikos Chantziaras]

Post by Thiago Macieira

Post by Nikos Chantziaras
From what I can tell, QPointer does not delete the object, so that's a
no go.
Putting the objects on the stack doesn't work either, because the Qt
object model does not support that (it will call 'delete' on the object
even if it's on the stack.)

So what are you requirements? Do you want to a container that does delete the
object it has, unless the parent has already deleted it?

Well, basically what I want is not having even a single 'new' or
'delete' anywhere in my code. I already achieved that with all my non-Qt
projects.

By now, I consider application-level code that uses 'new' or 'delete' as
bogus and in need of fixing.

[Jean-Michaël Celerier]

Post by Nikos Chantziaras
By now, I consider application-level code that uses 'new' or 'delete' as
bogus and in need of fixing.

This line of thought doesn't work at all with Qt. It's just how it is done
<http://doc.qt.io/qt-5/object.html>. If you are in such dire need of
guaranteeing the absence of memory problems :

template<typename T, typename... Args>
auto make_qobject(Args&&... args, QObject* parent) // Repeat for
QGraphicsItem / Object if necessary
{
assert(parent);
return new T{std::forward<Args>(args)..., parent};
}

QMainWindow win;
auto safe_obj_1 = make_qobject<QListWidget> (&win);
auto safe_obj_2 = make_qobject<QGridView>(&win);

[Konstantin Tokarev]

Post by Jean-Michaël Celerier

By now, I consider application-level code that uses 'new' or 'delete' as bogus and in need of fixing.

template<typename T, typename... Args>
auto make_qobject(Args&&... args, QObject* parent) // Repeat for QGraphicsItem / Object if necessary
{
    assert(parent);
    return new T{std::forward<Args>(args)..., parent};
}
QMainWindow win;
auto safe_obj_1 = make_qobject<QListWidget> (&win);
auto safe_obj_2 = make_qobject<QGridView>(&win);

Nevertheless that won't save from memory problems when parent needs to have longer lifetime then its children

[André Pönitz]

Post by Thiago Macieira

Post by Nikos Chantziaras
From what I can tell, QPointer does not delete the object, so that's a
no go.
Putting the objects on the stack doesn't work either, because the Qt
object model does not support that (it will call 'delete' on the object
even if it's on the stack.)

So what are you requirements? Do you want to a container that does delete the
object it has, unless the parent has already deleted it?

Well, basically what I want is not having even a single 'new' or 'delete'
anywhere in my code. I already achieved that with all my non-Qt projects.

remedyforirrationalfear.h:

namespace snakeoil {
template<typename T, typename... Args>
inline T* make_dynamic_object(Args&&... args)
{ return new T(std::forward<A>(args)...); }
}

myposhapp.cpp:

#include "remedyforirrationalfear.h"

// Oh Noes! Get out the garlic!
// auto x = new X(23);
auto x = snakeoil::make_dynamic_object<X>(23);

By now, I consider application-level code that uses 'new' or 'delete' as
bogus and in need of fixing.

What a happy world we live in.

Andre'

[Nikos Chantziaras]

Post by André Pönitz

Well, basically what I want is not having even a single 'new' or 'delete'
anywhere in my code. I already achieved that with all my non-Qt projects.

namespace snakeoil {
template<typename T, typename... Args>
inline T* make_dynamic_object(Args&&... args)
{ return new T(std::forward<A>(args)...); }
}
#include "remedyforirrationalfear.h"
// Oh Noes! Get out the garlic!
// auto x = new X(23);
auto x = snakeoil::make_dynamic_object<X>(23);

By now, I consider application-level code that uses 'new' or 'delete' as
bogus and in need of fixing.

What a happy world we live in.

That doesn't do anything useful. I think you're missing the point here.

[Giuseppe D'Angelo]

Post by Nikos Chantziaras
That doesn't do anything useful. I think you're missing the point here.

Did you try the earlier suggestion of using a QScopedPointer with a
custom deleter that holds a QPointer to the instance and deletes it
only if it has not been deleted already?

Cheers,

[Nikos Chantziaras]

Post by Giuseppe D'Angelo

Post by Nikos Chantziaras
That doesn't do anything useful. I think you're missing the point here.

Did you try the earlier suggestion of using a QScopedPointer with a
custom deleter that holds a QPointer to the instance and deletes it
only if it has not been deleted already?

Yes. But I now did the same with unique_ptr, so all is fine now :-)

[Thiago Macieira]

Post by Nikos Chantziaras

Post by Thiago Macieira
So what are you requirements? Do you want to a container that does delete
the object it has, unless the parent has already deleted it?

Well, basically what I want is not having even a single 'new' or
'delete' anywhere in my code. I already achieved that with all my non-Qt
projects.

If you don't want to use new or delete, use stack-based objects and only
create children objects as members. That is, what Nye suggested.

[Nikos Chantziaras]

Post by Thiago Macieira

Post by Nikos Chantziaras

Post by Thiago Macieira
So what are you requirements? Do you want to a container that does delete
the object it has, unless the parent has already deleted it?

Well, basically what I want is not having even a single 'new' or
'delete' anywhere in my code. I already achieved that with all my non-Qt
projects.

If you don't want to use new or delete, use stack-based objects and only
create children objects as members. That is, what Nye suggested.

The parents may delete them, and the children may have other children
they might delete.

[Alejandro Exojo]

Post by Nikos Chantziaras
Well, basically what I want is not having even a single 'new' or
'delete' anywhere in my code. I already achieved that with all my non-Qt
projects.
By now, I consider application-level code that uses 'new' or 'delete' as
bogus and in need of fixing.

I think you are confusing the path to arrive to some place, with the place
itself. make_shared and make_unique hide the "new", but their main reason to
exist is that some classes willingly throw on construction. That's not the
case of Qt classes, so using new on them is fine.

Now, what we all want is code that is easty to write, read and maintain, so we
want to make code that is as automatic as possible. That's why tools such as
smart pointers help, but only on part of the cases. There are things like
ScopeGuard, or other "RAII" classes which are not a smart pointer, so they
might not hide the "new", but are exactly as correct as your code without it.

Note that classes like QRunnable or QAbstractAnimation can delete themselves
when done. Nothing stops you to create a QDialog subclass that deletes itself
after you've got the result of exec() if you prefer that. It would be even
easier to spot problems, and with less "extreme" requirements on the C++
support of the compiler, IMHO.

[Nikos Chantziaras]

Post by Alejandro Exojo

Post by Nikos Chantziaras
Well, basically what I want is not having even a single 'new' or
'delete' anywhere in my code. I already achieved that with all my non-Qt
projects.
By now, I consider application-level code that uses 'new' or 'delete' as
bogus and in need of fixing.

I think you are confusing the path to arrive to some place, with the place
itself. make_shared and make_unique hide the "new", but their main reason to
exist is that some classes willingly throw on construction. That's not the
case of Qt classes, so using new on them is fine.

My own classes do, so using new is not fine.

Post by Alejandro Exojo
There are things like
ScopeGuard, or other "RAII" classes which are not a smart pointer, so they
might not hide the "new", but are exactly as correct as your code without it.

This results in code duplication, since you write the same cleanup code
everywhere.

Post by Alejandro Exojo
Note that classes like QRunnable or QAbstractAnimation can delete themselves
when done. Nothing stops you to create a QDialog subclass that deletes itself
after you've got the result of exec() if you prefer that. It would be even
easier to spot problems, and with less "extreme" requirements on the C++
support of the compiler, IMHO.

The easiest solution is to just have a smart pointer auto-delete it.
Cannot get any simpler than that.

[Jean-Michaël Celerier]

Post by Nikos Chantziaras
This results in code duplication, since you write the same cleanup code
everywhere.

I'd argue that if you use RAII and have cleanup code in more than one
place, you are not using RAII.
But could you give an example of this ?

[Nikos Chantziaras]

Post by Nikos Chantziaras
This results in code duplication, since you write the same cleanup
code everywhere.
I'd argue that if you use RAII and have cleanup code in more than one
place, you are not using RAII.
But could you give an example of this ?

SCOPE_GUARD { foo; bar; baz }

...

SCOPE_GUARD { foo; bar; baz }

...

SCOPE_GUARD { foo; bar; baz }

etc :-)

Scope guards are useful for one-off things, not for cleanup code you
would need to write over and over again.

[Nye]

This post might be inappropriate. Click to display it.

[Nikos Chantziaras]

Post by Nikos Chantziaras
From what I can tell, QPointer does not delete the object, so that's

a no go.
No one said it does. Perhaps I'm misunderstanding, are you searching for
a silver bullet for memory management?

I already found it. Question is how to make Qt work with it :-)

Post by Nikos Chantziaras
That's not good enough. I need to delete objects way before their

parent is deleted.
So delete them. As I said, parent is notified of the child's deletion.

If I delete them, the parent may already have deleted them. So I'd have
a double-delete.

Post by Nikos Chantziaras
When 'parent' gets deleted before 'dialog', then 'dialog' gets

deleted even though it's on the stack.
And how does the parent gets deleted before the 'dialog' if I may ask,
unless you're doing deletions from different threads ...?

I don't know. There's long call chains. Someone may delete the parent.
There's no way to know.

Anyway, the question was whether Qt had something already in place. I
now see I have to roll my own solution.

[Nye]

Post by Nikos Chantziaras

Post by Nye
No one said it does. Perhaps I'm misunderstanding, are you searching for
a silver bullet for memory management?

I already found it. Question is how to make Qt work with it :-)

You most certainly have not. I'm tired of repeating this, but here I go
again: C++ IS NOT JAVA!

Post by Nikos Chantziaras

Post by Nye
That's not good enough. I need to delete objects way before their
parent is deleted.
So delete them. As I said, parent is notified of the child's deletion.

If I delete them, the parent may already have deleted them. So I'd have a
double-delete.

We are running in a circle here. If the parent may delete the children
before you've lost all the references to them, then use QPointer.

Post by Nikos Chantziaras

Post by Nye
When 'parent' gets deleted before 'dialog', then 'dialog' gets
deleted even though it's on the stack.
And how does the parent gets deleted before the 'dialog' if I may ask,
unless you're doing deletions from different threads ...?

I don't know. There's long call chains. Someone may delete the parent.
There's no way to know.

The whole premise behind Mr. Sommers' code was that this is in one function
(one stack frame), so unless you do `delete this` in the middle of the
function (directly or indirectly) there doesn't seem be a way the parent
gets deleted.

Post by Nikos Chantziaras
Anyway, the question was whether Qt had something already in place. I now
see I have to roll my own solution.

It has, it provides a battery of wrapper classes for pointers usable for
different cases. However I get the feeling this won't be just enough for
you.

Well, basically what I want is not having even a single 'new' or 'delete'

Post by Nikos Chantziaras
anywhere in my code. I already achieved that with all my non-Qt projects.

By now, I consider application-level code that uses 'new' or 'delete' as

Post by Nikos Chantziaras
bogus and in need of fixing.

Refer to my first comment at the very top of this mail, I know it's getting
repetitive, but ... C++ is not Java!

I completely disagree with you :-)

Post by Nikos Chantziaras
[snippet]
This is a nightmare, and this is C++ code that's indeed probably from 20
years ago. If somebody writes code that like that today, I would want to
hire them.

I would refer you the Qt's source, I'm sure you'll be terrified of the
amount of raw pointers used. Forgive my bluntness, but what would your
suggestion be then? Scoped pointers can be used, but that's not much
different that manually deleting the objects. And shared pointers are for
things you share, not for things you're too lazy to delete ...

[André Somers]

Post by Nye
No one said it does. Perhaps I'm misunderstanding, are you searching for
a silver bullet for memory management?
I already found it. Question is how to make Qt work with it :-)
You most certainly have not. I'm tired of repeating this, but here I
go again: C++ IS NOT JAVA!

Post by Nikos Chantziaras
That's not good enough. I need to delete objects way before

their
parent is deleted.
So delete them. As I said, parent is notified of the child's deletion.
If I delete them, the parent may already have deleted them. So I'd
have a double-delete.
We are running in a circle here. If the parent may delete the children
before you've lost all the references to them, then use QPointer.

Post by Nikos Chantziaras
When 'parent' gets deleted before 'dialog', then 'dialog' gets

deleted even though it's on the stack.
And how does the parent gets deleted before the 'dialog' if I may ask,
unless you're doing deletions from different threads ...?
I don't know. There's long call chains. Someone may delete the
parent. There's no way to know.
The whole premise behind Mr. Sommers' code was that this is in one
function (one stack frame), so unless you do `delete this` in the
middle of the function (directly or indirectly) there doesn't seem be
a way the parent gets deleted.

Well, that's not quite true of course. If you want to show a dialog
created on the stack, you will have to use it's exec() method. That
spins an event loop, and that means that a lot can happen in the mean
time. It can vary from an incoming DBUS messages causing the application
to terminate to something in response to a signal from that dialog
itself, to some timer triggering a series of events that lead to the
destruction of the parent.

So, in that sense, Nikos is right.

I prefer just dealing with that using... new and ::open instead.

André

[Nye]

This post might be inappropriate. Click to display it.

[André Somers]

Post by Nikos Chantziaras
I've been removing every trance of 'new' and 'delete' from my code and
auto obj = std::make_unique<Type>(ctor args);
(Or std::make_shared, depending on the situation.)
But for QObjects with a parent, that can go horribly wrong. So I have
auto dialog = new QDialog(parent);
// work...
delete dialog;

Assuming the above is one function, you could just do:

auto dialog = QDialog(parent);
//work

//end of scope, dialog gets deleted

André

[Nikos Chantziaras]

Post by André Somers

Post by Nikos Chantziaras
I've been removing every trance of 'new' and 'delete' from my code and
auto obj = std::make_unique<Type>(ctor args);
(Or std::make_shared, depending on the situation.)
But for QObjects with a parent, that can go horribly wrong. So I have
auto dialog = new QDialog(parent);
// work...
delete dialog;

auto dialog = QDialog(parent);
//work
//end of scope, dialog gets deleted

When 'parent' gets deleted before 'dialog', then 'dialog' gets deleted
even though it's on the stack.

So that doesn't work.

[Bo Thorsen]

Post by Nikos Chantziaras
I've been removing every trance of 'new' and 'delete' from my code and
auto obj = std::make_unique<Type>(ctor args);
(Or std::make_shared, depending on the situation.)
But for QObjects with a parent, that can go horribly wrong. So I have to
auto dialog = new QDialog(parent);
// work...
delete dialog;
Yes, 'parent' will eventually delete 'dialog', but that doesn't help
much when 'parent' is a long lived object (like the main application
window.)
Is there a Qt-specific idiom where I can manage QObjects that have a
parent with a smart pointer and not have to worry about double-deletion?
Or, to phrase it in another way, is there a way to eliminate the 'new'
keyword completely from my code-base when using QObjects with a parent?

I'm still not sure if I should respond to this or not. You're clearly
trying to solve an unsolvable problem.

This mail is going to sound like a rant, because I've heard those types
of arguments from Qt newbies again and again. But it's actually not.
There's information here that will lead you to a code style that fits
the Qt memory model. Not your memory model.

First, you need to listen to this and fully understand it: There is *no*
way you can code C++ so memory errors are impossible.

Using smart pointers is just a silly pipe dream of the nineties that
unfortunately hasn't gone away. As Herb Sutter (I think, not completely
sure) once said: "Smart pointers is a solution in search of a problem".

Smart pointers is good for one thing only: To delete objects that
control their own life span.

For all other - and that's 99.9% - objects, it's cleaner simpler and
more efficient if you get rid of the notion that a system can do it for
you, and start giving clear lifetime ownership of your objects.

Yes, you can get crashes or memory loss. That's why you don't hire crap
coders to do C++, and check your code with tools like valgrind.

I'm sure you will have a long list of objections as to why I'm
completely wrong. Shared pointer lovers usually do. I don't care. I've
been doing Qt coding for 20 years and have tried every trick available
and come to the conclusion that it's actually only up to me writing good
code.

I hope this helps. I give the same type of arguments to my customers.
The smart ones get it, the others pay with increased maintenance cost
over the years.

Bo Thorsen,
Director, Viking Software.

[Nikos Chantziaras]

I completely disagree with you :-)

I hope you're not the type of guy who writes code like this:

something;
if (!something) {
delete;
return;
}
something else;
if (!something else) {
delete;
return;
}
try {
do stuff;
}
catch (...) {
delete;
throw;
}

This is a nightmare, and this is C++ code that's indeed probably from 20
years ago. If somebody writes code that like that today, I would want to
hire them.

Post by Bo Thorsen
I'm still not sure if I should respond to this or not. You're clearly
trying to solve an unsolvable problem.
This mail is going to sound like a rant, because I've heard those types
of arguments from Qt newbies again and again. But it's actually not.
There's information here that will lead you to a code style that fits
the Qt memory model. Not your memory model.
First, you need to listen to this and fully understand it: There is *no*
way you can code C++ so memory errors are impossible.
Using smart pointers is just a silly pipe dream of the nineties that
unfortunately hasn't gone away. As Herb Sutter (I think, not completely
sure) once said: "Smart pointers is a solution in search of a problem".
Smart pointers is good for one thing only: To delete objects that
control their own life span.
For all other - and that's 99.9% - objects, it's cleaner simpler and
more efficient if you get rid of the notion that a system can do it for
you, and start giving clear lifetime ownership of your objects.
Yes, you can get crashes or memory loss. That's why you don't hire crap
coders to do C++, and check your code with tools like valgrind.
I'm sure you will have a long list of objections as to why I'm
completely wrong. Shared pointer lovers usually do. I don't care. I've
been doing Qt coding for 20 years and have tried every trick available
and come to the conclusion that it's actually only up to me writing good
code.
I hope this helps. I give the same type of arguments to my customers.
The smart ones get it, the others pay with increased maintenance cost
over the years.
Bo Thorsen,
Director, Viking Software.

[Nikos Chantziaras]

Well, lots of strong opinions on this one :-)

Anyway, to each his own. Of course I know that C++ isn't Java, nor would
I want it to be. But just because C++ isn't Java doesn't mean I am in
favor of poor exception safety or lots of manual checking and cleaning
up. Yes, we still have to do these things, but the less of them we have
to do, the better.

So for my own needs, I just came up with this:

template <class T, typename... Args>
auto q_make_unique(Args&&... args)
{
auto tmp = std::make_unique<T>(std::forward<Args>(args)...);
auto deleter = [q_ptr = QPointer<T>(tmp.get())](const T* obj)
{
if (q_ptr) {
delete obj;
}
};
std::unique_ptr<T, decltype(deleter)>
uptr(tmp.release(), std::move(deleter));
return uptr;
}

Which probably can be done better with less overhead, but I'll settle
for "good enough" for now.

Post by Nikos Chantziaras
I've been removing every trance of 'new' and 'delete' from my code and
auto obj = std::make_unique<Type>(ctor args);
(Or std::make_shared, depending on the situation.)
But for QObjects with a parent, that can go horribly wrong. So I have to
auto dialog = new QDialog(parent);
// work...
delete dialog;
Yes, 'parent' will eventually delete 'dialog', but that doesn't help
much when 'parent' is a long lived object (like the main application
window.)
Is there a Qt-specific idiom where I can manage QObjects that have a
parent with a smart pointer and not have to worry about double-deletion?
Or, to phrase it in another way, is there a way to eliminate the 'new'
keyword completely from my code-base when using QObjects with a parent?