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 <[email protected]> | 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 _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
