1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QSharedDataPointer: Use new comparison helper macros

Browse Source 
Provide the new comparesEqual() helper function as an implementation of
the (in)equality operators and compareThreeWay() helper function for
the rest of the relational operators.
Use Q_DECLARE_STRONGLY_ORDERED to provide all relational operators.

Use the new Qt::totally_ordered_wrapper to wrap the "d" pointer to
avoid UB when performing comparisons of QSharedDataPointer.

The old implementation of the operators between QSharedDataPointer
and the normal pointer T* used to cause a bug but this was not noticed
because QSharedDataPointer didn't have any tests.
I added tests and tried to compare using operator<(). I got
build issues:
       error: no match for call to
       ‘(std::less<MyClass*>) (const MyClass*&, MyClass* const&)’
This happens because QSharedDataPointer has an implicit conversion
to MyClass*.
Making the normal pointer T* non-const as follow resolves the issue:
 DECLARE_COMPARE_SET(const QSharedDataPointer &p1, p1.d, T *ptr, ptr)
 DECLARE_COMPARE_SET(T *ptr, ptr, const QSharedDataPointer &p2, p2.d)

A similar bug happens if we make the normal pointer const in the
comparison macro Q_DECLARE_STRONGLY_ORDERED. The relational operators
will be ambiguous. The compiler picks between
operator<(QSharedDataPointer<MyClass>, MyClass*) and
opeartor<(MyClass*, MyClass*) (In case we're calling <).
Therefore, use:
   Q_DECLARE_STRONGLY_ORDERED(QSharedDataPointer, T*)

Add some comparisons related tests for QSharedDataPointer.
Use QT_TEST_ALL_COMPARISON_OPS macros in unit-tests.

Task-number: QTBUG-120306
Change-Id: I8d1159a01651bab442b79a89c53e064914103133
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
This commit is contained in:
Rym Bouabid 2024-04-03 16:56:34 +02:00
parent aca8235c75
commit 7131240754
8 changed files with 156 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/corelib/io/qprocess_p.h
View File

@ -182,8 +182,8 @@ template<> Q_INLINE_TEMPLATE void QSharedDataPointer<QProcessEnvironmentPrivate>
: new QProcessEnvironmentPrivate);
x->ref.ref();
if (d && !d->ref.deref())
delete d;
d = x;
delete d.get();
d.reset(x);
}
#if QT_CONFIG(process)

4
src/corelib/io/qurlquery.cpp
View File

@ -174,8 +174,8 @@ template<> void QSharedDataPointer<QUrlQueryPrivate>::detach()
: new QUrlQueryPrivate);
x->ref.ref();
if (d && !d->ref.deref())
delete d;
d = x;
delete d.get();
d.reset(x);
}
// Here's how we do the encoding in QUrlQuery

12
src/corelib/tools/qshareddata.cpp
View File

@ -49,6 +49,10 @@ QT_BEGIN_NAMESPACE
\since 4.0
\reentrant
\compares strong
\compareswith strong T* std::nullptr_t
\endcompareswith
QSharedDataPointer\<T\> makes writing your own \l {implicitly
shared} classes easy. QSharedDataPointer implements \l {thread-safe}
reference counting, ensuring that adding QSharedDataPointers to your
@ -331,13 +335,13 @@ QT_BEGIN_NAMESPACE
\e{d pointer}. This function does \e not call detach().
*/
/*! \fn template <class T> bool QSharedDataPointer<T>::operator==(const T *ptr, const QSharedDataPointer<T>& rhs)
Returns \c true if the \e{d pointer} of \a rhs is \a ptr.
/*! \fn template <class T> bool QSharedDataPointer<T>::operator==(T* const &lhs, const QSharedDataPointer<T>& rhs)
Returns \c true if the \e{d pointer} of \a rhs is \a lhs.
This function does \e not call detach().
*/
/*! \fn template <class T> bool QSharedDataPointer<T>::operator!=(const T *ptr, const QSharedDataPointer<T>& rhs)
Returns \c true if the \e{d pointer} of \a rhs is \e not \a ptr.
/*! \fn template <class T> bool QSharedDataPointer<T>::operator!=(T* const &lhs, const QSharedDataPointer<T>& rhs)
Returns \c true if the \e{d pointer} of \a rhs is \e not \a lhs.
\e{d pointer}. This function does \e not call detach().
*/

