[Development] The future of smart pointers in Qt API

[Daniel Teske]

> Hi Daniel,
>
> I like many things in this proposal, especially the principles; thanks for that!
>
> Where I’m on the fence is the replacing of setParent with “adoptChild”. I think of “parent” as a property of an object, so setParent/parent accessors are the API that fits into my mental model. I have a hard time thinking about “list of children” as that property..

Well this should not compile. (Or rather be optionally deprecated.)

unique_ptr<> a;
a->setParent(b);


>
> But I think the example below shows exactly the problem <cliff_hanger>
>
>> 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
>>   *parent)
>>      :
>> QWidget
>> (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);
>>
>
> This works, but now you are encoding the visual hierarchy of the widgets in two places: when asking the presumed parent to create the widgets; and when setting up the layout, which might do things differently (esp once you start refactoring complex UIs).#

That's not a problem. The code simply behaves as if you would have used 
the current constructors that take a parent parameter.

Ownership transfers between different qt parents don't violate the 
invariant. And they continue to work just like before.


>
>
>
>> Or if using unique_ptrs:
>>
>> Window::Window(QWidget
>>   *parent)
>>      :
>> QWidget
>> (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);
>
> I don’t think these lines above are correct.
>
> QBoxLayout::addWidget does *not* take ownership. Widgets are reparented to their correct parent once you call QWidget::setLayout in the next line, and QLayout operates on QLayoutItems that hold a weak pointer to the widgets that the layout places.

It's even more complicated than that, if the layout is already set on a 
widget a call to QBoxLayout::addWidget does transfer ownership.

But for a parent-less QBoxLayout::addWidget(QWidget *) does not take 
ownership. So whether a call to addWidget does transfer ownership is 
really hard to tell.

Which I personally find surprising and I actually missed that in my 
patch and Giuseppe had to point it out.

But that just means that QBoxLayout::addWidget(std::unique_ptr<>) has to 
behave imho saner in that edge case and actually delete the widgets that 
were added but never parented. (And I should really implement that to 
show how it would work.)

There's another pain point in the patch currently, and that has to do 
with the overloaded meaning of the parent ptr for QDialog. For QDialog 
in addition to the memory ownership that also controls modality. The 
other cases of overloaded meaning that I found in qtbase are as far as I 
can tell fine.

Meaning a line such as this:

QDialog dialog(parent);

is technically a doubly owned object, because it's both owned by the 
stack and by the passed parent. And that same problem makes it hard to 
make this memory safe.

The solution to that, is to fix Qt here. Modality and parent should not 
be coupled in this way.

daniel