[Development] The future of smart pointers in Qt API

[Daniel Teske]

> Parent-child relationship is not going to go away, it has purpose beyond object lifetime: the visual hierarchy is encoded in the parent/child relationship of widgets, for example.
>
> Making ownership of objects, and assumptions regarding their life-cycle, explicit in code is a good thing though. Using smart pointers is the idiomatic C++ way to do so. But it doens’t have to be the only way; a call to QObject::setParent is IMHO just as explicit, and parent/child in Qt is a concept you have to learn either way.
>
> But sometimes our APIs are messy. QTabWidget::addTab transfers ownership of the QWidget*; QTabWidget::addAction does not transfer ownership of the QAction*. Unless you understand fairly well how QAction works, or unless you read/remember the documentation, it’s impossible to see from the code what happens wrt to object ownership.
>
> API naming is not a scalable way to make the distinction, so finding how we can use smart pointers to make it clear in the code when ownership is transferred is a good idea.
>
> Perhaps the pragmatic approach is to address this in those APIs that take a QObject* but don’t transfer ownership. I believe those are much fewer. Otherwise, the convention can be that any API that takes a raw pointer takes ownership. How this is implemented internally doesn’t matter.
>

I have already invested quite a bit of time and thinking into how to add 
unique_ptr apis to Qt. And I encourage everyone to go back and read my 
previous mails, and/or the actual patch I produced.

Because I have already presented a design that is:

* Backwards compatible, existing code continues to work.
* And adds a lot of memory safety to Qt. Though not without a loop-hole.
* And actually exists as a experimental patch, not just some abstract idea.

To explain it once more, the key insight is the following invariant:

A QObject[1] is either owned by its parent, or by a unique_ptr[2].

[1]: Some classes use the same parent-child concept as QObject.
[2]: Stack allocated objects are essentially equivalent to unique_ptr 
held ones.

That implies that a unique_ptr owned QObject should have no parent.

That invariant ensures that there's exactly one owner for each object, 
and thus ensures that every object is deleted.

But it doesn't guarantee that the owner is what the user expected.

So to take your examples:

QTabWidget::addAction: This never transfers ownership, thus the 
invariant holds.

parent->addTab(tab): This has three cases:

* tab is owned already by parent: Invariant holds
* tab is owned by a different parent: Invariant holds

And the last case, and that's the small loop hole in the design:
* tab has no parent

That means, the child is owned by a unique_ptr, and the user must have 
called .get() on it.

Thus the small case where the user has to be cautious is when they have 
a raw pointer for a object owned by a unique_ptr. Luckily with the 
design I have this is should be rare.

In addition to the existing QTabWidget::addTab, the patch then adds:

QTabWidget::addTab(unique_ptr<>) for which the invariant obviously holds.

Now to take your next example: setParent is not fine. setParent can be 
used in 4 different ways:

child->setParent(child->parent()): Invariant holds
child->setParent(newParent): Invariant holds
child->setParent(nullptr): Nope
orphan->setParent(parent): Nope

The solution is I have is to replace setParent with two functions: 
adoptChild + releaseChild, which both ensure that the invariant always 
holds.

And to deprecate setParent. I actually made that deprecation opt-in, 
because of the next part:

Construction of QObjects:

The goal is to enable modern C++, meaning that users can choose to never 
use "new".

The first problem is then that a ctor such as this:

QTabWidget(QWidget *parent = nullptr)

can be used via make_unique:

auto child = make_unique<QTabWidget>(someParent);

That call would then violate the invariant, and thus in my patch I split 
up the ctor of QTabWidget into 2:

QTabWidget()
QTabWidget(QWidget *parent)

This is obviously backwards compatible.
The first ctor can be used without violating the invariant, whereas the 
second one violates it. And thus it's again optionally deprecated.

Now for some classes the parent ptr has additional meaning beyond just 
being the owner. E.g. for the layout classes. For that reason there's a 
bit of infrastructure, which makes this work:

parent->makeChild<QTabWidget>();

or to take a more complex example:

parent->makeChild<QLabel>("Hello World!");

(The infrastructure is a third constructor, and a forwarding function 
defined via Q_OBJECT)

For example, converting some random code on 
https://doc.qt.io/qt-5/qtwidgets-widgets-lineedits-example.html to use 
makeChild would look like this:

Window::Window(QWidget  <https://doc.qt.io/qt-5/qwidget.html>  *parent)
     :QWidget  <https://doc.qt.io/qt-5/qwidget.html>(parent)
{
   QGroupBox *echoGroup = makeChild<QGroupBox>(tr("Echo"));
   QLabel *echoLabel =echoGroup->makeChild<QLabel>(tr("Mode"));
   QComboBox *comboBox =echoGroup->makeChild<QComboBox>();
   [...]
   QGridLayout *echoLayout = echoGroup->makeChild<QGridLayout>();
echoLayout->addWidget(echoLabel,0,0);echoLayout->addWidget(echoComboBox,0,1);echoLayout->addWidget(echoLineEdit,1,0,1,2);
   

Or if using unique_ptrs:

Window::Window(QWidget  <https://doc.qt.io/qt-5/qwidget.html>  *parent)
     :QWidget  <https://doc.qt.io/qt-5/qwidget.html>(parent)
{
   auto echoGroup = std::make_unique<QGroupBox>(tr("Echo"));
   auto echoLabel = std::make_unique<QLabel>(tr("Mode"));
   auto comboBox = std::make_unique<QComboBox>();
   [...]
   auto echoLayout = std::make_unique<QGridLayout>();
echoLayout->addWidget(std::move(echoLabel),0,0);echoLayout->addWidget(std::move(echoComboBox),0,1);echoLayout->addWidget(std::move(echoLineEdit),1,0,1,2);echoGroup->setLayout(std::move(echoLayout));

And obviously the use can choose whatever they like, though I would 
recommend the first variant.

Imho this is the nicest aspect of this design. Adding lots of unique_ptr 
apis lead to no usage of unique_ptrs. Instead, there's just a clearer 
way to construct the usual parent-child relationships.

There are some details I'm omitting here, and some apis that needs more 
work but that's the core idea.

daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20200201/3e28430b/attachment-0001.html>