98
src/corelib/tools/qshareddata.h
View File

@ -6,6 +6,7 @@
#include <QtCore/qglobal.h>
#include <QtCore/qatomic.h>
#include <QtCore/qcompare.h>
#include <QtCore/qhashfunctions.h>
#include <functional>
@ -38,22 +39,22 @@ public:
typedef T *pointer;
void detach() { if (d && d->ref.loadRelaxed() != 1) detach_helper(); }
T &operator*() { detach(); return *d; }
const T &operator*() const { return *d; }
T *operator->() { detach(); return d; }
const T *operator->() const noexcept { return d; }
operator T *() { detach(); return d; }
operator const T *() const noexcept { return d; }
T *data() { detach(); return d; }
T *get() { detach(); return d; }
const T *data() const noexcept { return d; }
const T *get() const noexcept { return d; }
const T *constData() const noexcept { return d; }
T *take() noexcept { return std::exchange(d, nullptr); }
T &operator*() { detach(); return *(d.get()); }
const T &operator*() const { return *(d.get()); }
T *operator->() { detach(); return d.get(); }
const T *operator->() const noexcept { return d.get(); }
operator T *() { detach(); return d.get(); }
operator const T *() const noexcept { return d.get(); }
T *data() { detach(); return d.get(); }
T *get() { detach(); return d.get(); }
const T *data() const noexcept { return d.get(); }
const T *get() const noexcept { return d.get(); }
const T *constData() const noexcept { return d.get(); }
T *take() noexcept { return std::exchange(d, nullptr).get(); }
Q_NODISCARD_CTOR
QSharedDataPointer() noexcept : d(nullptr) { }
~QSharedDataPointer() { if (d && !d->ref.deref()) delete d; }
~QSharedDataPointer() { if (d && !d->ref.deref()) delete d.get(); }
Q_NODISCARD_CTOR
explicit QSharedDataPointer(T *data) noexcept : d(data)
@ -67,10 +68,10 @@ public:
void reset(T *ptr = nullptr) noexcept
{
if (ptr != d) {
if (ptr != d.get()) {
if (ptr)
ptr->ref.ref();
T *old = std::exchange(d, ptr);
T *old = std::exchange(d, Qt::totally_ordered_wrapper(ptr)).get();
if (old && !old->ref.deref())
delete old;
}
@ -78,7 +79,7 @@ public:
QSharedDataPointer &operator=(const QSharedDataPointer &o) noexcept
{
reset(o.d);
reset(o.d.get());
return *this;
}
inline QSharedDataPointer &operator=(T *o) noexcept
@ -96,33 +97,34 @@ public:
void swap(QSharedDataPointer &other) noexcept
{ qt_ptr_swap(d, other.d); }
#define DECLARE_COMPARE_SET(T1, A1, T2, A2) \
friend bool operator<(T1, T2) noexcept \
{ return std::less<T*>{}(A1, A2); } \
friend bool operator<=(T1, T2) noexcept \
{ return !std::less<T*>{}(A2, A1); } \
friend bool operator>(T1, T2) noexcept \
{ return std::less<T*>{}(A2, A1); } \
friend bool operator>=(T1, T2) noexcept \
{ return !std::less<T*>{}(A1, A2); } \
friend bool operator==(T1, T2) noexcept \
{ return A1 == A2; } \
friend bool operator!=(T1, T2) noexcept \
{ return A1 != A2; } \
DECLARE_COMPARE_SET(const QSharedDataPointer &p1, p1.d, const QSharedDataPointer &p2, p2.d)
DECLARE_COMPARE_SET(const QSharedDataPointer &p1, p1.d, const T *ptr, ptr)
DECLARE_COMPARE_SET(const T *ptr, ptr, const QSharedDataPointer &p2, p2.d)
DECLARE_COMPARE_SET(const QSharedDataPointer &p1, p1.d, std::nullptr_t, nullptr)
DECLARE_COMPARE_SET(std::nullptr_t, nullptr, const QSharedDataPointer &p2, p2.d)
protected:
T *clone();
private:
friend bool comparesEqual(const QSharedDataPointer &lhs, const QSharedDataPointer &rhs) noexcept
{ return lhs.d == rhs.d; }
friend Qt::strong_ordering
compareThreeWay(const QSharedDataPointer &lhs, const QSharedDataPointer &rhs) noexcept
{ return Qt::compareThreeWay(lhs.d, rhs.d); }
Q_DECLARE_STRONGLY_ORDERED(QSharedDataPointer)
friend bool comparesEqual(const QSharedDataPointer &lhs, const T *rhs) noexcept
{ return lhs.d == rhs; }
friend Qt::strong_ordering
compareThreeWay(const QSharedDataPointer &lhs, const T *rhs) noexcept
{ return Qt::compareThreeWay(lhs.d, rhs); }
Q_DECLARE_STRONGLY_ORDERED(QSharedDataPointer, T*)
friend bool comparesEqual(const QSharedDataPointer &lhs, std::nullptr_t) noexcept
{ return lhs.d == nullptr; }
friend Qt::strong_ordering
compareThreeWay(const QSharedDataPointer &lhs, std::nullptr_t) noexcept
{ return Qt::compareThreeWay(lhs.d, nullptr); }
Q_DECLARE_STRONGLY_ORDERED(QSharedDataPointer, std::nullptr_t)
void detach_helper();
T *d;
Qt::totally_ordered_wrapper<T *> d;
};
template <typename T>
@ -198,6 +200,20 @@ public:
void swap(QExplicitlySharedDataPointer &other) noexcept
{ qt_ptr_swap(d, other.d); }
#define DECLARE_COMPARE_SET(T1, A1, T2, A2) \
friend bool operator<(T1, T2) noexcept \
{ return std::less<T*>{}(A1, A2); } \
friend bool operator<=(T1, T2) noexcept \
{ return !std::less<T*>{}(A2, A1); } \
friend bool operator>(T1, T2) noexcept \
{ return std::less<T*>{}(A2, A1); } \
friend bool operator>=(T1, T2) noexcept \
{ return !std::less<T*>{}(A1, A2); } \
friend bool operator==(T1, T2) noexcept \
{ return A1 == A2; } \
friend bool operator!=(T1, T2) noexcept \
{ return A1 != A2; } \
DECLARE_COMPARE_SET(const QExplicitlySharedDataPointer &p1, p1.d, const QExplicitlySharedDataPointer &p2, p2.d)
DECLARE_COMPARE_SET(const QExplicitlySharedDataPointer &p1, p1.d, const T *ptr, ptr)
DECLARE_COMPARE_SET(const T *ptr, ptr, const QExplicitlySharedDataPointer &p2, p2.d)
@ -227,9 +243,9 @@ Q_OUTOFLINE_TEMPLATE void QSharedDataPointer<T>::detach_helper()
{
T *x = clone();
x->ref.ref();
if (!d->ref.deref())
delete d;
d = x;
if (!d.get()->ref.deref())
delete d.get();
d.reset(x);
}
template <typename T>
@ -280,7 +296,7 @@ template<typename T> Q_DECLARE_TYPEINFO_BODY(QExplicitlySharedDataPointer<T>, Q_
template<> QSharedDataPointer<Class>::~QSharedDataPointer() \
{ \
if (d && !d->ref.deref()) \
delete d; \
delete d.get(); \
}
#define QT_DECLARE_QESDP_SPECIALIZATION_DTOR(Class) \

8
src/network/kernel/qnetworkproxy.cpp
View File

@ -450,8 +450,8 @@ template<> void QSharedDataPointer<QNetworkProxyPrivate>::detach()
: new QNetworkProxyPrivate);
x->ref.ref();
if (d && !d->ref.deref())
delete d;
d = x;
delete d.get();
d.reset(x);
}
/*!
@ -937,8 +937,8 @@ template<> void QSharedDataPointer<QNetworkProxyQueryPrivate>::detach()
: new QNetworkProxyQueryPrivate);
x->ref.ref();
if (d && !d->ref.deref())
delete d;
d = x;
delete d.get();
d.reset(x);
}
/*!

1
tests/auto/corelib/tools/CMakeLists.txt
View File

@ -44,6 +44,7 @@ add_subdirectory(qscopeguard)
add_subdirectory(qtaggedpointer)
add_subdirectory(qtyperevision)
add_subdirectory(qset)
add_subdirectory(qshareddatapointer)
add_subdirectory(qsharedpointer)
add_subdirectory(qsize)
add_subdirectory(qsizef)

19
tests/auto/corelib/tools/qshareddatapointer/CMakeLists.txt Normal file
View File

@ -0,0 +1,19 @@
# Copyright (C) 2024 The Qt Company Ltd.
# SPDX-License-Identifier: BSD-3-Clause
#####################################################################
## tst_qshareddatapointer Test:
#####################################################################
if(NOT QT_BUILD_STANDALONE_TESTS AND NOT QT_BUILDING_QT)
cmake_minimum_required(VERSION 3.16)
project(tst_qshareddatapointer LANGUAGES CXX)
find_package(Qt6BuildInternals REQUIRED COMPONENTS STANDALONE_TEST)
endif()
qt_internal_add_test(tst_qshareddatapointer
SOURCES
tst_qshareddatapointer.cpp
LIBRARIES
Qt::TestPrivate
)

63
tests/auto/corelib/tools/qshareddatapointer/tst_qshareddatapointer.cpp Normal file
View File

@ -0,0 +1,63 @@
// Copyright (C) 2024 The Qt Company Ltd.
// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only
#include <QTest>
#include <QtTest/private/qcomparisontesthelper_p.h>
#include <QtCore/QSharedData>
class tst_QSharedDataPointer : public QObject
{
Q_OBJECT
private slots:
void compareCompiles();
void compare();
};
class MyClass : public QSharedData
{
public:
MyClass(int v) : m_value(v)
{
ref.ref();
};
int m_value;
};
void tst_QSharedDataPointer::compareCompiles()
{
QTestPrivate::testAllComparisonOperatorsCompile<QSharedDataPointer<MyClass>>();
QTestPrivate::testAllComparisonOperatorsCompile<QSharedDataPointer<MyClass>, MyClass*>();
QTestPrivate::testAllComparisonOperatorsCompile<QSharedDataPointer<MyClass>, std::nullptr_t>();
}
void tst_QSharedDataPointer::compare()
{
const QSharedDataPointer<MyClass> ptr;
const QSharedDataPointer<MyClass> ptr2;
QT_TEST_ALL_COMPARISON_OPS(ptr, nullptr, Qt::strong_ordering::equal);
QT_TEST_ALL_COMPARISON_OPS(ptr2, nullptr, Qt::strong_ordering::equal);
QT_TEST_ALL_COMPARISON_OPS(ptr, ptr2, Qt::strong_ordering::equal);
const QSharedDataPointer<MyClass> copy(ptr);
QT_TEST_ALL_COMPARISON_OPS(ptr, copy, Qt::strong_ordering::equal);
const MyClass* pointer = nullptr;
QT_TEST_ALL_COMPARISON_OPS(pointer, ptr, Qt::strong_ordering::equal);
const QSharedDataPointer<MyClass> pointer2(new MyClass(0), QAdoptSharedDataTag{});
QT_TEST_ALL_COMPARISON_OPS(pointer, pointer2, Qt::strong_ordering::less);
std::array<MyClass, 2> myArray {MyClass(1), MyClass(0)};
const QSharedDataPointer<const MyClass> val0(&myArray[0]);
const QSharedDataPointer<const MyClass> val1(&myArray[1]);
QT_TEST_ALL_COMPARISON_OPS(val0, val1, Qt::strong_ordering::less);
QT_TEST_ALL_COMPARISON_OPS(val0, &myArray[1], Qt::strong_ordering::less);
QT_TEST_ALL_COMPARISON_OPS(val1, val0, Qt::strong_ordering::greater);
QT_TEST_ALL_COMPARISON_OPS(&myArray[1], val0, Qt::strong_ordering::greater);
}
QTEST_MAIN(tst_QSharedDataPointer)
#include "tst_qshareddatapointer.moc"