<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>
</p>
<div class="moz-text-html" lang="x-unicode">
<p>Hi,</p>
<p>I've done some research into how supporting unique_ptr in Qt
might look like.</p>
<p>I have a patch which adds apis using unique_ptr for ownership
transfer to almost all of QtBase and a patch for Qt Creator to
show how using those apis looks like. <br>
</p>
<p>Qt: <a class="moz-txt-link-freetext"
href="https://codereview.qt-project.org/231543">https://codereview.qt-project.org/231543</a><br>
Qt Creator: <a class="moz-txt-link-freetext"
href="https://codereview.qt-project.org/231545">https://codereview.qt-project.org/231545</a><br>
</p>
<p>It's nowhere finished, and I have no expectations of it going
into Qt in the near future, nor the intention to finish the
patch. The intent was, to have some fun, to explore the space of
what is possible, to show how it could look like and to maybe
start a discussion on what Qt can and should do in that
direction. Also I run out of time to properly polish it.<br>
</p>
<p>This patch is backwards compatible, existing code continues to
work exactly the same and has a very high degree of source
compatibility. It allows users to opt-in to warnings that
indicate unclear ownership semantics and allows for piece by
piece porting of code. It requires C++17 though. And
unfortunately due to a bug MSVC 15.7 is not good enough. That
bug in guaranteed copy elision is supposed to be fixed in 15.8.<br>
</p>
<p>The additional apis are enabled via a configure flag and are
inline functions.<br>
</p>
<p>The core idea is this invariant:</p>
<p>A Qt Object either is owned by a unique_ptr, is owned by a
parent, or is allocated on the stack. (A stack allocated object,
might or might not have a parent.)<br>
</p>
<p>Thus the first addition are 3 new functions for creating
objects, all very similar to std::make_unique.<br>
</p>
<p>std::unique_ptr<T> qmakeUnique<T>(Args ...);<br>
T* QObject::makeChild(Args ...);<br>
T makeOnStack(ParentType, Args ...);<br>
</p>
<p>Those functions ensure that the returned object observe the
invariant. That is, qmakeUnique passes a nullptr as the parent
pointer to T. And widget->makeChild(...) passes widget as
the parent pointer. Thus if the constructor uses the parent
pointer to set it's parent, the invariant holds and is ensured
at compile time. Some types, e.g. QEvent, QTreeWidgetItem are
treated a bit differently, see the patch for details.<br>
</p>
<p>I'll discuss various classes of functions:</p>
<p>* Functions "taking" ownership, e.g. QLayout::addWidget( child
)<br>
</p>
<p>Note: Qt functions that "take" ownership, don't actually always
take ownership. If the passed in argument is already owned by
the right parent, ownership is not transferred.</p>
<p>To ensure that the invariant holds, there are two cases to
consider:</p>
<p>a) The child is owned by a parent. => Use the
addWidget(QWidget *rawPointer) overload<br>
b) The child is owned by a unique_ptr => Use the new
addWidget(std::unique_ptr<QWidget> widget) overload<br>
</p>
<p>For example this code is then possible:<br>
</p>
<p>auto label = qmakeUnique("label text");<br>
layout->addWidget(std::move(label));</p>
<p>A undiagnosed and potential pitfall, is this code:<br>
</p>
<p>auto label = qmakeUnique("label text");<br>
layout->addWidget(label.get());</p>
<p>which leads to double deletion. Calling a qt function that
takes ownership with a raw pointer derived from a unique_ptr is
a user bug. While obvious in this case, that isn't always so.<br>
</p>
<p>* Functions releasing ownership / creating new objects<br>
</p>
<p>There aren't actually not a lot of functions in this category.
For example QLayout::removeWidget() does not release ownership
of the widget, instead ownership is transferred if the widget is
added to a different layout.</p>
<p>For functions that actually do release ownership / create
objects, add a function returning a unique_ptr mostly named
either "makeXXX" or "releaseXXX".</p>
<p>So how do I prevent users from calling the old function? <br>
</p>
<p>a) Not at all, this makes the code fully backwards compatible.<br>
b) The user can opt into deprecation warnings by defining: <span
style=" color:#800080;">QT_DEPRECATED_UNSAFE_API</span> For Qt
Creator patch, I defined that only in the coreplugin plugin.</p>
<p>* Other points<br>
I probably missed quite a few special cases, since my aim was to
cover as much as qtbase as possible, without spending too much
time on each individual class. But for the most part, there
weren't that many special cases. I left a few "TODO"s in the
patch for apis that would take more work. I don't think there
was anything completely unsolvable, but quite a few pain points.<br>
</p>
<p>Having unique_ptr as a vocabulary type also has some other
benefits:</p>
<p>Some apis in Qt would really love to have a way to ensure that
the caller takes care of deleting the resource. But because
there is no way in C++98 to ensure that, they don't transfer
ownership. For example, the documentation of
QTcpServer::nextPendingConnection() is:</p>
<p>> The socket is created as a child of the server, which
means that it is automatically deleted when the <a
href="https://doc.qt.io/qt-5/qtcpserver.html">QTcpServer</a>
object is destroyed. <br>
> It is still a good idea to delete the object explicitly
when you are done with it, to avoid wasting memory.</p>
<p>With unique_ptr there would be a way to ensure that the caller
takes ownership by returning the QTcpSocket wrapped in a
unique_ptr.<br>
</p>
<p>Also while writing this patch, I noticed several functions
whose ownership semantics are too strange for my taste and
should arguably be changed even without adding unique_ptr. For
example: void QGraphicsGridLayout::removeAt(int index) transfers
ownership of a item that is neither a parameter nor a return
value.</p>
<p>I also ported a very small part of Creator to use those new
apis and I actually found only one place that leaks memory. Some
of the code looks arguably better, imho makeChild does make the
code easier to read. In other parts, Creator has fancy memory
ownership semantics, which are not easily representable with
unique_ptr's. And unique_ptr can be bit awkward to use.</p>
<p>And as a last though, having unique_ptr as part of the
vocabulary types also allows for better apis:</p>
<p>Consider QScrollArea and its functions: void setWidget(QWidget
*w); void setCornerWidget(QWidget *w);</p>
<p>Even though they have the exact same signature, and very
similar names, their behaviour is different in how they treat
the old replaced widget.</p>
<p>setWidget() deletes the old widget, there is afaik no way to
reuse a widget once it has been set via setWidget.</p>
<p>On the other hand, setCornerWidget() merely hides the old
widget. (I think the use case here is switching between multiple
corner widgets.)</p>
<p>With unique_ptr, a imho better api would be:</p>
<p>unique_ptr<QWidget *>
replaceWidget/replaceCornerWidget(QWidget *)</p>
<p>If the user does not care about the old widget, just ignoring
the return value gets the old widget deleted. If the user wants
to reuse the widget somewhere, s/he has ownership of it.<br>
</p>
<p>* Short guide to patch reading:</p>
<p>Since the patches are pretty big and most of it is pretty
uninteresting, for reading it I suggest:</p>
<p>1) Look at the changes in qglobal.h.<br>
2) Look at the qwidget.h<br>
3) Look at qtreewidgetitem.h<br>
4) Look at the creator patch at a random file<br>
5) Look at TODO comments which explain some of the remaining
problems<br>
</p>
<p>* My Conclusion</p>
<p>Adding unique_ptr supporting apis is possible. (Though clearly
this patch would need a lot of refining.) But, it adds a lot of
complexity, and can be awkward to use. On the other hand, as
more users become accustomed with using unique_ptr in their
code, the demand for Qt to support them will grow. So imho
research in that direction must happen.<br>
</p>
<p>Since the mail is already pretty long, I'll stop here. If you
have questions or suggestions, due to some technical
difficulties, I can't respond until Friday.</p>
<p>I'll be at the Contributor Summit, so if there's sufficient
interest I can add a session to the schedule.</p>
<p>daniel<br>
</p>
</div>
</body>
</html>