From 409b3b42a233ca58c075a75f35be5ba9b351c7de Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Thu, 29 Sep 2011 11:52:33 +0200 Subject: [PATCH] Remove cast and assignment operators from QAtomicInt and QAtomicPointer This is a source incompatible change. There is concern that the convenience of the implicit cast and assignment operators can lead to misuse. Several commits have already been done that remove excess use of the implicit cast, which is a *volatile* read every time it's used. Users of the QAtomic* API should have to think about when they are loading the value, and if they do or don't need the acquire memory barrier on load. The code that people would write using this API is meant to be multi-threaded, concurrent, and correct. The API should not allow them to inadvertently, possibly unknowingly, shoot themselves in the foot. SC-break-rubber-stamped-by: Lars Knoll Change-Id: I88fbc26d9db7b5ec80a58ad6271ffa13bbfd191f Reviewed-by: Thiago Macieira --- src/corelib/thread/qatomic.h | 68 --------------- .../thread/qatomicint/tst_qatomicint.cpp | 77 +---------------- .../qatomicpointer/tst_qatomicpointer.cpp | 82 ++----------------- .../qsharedpointer/tst_qsharedpointer.cpp | 4 +- 4 files changed, 10 insertions(+), 221 deletions(-) diff --git a/src/corelib/thread/qatomic.h b/src/corelib/thread/qatomic.h index 34b90115754..eff2182e283 100644 --- a/src/corelib/thread/qatomic.h +++ b/src/corelib/thread/qatomic.h @@ -78,43 +78,12 @@ public: store(other.load()); } - Q_DECL_DEPRECATED - inline QAtomicInt &operator=(int value) - { - this->store(value); - return *this; - } - inline QAtomicInt &operator=(const QAtomicInt &other) { this->store(other.load()); return *this; } - Q_DECL_DEPRECATED - inline bool operator==(int value) const - { - return this->load() == value; - } - - Q_DECL_DEPRECATED - inline bool operator!=(int value) const - { - return this->load() != value; - } - - Q_DECL_DEPRECATED - inline operator int() const - { - return this->load(); - } - - Q_DECL_DEPRECATED - inline bool operator!() const - { - return !this->load(); - } - #ifdef qdoc static bool isReferenceCountingNative(); static bool isReferenceCountingWaitFree(); @@ -168,49 +137,12 @@ public: this->store(other.load()); } - Q_DECL_DEPRECATED - inline QAtomicPointer &operator=(T *value) - { - this->store(value); - return *this; - } - inline QAtomicPointer &operator=(const QAtomicPointer &other) { this->store(other.load()); return *this; } - Q_DECL_DEPRECATED - inline bool operator==(T *value) const - { - return this->load() == value; - } - - Q_DECL_DEPRECATED - inline bool operator!=(T *value) const - { - return this->load() != value; - } - - Q_DECL_DEPRECATED - inline bool operator!() const - { - return !this->load(); - } - - Q_DECL_DEPRECATED - inline operator T *() const - { - return this->load(); - } - - Q_DECL_DEPRECATED - inline T *operator->() const - { - return this->load(); - } - #ifdef qdoc static bool isTestAndSetNative(); static bool isTestAndSetWaitFree(); diff --git a/tests/auto/corelib/thread/qatomicint/tst_qatomicint.cpp b/tests/auto/corelib/thread/qatomicint/tst_qatomicint.cpp index 3c67f989f88..46068496881 100644 --- a/tests/auto/corelib/thread/qatomicint/tst_qatomicint.cpp +++ b/tests/auto/corelib/thread/qatomicint/tst_qatomicint.cpp @@ -61,14 +61,6 @@ private slots: void constructor(); void copy_constructor_data(); void copy_constructor(); - void equality_operator_data(); - void equality_operator(); - void inequality_operator_data(); - void inequality_operator(); - void not_operator_data(); - void not_operator(); - void cast_operator_data(); - void cast_operator(); void assignment_operator_data(); void assignment_operator(); @@ -185,71 +177,6 @@ void tst_QAtomicInt::copy_constructor() QCOMPARE(atomic5.load(), value); } -void tst_QAtomicInt::equality_operator_data() -{ - QTest::addColumn("value1"); - QTest::addColumn("value2"); - QTest::addColumn("result"); - - QTest::newRow("success0") << 1 << 1 << 1; - QTest::newRow("success1") << -1 << -1 << 1; - QTest::newRow("failure0") << 0 << 1 << 0; - QTest::newRow("failure1") << 1 << 0 << 0; - QTest::newRow("failure2") << 0 << -1 << 0; - QTest::newRow("failure3") << -1 << 0 << 0; -} - -void tst_QAtomicInt::equality_operator() -{ - QFETCH(int, value1); - QFETCH(int, value2); - QAtomicInt x = value1; - QTEST(x == value2 ? 1 : 0, "result"); -} - -void tst_QAtomicInt::inequality_operator_data() -{ - QTest::addColumn("value1"); - QTest::addColumn("value2"); - QTest::addColumn("result"); - - QTest::newRow("failure0") << 1 << 1 << 0; - QTest::newRow("failure1") << -1 << -1 << 0; - QTest::newRow("success0") << 0 << 1 << 1; - QTest::newRow("success1") << 1 << 0 << 1; - QTest::newRow("success2") << 0 << -1 << 1; - QTest::newRow("success3") << -1 << 0 << 1; -} - -void tst_QAtomicInt::inequality_operator() -{ - QFETCH(int, value1); - QFETCH(int, value2); - QAtomicInt x = value1; - QTEST(x != value2 ? 1 : 0, "result"); -} - -void tst_QAtomicInt::not_operator_data() -{ constructor_data(); } - -void tst_QAtomicInt::not_operator() -{ - QFETCH(int, value); - QAtomicInt atomic = value; - QCOMPARE(!atomic, !value); -} - -void tst_QAtomicInt::cast_operator_data() -{ constructor_data(); } - -void tst_QAtomicInt::cast_operator() -{ - QFETCH(int, value); - QAtomicInt atomic = value; - int copy = atomic; - QCOMPARE(copy, value); -} - void tst_QAtomicInt::assignment_operator_data() { QTest::addColumn("value"); @@ -271,9 +198,9 @@ void tst_QAtomicInt::assignment_operator() { QAtomicInt atomic1 = value; atomic1 = newval; - QCOMPARE(int(atomic1), newval); + QCOMPARE(atomic1.load(), newval); atomic1 = value; - QCOMPARE(int(atomic1), value); + QCOMPARE(atomic1.load(), value); QAtomicInt atomic2 = newval; atomic1 = atomic2; diff --git a/tests/auto/corelib/thread/qatomicpointer/tst_qatomicpointer.cpp b/tests/auto/corelib/thread/qatomicpointer/tst_qatomicpointer.cpp index 73266205afd..6be25f3dd3c 100644 --- a/tests/auto/corelib/thread/qatomicpointer/tst_qatomicpointer.cpp +++ b/tests/auto/corelib/thread/qatomicpointer/tst_qatomicpointer.cpp @@ -55,11 +55,7 @@ private slots: void constructor(); void copy_constructor(); - void equality_operator(); - void inequality_operator(); void assignment_operator(); - void star_operator(); - void dereference_operator(); void isTestAndSetNative(); void isTestAndSetWaitFree(); @@ -157,52 +153,6 @@ void tst_QAtomicPointer::copy_constructor() QCOMPARE(atomic3_copy.load(), atomic3.load()); } -void tst_QAtomicPointer::equality_operator() -{ - void *one = this; - void *two = &one; - void *three = &two; - - QAtomicPointer atomic1 = one; - QAtomicPointer atomic2 = two; - QAtomicPointer atomic3 = three; - - QVERIFY(atomic1 == one); - QVERIFY(!(atomic1 == two)); - QVERIFY(!(atomic1 == three)); - - QVERIFY(!(atomic2 == one)); - QVERIFY(atomic2 == two); - QVERIFY(!(atomic2 == three)); - - QVERIFY(!(atomic3 == one)); - QVERIFY(!(atomic3 == two)); - QVERIFY(atomic3 == three); -} - -void tst_QAtomicPointer::inequality_operator() -{ - void *one = this; - void *two = &one; - void *three = &two; - - QAtomicPointer atomic1 = one; - QAtomicPointer atomic2 = two; - QAtomicPointer atomic3 = three; - - QVERIFY(!(atomic1 != one)); - QVERIFY(atomic1 != two); - QVERIFY(atomic1 != three); - - QVERIFY(atomic2 != one); - QVERIFY(!(atomic2 != two)); - QVERIFY(atomic2 != three); - - QVERIFY(atomic3 != one); - QVERIFY(atomic3 != two); - QVERIFY(!(atomic3 != three)); -} - void tst_QAtomicPointer::assignment_operator() { void *one = this; @@ -213,37 +163,17 @@ void tst_QAtomicPointer::assignment_operator() QAtomicPointer atomic2 = two; QAtomicPointer atomic3 = three; - QVERIFY(atomic1 == one); - QVERIFY(atomic2 == two); - QVERIFY(atomic3 == three); + QVERIFY(atomic1.load() == one); + QVERIFY(atomic2.load() == two); + QVERIFY(atomic3.load() == three); atomic1 = two; atomic2 = three; atomic3 = one; - QVERIFY(atomic1 == two); - QVERIFY(atomic2 == three); - QVERIFY(atomic3 == one); -} - -struct Type -{ - inline const Type *self() const - { return this; } -}; - -void tst_QAtomicPointer::star_operator() -{ - Type t; - QAtomicPointer p = &t; - QCOMPARE((*p).self(), t.self()); -} - -void tst_QAtomicPointer::dereference_operator() -{ - Type t; - QAtomicPointer p = &t; - QCOMPARE(p->self(), t.self()); + QVERIFY(atomic1.load() == two); + QVERIFY(atomic2.load() == three); + QVERIFY(atomic3.load() == one); } void tst_QAtomicPointer::isTestAndSetNative() diff --git a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp index 416ce147beb..e62ee0f0511 100644 --- a/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp +++ b/tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp @@ -1545,8 +1545,8 @@ void tst_QSharedPointer::threadStressTest() // verify that the count is the right range int minValue = strongThreadCount; int maxValue = strongThreadCount + weakThreadCount; - QVERIFY(counter >= minValue); - QVERIFY(counter <= maxValue); + QVERIFY(counter.load() >= minValue); + QVERIFY(counter.load() <= maxValue); } }