From 37e0953613ef9a3db137bc8d3076441d9ae317d9 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Tue, 16 Mar 2021 10:46:16 +0100 Subject: [PATCH 1/2] QHash: add a Qt 7 TODO The hashing seed's type has been changed from int to size_t in Qt 6. However the functions setting/getting the seed, and the seed itself, are still simply int, meaning that we've crippled our seeding. Add a TODO to amend it. Change-Id: Ie9dd177149ec299ccf16d4e31f9f4b065804cfed Reviewed-by: Lars Knoll --- src/corelib/tools/qhash.cpp | 1 + src/corelib/tools/qhashfunctions.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/corelib/tools/qhash.cpp b/src/corelib/tools/qhash.cpp index 933dff84ab..092985887f 100644 --- a/src/corelib/tools/qhash.cpp +++ b/src/corelib/tools/qhash.cpp @@ -740,6 +740,7 @@ static uint qt_create_qhash_seed() /* The QHash seed itself. */ +// ### Qt 7: this should use size_t, not int. static QBasicAtomicInt qt_qhash_seed = Q_BASIC_ATOMIC_INITIALIZER(-1); /*! diff --git a/src/corelib/tools/qhashfunctions.h b/src/corelib/tools/qhashfunctions.h index 25f9c1f1e4..018c317834 100644 --- a/src/corelib/tools/qhashfunctions.h +++ b/src/corelib/tools/qhashfunctions.h @@ -64,6 +64,7 @@ class QByteArray; class QString; class QLatin1String; +// ### Qt 7: these should use size_t, not int. Q_CORE_EXPORT int qGlobalQHashSeed(); Q_CORE_EXPORT void qSetGlobalQHashSeed(int newSeed); From 14f9f00fdb2dc428610c08e3d9d03e38e9602166 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Tue, 16 Mar 2021 10:53:24 +0100 Subject: [PATCH 2/2] QSqlQuery: make it a move only type QSqlQuery is a broken value class. Copying one object would mean copying database state (the result set, the cursor position, etc.) which isn't generally available for all database drivers. For that reason, the current implementation does not honor value semantics -- modifying a QSqlQuery object has visible side effects on its existing copies (!). The correct solution is to accept that QSqlQuery is a move only type, not a value type. Add move semantics to it, and deprecate its copies. (We can't just *remove* copies in Qt 6 due to SC/BC constraints). [ChangeLog][QtSql][QSqlQuery] QSqlQuery copy operations have been deprecated. QSqlQuery copy semantics cannot be implemented correctly, as it's not generally possible to copy a result set of a query when copying the corresponding QSqlQuery object. This resulted in modifications on a QSqlQuery having visible (and unintended) side effects on its copies. Instead, treat QSqlQuery as a move-only type. Fixes: QTBUG-91766 Change-Id: Iabd3aa605332a5c15c524303418bf17a21ed520b Reviewed-by: Edward Welbourne Reviewed-by: Andy Shaw Reviewed-by: Volker Hilsheimer --- src/sql/kernel/qsqlquery.cpp | 54 ++++++++++++++----- src/sql/kernel/qsqlquery.h | 23 +++++++- src/sql/models/qsqlquerymodel.cpp | 31 ++++++++--- src/sql/models/qsqlquerymodel.h | 4 ++ .../qsqlquerymodel/tst_qsqlquerymodel.cpp | 2 +- 5 files changed, 93 insertions(+), 21 deletions(-) diff --git a/src/sql/kernel/qsqlquery.cpp b/src/sql/kernel/qsqlquery.cpp index d51d596d14..b78566e828 100644 --- a/src/sql/kernel/qsqlquery.cpp +++ b/src/sql/kernel/qsqlquery.cpp @@ -243,12 +243,18 @@ QSqlQuery::QSqlQuery(QSqlResult *result) QSqlQuery::~QSqlQuery() { - if (!d->ref.deref()) + if (d && !d->ref.deref()) delete d; } +#if QT_DEPRECATED_SINCE(6, 2) /*! Constructs a copy of \a other. + + \obsolete QSqlQuery cannot be meaningfully copied. Prepared + statements, bound values and so on will not work correctly, depending + on your database driver (for instance, changing the copy will affect + the original). Treat QSqlQuery as a move-only type instead. */ QSqlQuery::QSqlQuery(const QSqlQuery& other) @@ -257,6 +263,41 @@ QSqlQuery::QSqlQuery(const QSqlQuery& other) d->ref.ref(); } +/*! + Assigns \a other to this object. + + \obsolete QSqlQuery cannot be meaningfully copied. Prepared + statements, bound values and so on will not work correctly, depending + on your database driver (for instance, changing the copy will affect + the original). Treat QSqlQuery as a move-only type instead. +*/ + +QSqlQuery& QSqlQuery::operator=(const QSqlQuery& other) +{ + qAtomicAssign(d, other.d); + return *this; +} +#endif + +/*! + \fn QSqlQuery::QSqlQuery(QSqlQuery &&other) noexcept + \since 6.2 + Move-constructs a QSqlQuery from \a other. +*/ + +/*! + \fn QSqlQuery &QSqlQuery::operator=(QSqlQuery &&other) noexcept + \since 6.2 + Move-assigns \a other to this object. +*/ + +/*! + \fn void QSqlQuery::swap(QSqlQuery &other) noexcept + \since 6.2 + Swaps \a other to this object. This operation is very + fast and never fails. +*/ + /*! \internal */ @@ -299,17 +340,6 @@ QSqlQuery::QSqlQuery(const QSqlDatabase &db) qInit(this, QString(), db); } - -/*! - Assigns \a other to this object. -*/ - -QSqlQuery& QSqlQuery::operator=(const QSqlQuery& other) -{ - qAtomicAssign(d, other.d); - return *this; -} - /*! Returns \c true if the query is not \l{isActive()}{active}, the query is not positioned on a valid record, diff --git a/src/sql/kernel/qsqlquery.h b/src/sql/kernel/qsqlquery.h index b270441273..b34452c302 100644 --- a/src/sql/kernel/qsqlquery.h +++ b/src/sql/kernel/qsqlquery.h @@ -61,10 +61,29 @@ public: explicit QSqlQuery(QSqlResult *r); explicit QSqlQuery(const QString& query = QString(), const QSqlDatabase &db = QSqlDatabase()); explicit QSqlQuery(const QSqlDatabase &db); - QSqlQuery(const QSqlQuery& other); - QSqlQuery& operator=(const QSqlQuery& other); + +#if QT_DEPRECATED_SINCE(6, 2) + QT_DEPRECATED_VERSION_X_6_2("QSqlQuery is not meant to be copied. Use move construction instead.") + QSqlQuery(const QSqlQuery &other); + QT_DEPRECATED_VERSION_X_6_2("QSqlQuery is not meant to be copied. Use move assignment instead.") + QSqlQuery& operator=(const QSqlQuery &other); +#else + QSqlQuery(const QSqlQuery &other) = delete; + QSqlQuery& operator=(const QSqlQuery &other) = delete; +#endif + + QSqlQuery(QSqlQuery &&other) noexcept + : d(std::exchange(other.d, nullptr)) + {} + QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QSqlQuery) + ~QSqlQuery(); + void swap(QSqlQuery &other) noexcept + { + qSwap(d, other.d); + } + bool isValid() const; bool isActive() const; bool isNull(int field) const; diff --git a/src/sql/models/qsqlquerymodel.cpp b/src/sql/models/qsqlquerymodel.cpp index 6d4e2c09c1..ceada8cae2 100644 --- a/src/sql/models/qsqlquerymodel.cpp +++ b/src/sql/models/qsqlquerymodel.cpp @@ -418,6 +418,20 @@ void QSqlQueryModel::queryChange() // do nothing } + +/*! + \obsolete + \overload + \since 4.5 + + Use the \c{setQuery(QSqlQuery &&query)} overload instead. +*/ +void QSqlQueryModel::setQuery(const QSqlQuery &query) +{ + QT_IGNORE_DEPRECATIONS(QSqlQuery copy = query;) + setQuery(std::move(copy)); +} + /*! Resets the model and sets the data provider to be the given \a query. Note that the query must be active and must not be @@ -428,9 +442,11 @@ void QSqlQueryModel::queryChange() \note Calling setQuery() will remove any inserted columns. + \since 6.2 + \sa query(), QSqlQuery::isActive(), QSqlQuery::setForwardOnly(), lastError() */ -void QSqlQueryModel::setQuery(const QSqlQuery &query) +void QSqlQueryModel::setQuery(QSqlQuery &&query) { Q_D(QSqlQueryModel); beginResetModel(); @@ -443,11 +459,11 @@ void QSqlQueryModel::setQuery(const QSqlQuery &query) d->bottom = QModelIndex(); d->error = QSqlError(); - d->query = query; + d->query = std::move(query); d->rec = newRec; d->atEnd = true; - if (query.isForwardOnly()) { + if (d->query.isForwardOnly()) { d->error = QSqlError(QLatin1String("Forward-only queries " "cannot be used in a data model"), QString(), QSqlError::ConnectionError); @@ -455,13 +471,13 @@ void QSqlQueryModel::setQuery(const QSqlQuery &query) return; } - if (!query.isActive()) { - d->error = query.lastError(); + if (!d->query.isActive()) { + d->error = d->query.lastError(); endResetModel(); return; } - if (query.driver()->hasFeature(QSqlDriver::QuerySize) && d->query.size() > 0) { + if (d->query.driver()->hasFeature(QSqlDriver::QuerySize) && d->query.size() > 0) { d->bottom = createIndex(d->query.size() - 1, d->rec.count() - 1); } else { d->bottom = createIndex(-1, d->rec.count() - 1); @@ -545,11 +561,14 @@ bool QSqlQueryModel::setHeaderData(int section, Qt::Orientation orientation, \sa setQuery() */ +QT_WARNING_PUSH +QT_WARNING_DISABLE_DEPRECATED QSqlQuery QSqlQueryModel::query() const { Q_D(const QSqlQueryModel); return d->query; } +QT_WARNING_POP /*! Returns information about the last error that occurred on the diff --git a/src/sql/models/qsqlquerymodel.h b/src/sql/models/qsqlquerymodel.h index 427b369ae2..d375093bc6 100644 --- a/src/sql/models/qsqlquerymodel.h +++ b/src/sql/models/qsqlquerymodel.h @@ -76,7 +76,11 @@ public: bool insertColumns(int column, int count, const QModelIndex &parent = QModelIndex()) override; bool removeColumns(int column, int count, const QModelIndex &parent = QModelIndex()) override; +#if QT_DEPRECATED_SINCE(6, 2) + QT_DEPRECATED_VERSION_X_6_2("QSqlQuery is not meant to be copied. Pass it by move instead.") void setQuery(const QSqlQuery &query); +#endif + void setQuery(QSqlQuery &&query); void setQuery(const QString &query, const QSqlDatabase &db = QSqlDatabase()); QSqlQuery query() const; diff --git a/tests/auto/sql/models/qsqlquerymodel/tst_qsqlquerymodel.cpp b/tests/auto/sql/models/qsqlquerymodel/tst_qsqlquerymodel.cpp index f323c8b984..ce65dcf674 100644 --- a/tests/auto/sql/models/qsqlquerymodel/tst_qsqlquerymodel.cpp +++ b/tests/auto/sql/models/qsqlquerymodel/tst_qsqlquerymodel.cpp @@ -573,7 +573,7 @@ void tst_QSqlQueryModel::setQueryWithNoRowsInResultSet() // The query's result set will be empty so no signals should be emitted! QSqlQuery query(db); QVERIFY_SQL(query, exec("SELECT * FROM " + qTableName("test", __FILE__, db) + " where 0 = 1")); - model.setQuery(query); + model.setQuery(std::move(query)); QCOMPARE(modelRowsAboutToBeInsertedSpy.count(), 0); QCOMPARE(modelRowsInsertedSpy.count(), 0); }