[Development] unique_ptr and Qt, Take 2

[Daniel Teske]

Иван Комиссаров:
> IMHO, it should be a Qt6 feature. It’s awesome.
>
> However, I liked the idea (is it deprecated or what?) of a template 
> method
> template<typename T, typename Args…>
> std::observer_ptr<QObject> 
> qMakeChild(gsll::not_null<std::observer_ptr<QObject>> parent, Args… args)
> {
>     return parent->adoptChild(std::make_unique<T>(args));
> }

For some classes, notably QLayout constructing with a parent is 
different from setting a parent later. The former automatically sets the 
layout, whereas the later doesn't.

So, what you would need to do is in pseudo code:

std::observer_ptr<QObject> 
qMakeChild(gsll::not_null<std::observer_ptr<QObject>> parent, Args… args)
{
      auto obj = std::make_unique<T>(args);
      if hasMember(T:initWithParent)
          obj->initWithParent(parent)
     return parent->adoptChild(std::move(obj));
}

And obviously create a initWithParent method on all classes that are 
special like QLayout. I didn't explore that, as it didn't feel that nice 
to me


>
> Maybe I’m asking too much, but it would be nice get rid of raw 
> pointers completely switching to pair std::unique_ptr/std::observer_ptr
>
I don't think observer_ptr has only miniscule benefit over a raw pointer 
and I think it's ugly.

Thiago Macieira:
> On Friday, 3 May 2019 10:22:20 PDT Daniel Teske wrote:
>> std::unique_ptr<QPushButton> rightButton =
>> std::make_unique<QPushButton>("RIGHT");
>> layout->addWidget(std::move(rightButton));
> The problem in this particular example is that once you've added the 
> widget,
> the rightButton smart pointer no longer has a pointer. You can't 
> continue to
> set up your push button. In most cases, this is just a matter of 
> moving the
> set up before the addition / reparenting, or using the other idiom 
> where the
> object is never in a smart pointer in the first place.
>
> So this begs the question of whether std::unique_ptr is the best smart 
> pointer
> for this scenario. Would it make sense to create one that understands 
> parent-
> child relationship?

addWidget could return a raw pointer to he widget, also that would 
require making addWidget and a bunch of other functions templates.

(They need to be templates, because we want to return the subclass that 
is stored in the unique_ptr, that is: T * addWidget(std::unique_ptr<T>) 
is the right signature.)

But, on the topic of using our own smart pointer class.

I think unique_ptr actually works well enough with Qt's parent/child 
relationship with the right apis, because both use similar enough concepts.

Using the standards mechanism for memory management has the benefit that 
it is well-known outside the Qt world and aligns Qt better with modern 
practices, whereas our own smart pointer is again our own very different 
corner of C++.

I think that standard C++ is developing quite rapidly and for Qt's to 
profit from that development it needs to try harder to adopt changes in 
C++. And I believe that to be critical to Qt's long term success.

> std::unique_ptr<QPushButton> rightButton = 
> std::make_unique<QPushButton>("RIGHT");
> layout->addWidget(std::move(rightButton)); 

> To really, really, really nitpick, this last line is (or feels) wrong: 
> layouts do not take ownership of the widgets they manage. An ownership 
> transfer MAY happen immediately (if the layout is installed) or later 
> (as soon as the layout is installed). 
I actually didn't know that and must have missed it. ( There are 
obviously lots of apis, I did check the documentation and read the 
implementation of most functions, but it's rather likely that a bunch of 
my implementations miss subtle details.)

So, what the function should do is to always do take ownership and if 
that requires behaving a bit differently than addWidget, so be it.

For example one case where I did catch something like that, the two 
variants of QAbstractItemView::setIndexWidgetbehave slightly differently.

* If the model index is valid, both take ownership of the widget.
* If the model index is invalid
     * The normal version does nothing
     * The unique_ptr version deletes the widget

Now, I'm in a bit of a hurry atm, but QLayout basically would need to 
ensure that any unique_ptrs that it took ownership of are either passed 
on to its parent widget, or are deleted in ~QLayout.

daniel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20190504/0a743cad/attachment.html>