diff options
author | Giuseppe D'Angelo <giuseppe.dangelo@kdab.com> | 2023-09-13 10:35:34 +0200 |
---|---|---|
committer | Giuseppe D'Angelo <giuseppe.dangelo@kdab.com> | 2023-10-25 04:47:47 +0200 |
commit | 6a93ec2435b8a453d43956fef6afa5f091d70ffc (patch) | |
tree | 893fe600fa355bbb44f44abb617b6f31a62e2419 /src/corelib/thread/qbasicatomic.h | |
parent | 6f9271ce7c9c7c238814bf03ddfb25d2ae5304e7 (diff) |
QAtomic: remove the copy ctor and assignment operator
These special member functions have no purpose.
We never *documented* their semantics. Any code using them is
unconditionally wrong (which semantics was it assuming?), so we can
accept the SIC (type A). If a user needs such a copy, they would have to
reason on the intended semantics (relaxed? acquire/release?) and be
explicit in their code. Especially for assignment, they would need
understand the consequences of the memory ordering that apply on _each_
atomic object involved and not on the assignment operation as a whole
(there are no such semantics).
Testing this change on qtbase has already found bugs.
From a purely technical point of view: we don't guarantee lock-free
atomics nor we require them from the underlying platform. An atomic is
therefore allowed to be implemented as a mutex protecting a value, and
mutexes are not copiable. std::atomic follows the exactly same pattern
(not copiable nor copy-assignable) for exactly the same reasons, and Qt
atomics are implemented on top of std:: ones.
[ChangeLog][QtCore] The copy constructor and assignment operators of
Qt atomic classes (QAtomicInteger, QAtomicPointer) have been removed.
Their usage in user code should be considered a programming error, as no
memory ordering semantics were ever documented for these operations (and
therefore relying on any specific semantic would be relying on
undocumented, unportable behavior). This matches the API of the
std::atomic class in C++. Note that you can still use explicit
load/store operations to transfer a value across two Qt atomic objects,
and therefore use the memory ordering specified for the load/store
operations.
Change-Id: Iab653bad761afb8b3e3b6a967ece7b28713aa944
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src/corelib/thread/qbasicatomic.h')
-rw-r--r-- | src/corelib/thread/qbasicatomic.h | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/src/corelib/thread/qbasicatomic.h b/src/corelib/thread/qbasicatomic.h index 6d061ea49a..4dd67f57c3 100644 --- a/src/corelib/thread/qbasicatomic.h +++ b/src/corelib/thread/qbasicatomic.h @@ -39,7 +39,7 @@ public: T loadAcquire() const noexcept { return Ops::loadAcquire(_q_value); } void storeRelease(T newValue) noexcept { Ops::storeRelease(_q_value, newValue); } operator T() const noexcept { return loadAcquire(); } - T operator=(T newValue) noexcept { storeRelease(newValue); return newValue; } + QBasicAtomicInteger &operator=(T newValue) noexcept { storeRelease(newValue); return *this; } static constexpr bool isReferenceCountingNative() noexcept { return Ops::isReferenceCountingNative(); } static constexpr bool isReferenceCountingWaitFree() noexcept { return Ops::isReferenceCountingWaitFree(); } @@ -171,7 +171,7 @@ public: void storeRelaxed(Type newValue) noexcept { Ops::storeRelaxed(_q_value, newValue); } operator Type() const noexcept { return loadAcquire(); } - Type operator=(Type newValue) noexcept { storeRelease(newValue); return newValue; } + QBasicAtomicPointer &operator=(Type newValue) noexcept { storeRelease(newValue); return *this; } // Atomic API, implemented in qatomic_XXX.h Type loadAcquire() const noexcept { return Ops::loadAcquire(_q_value); } |