[Development] unique_ptr and Qt, Take 2

[Lars Knoll]

On 6 May 2019, at 20:23, André Pönitz <apoenitz at t-online.de<mailto:apoenitz at t-online.de>> wrote:

On Mon, May 06, 2019 at 07:41:05AM +0000, Lars Knoll wrote:
On 6 May 2019, at 09:30, Christian Kandeler
<christian.kandeler at qt.io<mailto:christian.kandeler at qt.io>> wrote:

On Sat, 04 May 2019 09:06:39 +0200 Allan Sandfeld Jensen
<kde at carewolf.com<mailto:kde at carewolf.com>> wrote:

On Samstag, 4. Mai 2019 00:43:10 CEST Thiago Macieira wrote:
On Friday, 3 May 2019 13:00:52 PDT Иван Комиссаров wrote:
Which should be considered bad practice and banned on an API
level

No way.

Are you going to forbid creation of QFile on the stack?

Perhaps QFile shouldn't be the same kind of base object type as
QWidgets? Or not use the same smart pointer.

Though even making QWidgets not allowed on the stack, while
sensible, would break a many of our tests, where we "abuse" that it
is technically possible in simple cases.

Doesn't almost every project create its main widget on the stack?

Not sure whether it’s most projects, but there certainly are users
doing it. And we’ve been using the pattern in our examples in some
cases as well. But I can relate to Allan that creating widgets on the
stack is bad style (as it conflicts with the way we memory manage
them), and if I’d have a choice today, I would probably prefer to
enforce them to be created on the heap by some means.

I wonder whether there's a solution in the following direction:

Have something like the following for each QObject derived class Foo
(for which it makes sense. E.g. perhaps all widgets, but not QFile):

namespace Qt {

class Foo
{
public:
  Foo() : p(new QFoo), owning(true) {}
   // more ctors forwarding to QFoo ctor

  ~Foo() { if (owning) delete p; }

   // make  movable but not copiable...

  // forward QFoo interface to p->

private:
  QPointer<QFoo> p;  // Maybe QFoo *p.
  bool owning;  // Perhaps somehow mangled into p
  ...
  // whatever else is needed. Shouldn't be much.
};

} // namespace Qt

Those things could be 'stack only', and be reasonably explicit
about ownership without being too much of a pain to use.

Example code could be sth like:

int main(...)
{
  // a will own its a.p, and will destroy it when w itself dies.
  Application a(...);

  // w will own its w.p, and will destroy it when w itself dies.
  MainWindow w;

  // addChild<Foo> would forward to QFoo's constructor and set 'owning'
  // to false on the resulting Foo, i.e. l is now known to be owned
  //  by something else, and will not destroy l.p in its dtor
  // w takes ownership of l, e.g. using the classic QObject
  // parent/child mechanisme on w.p and l.p. Or something else.
  Label l = w.addChild<Label>("Something");

  l.setSomething(..,); // call l.p->setSomething(...)

  w.show();  // call w.p->show()

  return a.exec(); // call a.p->show()

  // l will be destroyed, but not l.p as l.owning == false
  // w will be destroyed including w.p, and trigger the
  //   destruction of QItems created/owned by it, e.g. l.p
  // a will be destroyed including a.p
}

The main benefits I see here that this scheme can co-exist with current
code, i.e. code bases could slowly opt-in and migrate step by step, and
it does not add the need to sprinkle user side code with the typical
smart pointer line noise.

Interesting idea. I sometimes thought that if we’d be creating Qt from scratch, one could make it all value based and have the objects on the heap hidden behind it. This would give something similar. As an added benefit, we could probably allocate the object and it’s private in one go without requiring the TLS hacks that https://codereview.qt-project.org/#/c/165445/ uses.

This could maybe bet combined with a custom template class handling our pointers (instead of unique_ptr) that understands the parent/child ownership model, plus typedefs for the different QObject based classes in the Qt namespace.

Cheers,
Lars


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