<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>