From 889d40ebe2d9d0e92caea2749608720f7c088173 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Fri, 3 Jan 2020 17:18:02 +0100 Subject: Centralize the implementation of move assignment operators At the moment we have two main strategies for dealing with move assignment in Qt: 1) move-and-swap, used by "containers" (in the broad sense): containers, but also smart pointers and similar classes that can hold user-defined types; 2) pure swap, used by containers that hold only memory (e.g. QString, QByteArray, ...) as well as most implicitly shared datatypes. Given the fact that a move assignment operator's code is just boilerplate (whether it's move-and-swap or pure swap), provide two _strictly internal_ macros to help write them, and apply the macros across corelib and gui, porting away from the hand-rolled implementations. The rule of thumb when porting to the new macros is: * Try to stick to the existing code behavior, unless broken * if changing, then follow this checklist: * if the class does not have a move constructor => pure swap (but consider ADDING a move constructor, if possible!) * if the class does have a move constructor, try to follow the criteria above, namely: * if the class holds only memory, pure swap; * if the class may hold anything else but memory (file handles, etc.), then move and swap. Noteworthy details: * some operators planned to be removed in Qt 6 were not ported; * as drive-by, some move constructors were simplified to be using qExchange(); others were outright broken and got fixed; * some contained some more interesting code and were not touched. Change-Id: Idaab3489247dcbabb6df3fa1e5286b69e1d372e9 Reviewed-by: Allan Sandfeld Jensen --- src/gui/painting/qbrush.h | 3 +-- src/gui/painting/qcolorspace.h | 7 +------ src/gui/painting/qcolortransform.h | 6 +----- src/gui/painting/qpagelayout.h | 2 +- src/gui/painting/qpagesize.h | 2 +- src/gui/painting/qpainterpath.h | 3 +-- src/gui/painting/qpen.h | 5 ++--- src/gui/painting/qregion.h | 5 ++--- 8 files changed, 10 insertions(+), 23 deletions(-) (limited to 'src/gui/painting') diff --git a/src/gui/painting/qbrush.h b/src/gui/painting/qbrush.h index 3b7b949bde..fea5afad57 100644 --- a/src/gui/painting/qbrush.h +++ b/src/gui/painting/qbrush.h @@ -78,8 +78,7 @@ public: ~QBrush(); QBrush &operator=(const QBrush &brush); - inline QBrush &operator=(QBrush &&other) noexcept - { qSwap(d, other.d); return *this; } + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QBrush) inline void swap(QBrush &other) noexcept { qSwap(d, other.d); } diff --git a/src/gui/painting/qcolorspace.h b/src/gui/painting/qcolorspace.h index 08c9944301..7505c6eca7 100644 --- a/src/gui/painting/qcolorspace.h +++ b/src/gui/painting/qcolorspace.h @@ -95,12 +95,7 @@ public: QColorSpace(QColorSpace &&colorSpace) noexcept : d_ptr(qExchange(colorSpace.d_ptr, nullptr)) { } - QColorSpace &operator=(QColorSpace &&colorSpace) noexcept - { - // Make the deallocation of this->d_ptr happen in ~QColorSpace() - QColorSpace(std::move(colorSpace)).swap(*this); - return *this; - } + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QColorSpace) void swap(QColorSpace &colorSpace) noexcept { qSwap(d_ptr, colorSpace.d_ptr); } diff --git a/src/gui/painting/qcolortransform.h b/src/gui/painting/qcolortransform.h index 94b6b3a385..fa52f2e152 100644 --- a/src/gui/painting/qcolortransform.h +++ b/src/gui/painting/qcolortransform.h @@ -64,11 +64,7 @@ public: QColorTransform{other}.swap(*this); return *this; } - QColorTransform &operator=(QColorTransform &&other) noexcept - { - QColorTransform{std::move(other)}.swap(*this); - return *this; - } + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QColorTransform) void swap(QColorTransform &other) noexcept { qSwap(d, other.d); } diff --git a/src/gui/painting/qpagelayout.h b/src/gui/painting/qpagelayout.h index 6965606252..a7858edfaf 100644 --- a/src/gui/painting/qpagelayout.h +++ b/src/gui/painting/qpagelayout.h @@ -81,7 +81,7 @@ public: const QMarginsF &margins, Unit units = Point, const QMarginsF &minMargins = QMarginsF(0, 0, 0, 0)); QPageLayout(const QPageLayout &other); - QPageLayout &operator=(QPageLayout &&other) noexcept { swap(other); return *this; } + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QPageLayout) QPageLayout &operator=(const QPageLayout &other); ~QPageLayout(); diff --git a/src/gui/painting/qpagesize.h b/src/gui/painting/qpagesize.h index 1fa6c86081..fbaec0da3f 100644 --- a/src/gui/painting/qpagesize.h +++ b/src/gui/painting/qpagesize.h @@ -232,7 +232,7 @@ public: const QString &name = QString(), SizeMatchPolicy matchPolicy = FuzzyMatch); QPageSize(const QPageSize &other); - QPageSize &operator=(QPageSize &&other) noexcept { swap(other); return *this; } + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QPageSize) QPageSize &operator=(const QPageSize &other); ~QPageSize(); diff --git a/src/gui/painting/qpainterpath.h b/src/gui/painting/qpainterpath.h index a839a73b4b..0099a7408a 100644 --- a/src/gui/painting/qpainterpath.h +++ b/src/gui/painting/qpainterpath.h @@ -92,8 +92,7 @@ public: explicit QPainterPath(const QPointF &startPoint); QPainterPath(const QPainterPath &other); QPainterPath &operator=(const QPainterPath &other); - inline QPainterPath &operator=(QPainterPath &&other) noexcept - { qSwap(d_ptr, other.d_ptr); return *this; } + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QPainterPath) ~QPainterPath(); inline void swap(QPainterPath &other) noexcept { d_ptr.swap(other.d_ptr); } diff --git a/src/gui/painting/qpen.h b/src/gui/painting/qpen.h index 38282bda39..db280c765b 100644 --- a/src/gui/painting/qpen.h +++ b/src/gui/painting/qpen.h @@ -71,9 +71,8 @@ public: QPen &operator=(const QPen &pen) noexcept; QPen(QPen &&other) noexcept - : d(other.d) { other.d = nullptr; } - QPen &operator=(QPen &&other) noexcept - { qSwap(d, other.d); return *this; } + : d(qExchange(other.d, nullptr)) {} + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QPen) void swap(QPen &other) noexcept { qSwap(d, other.d); } Qt::PenStyle style() const; diff --git a/src/gui/painting/qregion.h b/src/gui/painting/qregion.h index 8b991f2fb2..8f52185ed9 100644 --- a/src/gui/painting/qregion.h +++ b/src/gui/painting/qregion.h @@ -70,12 +70,11 @@ public: QRegion(const QPolygon &pa, Qt::FillRule fillRule = Qt::OddEvenFill); QRegion(const QRegion ®ion); QRegion(QRegion &&other) noexcept - : d(other.d) { other.d = const_cast(&shared_empty); } + : d(qExchange(other.d, const_cast(&shared_empty))) {} QRegion(const QBitmap &bitmap); ~QRegion(); QRegion &operator=(const QRegion &); - inline QRegion &operator=(QRegion &&other) noexcept - { qSwap(d, other.d); return *this; } + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QRegion); inline void swap(QRegion &other) noexcept { qSwap(d, other.d); } bool isEmpty() const; bool isNull() const; -- cgit v1.2.1