[Development] [API] homogenising pimpled value class copying and moving
Marc Mutz
marc.mutz at kdab.com
Tue May 8 15:22:55 CEST 2012
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
More information about the Development
mailing list