[Development] [API] homogenising pimpled value class copying and moving

[Marc Mutz]

Hi,

I've just uploaded a patch series that aims to homogenise the value classes, 
at least the pimpled (d-pointered) ones, with respect to basic C++ operations 
like copying and moving. QEasingCurve can be seen as the prototype of these 
classes:
  a. member-swap()
  b. inline copy and move assignment operators, formulated in terms of swap()
  c. inline move constructor
I'm using Q_DECLARE_SHARED (do we need a separate Q_DECLARE_SCOPED for those 
classes that don't use Q*SharedDataPointer, but QScopedPointer?) to 
centralise some parts of the implementation, and member-swap makes this 
possible. I think we can safely rule out extra-tree users of 
Q_DECLARE_SHARED, since for some time now, Q_DECLARE_SHARED must be used 
within a QT_BEGIN_NAMESPACE/QT_END_NAMESPACE block.

I'm not 100% there, yet, but this is what I have so far:

1. Adding member-swaps to classes that still miss them:
  https://codereview.qt-project.org/25586 (QtCore)
  https://codereview.qt-project.org/25587 (QtGui)
  https://codereview.qt-project.org/25588 (QtDBus)
  https://codereview.qt-project.org/25589 (QtNetwork)

2. Changing Q_DECLARE_SHARED to require member-swap for the specialisation of
    std::swap/qSwap, instead of the public data_ptr() hack:
  https://codereview.qt-project.org/25590

At this point, we can Q_DECLARE_SHARED a lot more classes, because we don't 
need to leak the data_ptr() implementation detail anymore. Example: we can 
hide details like the additional members of QVariant, QFont, QPalette inside 
their swap() method, so they now can also be used with Q_DECLARE_SHARED.

We can also start to use Q_DECLARE_SHARED as a homogeniser for shared value 
classes. Eg., we can centrally mark them as movable, because by definition 
they all are:

3. Move Q_DECLARE_TYPEINFO( T, Q_MOVABLE_TYPE ) into Q_DECLARE_SHARED
  https://codereview.qt-project.org/25591

The next four patches make many more value classes Q_DECLARE_SHARED:

4. Declare classes as shared:
  https://codereview.qt-project.org/25592 (QtCore)
  https://codereview.qt-project.org/25593 (QtGui)
  https://codereview.qt-project.org/25594 (QtDBus)
  https://codereview.qt-project.org/25595 (QtNetwork)

The next goal is to centrally implement their copy assignment operators using 
the copy-swap idiom (more maintainable: all ref-count logic is in copy ctor 
and dtor, op= can be inline, strongly exception-safe).

Before doing that, we need to fix some classes that use op= in their 
implementation of the copy ctor:

5. QtGui: replace some copies with swaps:
  https://codereview.qt-project.org/25596

Now we can implement the copy assignment operator centrally in 
Q_DECLARE_SHARED:

6. inline copy assignment operators
  https://codereview.qt-project.org/25597

That's it for now. If you think these are a good idea, they'd need to go into 
5.0 (because the copy assignment operator is now inline). The plan is make 
variants of DECLARE_SHARED for template classes, too, and to add the move 
assignment operator like this:
  Foo &Foo::operator(Foo &&other) { other.swap(*this); return *this; }

I'm also giving each class a move constructor. There, the classes which hold 
their pimpl in smart pointers create the problem[1] that the move ctor cannot 
be inline. I'm tempted to remove the use of these in favour of going back to 
naked pointers, which allow the move ctor to be inline. Does anyone feel very 
strongly about that?

But all these patches need more cleanups first (and some can potentially wait 
for 5.1, as Qt doesn't seem to have any non-inline move operations yet).

[1] 
http://stackoverflow.com/questions/9417477/where-does-the-destructor-hide-in-this-code

Thanks,
Marc

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-Independent Software Solutions