diff --git a/src/corelib/.prev_CMakeLists.txt b/src/corelib/.prev_CMakeLists.txt index 7a3d45d587..48e6ef5afb 100644 --- a/src/corelib/.prev_CMakeLists.txt +++ b/src/corelib/.prev_CMakeLists.txt @@ -81,7 +81,7 @@ qt_internal_add_module(Core kernel/qelapsedtimer.cpp kernel/qelapsedtimer.h kernel/qeventloop.cpp kernel/qeventloop.h kernel/qfunctions_p.h - kernel/qiterable.cpp kernel/qiterable.h kernel/qiterable_p.cpp kernel/qiterable_p.h + kernel/qiterable.cpp kernel/qiterable.h kernel/qiterable_p.h kernel/qmath.cpp kernel/qmath.h kernel/qmetacontainer.cpp kernel/qmetacontainer.h kernel/qmetaobject.cpp kernel/qmetaobject.h kernel/qmetaobject_p.h diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index e9c934acf4..8a3bc0ab84 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -103,7 +103,7 @@ qt_internal_add_module(Core kernel/qelapsedtimer.cpp kernel/qelapsedtimer.h kernel/qeventloop.cpp kernel/qeventloop.h kernel/qfunctions_p.h - kernel/qiterable.cpp kernel/qiterable.h kernel/qiterable_p.cpp kernel/qiterable_p.h + kernel/qiterable.cpp kernel/qiterable.h kernel/qiterable_p.h kernel/qmath.cpp kernel/qmath.h kernel/qmetacontainer.cpp kernel/qmetacontainer.h kernel/qmetaobject.cpp kernel/qmetaobject.h kernel/qmetaobject_p.h diff --git a/src/corelib/kernel/kernel.pri b/src/corelib/kernel/kernel.pri index 518caa4494..0b6c8cbc76 100644 --- a/src/corelib/kernel/kernel.pri +++ b/src/corelib/kernel/kernel.pri @@ -75,7 +75,6 @@ SOURCES += \ kernel/qvariant.cpp \ kernel/qcoreglobaldata.cpp \ kernel/qiterable.cpp \ - kernel/qiterable_p.cpp \ kernel/qsequentialiterable.cpp \ kernel/qassociativeiterable.cpp \ kernel/qsharedmemory.cpp \ diff --git a/src/corelib/kernel/qassociativeiterable.cpp b/src/corelib/kernel/qassociativeiterable.cpp index 780ae35a42..5e30ae291e 100644 --- a/src/corelib/kernel/qassociativeiterable.cpp +++ b/src/corelib/kernel/qassociativeiterable.cpp @@ -187,40 +187,46 @@ QVariantConstPointer QAssociativeConstIterator::operator->() const /*! Retrieves a const_iterator pointing to the element at the given \a key, or - the end of the container if that key does not exist. + the end of the container if that key does not exist. If the \a key isn't + convertible to the expected type, the end of the container is returned. */ QAssociativeIterable::const_iterator QAssociativeIterable::find(const QVariant &key) const { const QMetaAssociation meta = metaContainer(); - QVariant converted = key; - const void *keyData = QIterablePrivate::coerceType(converted, meta.keyMetaType()); - return const_iterator(QConstIterator( - this, meta.createConstIteratorAtKey(constIterable(), keyData))); + QtPrivate::QVariantTypeCoercer coercer; + if (const void *keyData = coercer.convert(key, meta.keyMetaType())) { + return const_iterator(QConstIterator(this, meta.createConstIteratorAtKey( + constIterable(), keyData))); + } + return constEnd(); } /*! Retrieves an iterator pointing to the element at the given \a key, or - the end of the container if that key does not exist. + the end of the container if that key does not exist. If the \a key isn't + convertible to the expected type, the end of the container is returned. */ QAssociativeIterable::iterator QAssociativeIterable::mutableFind(const QVariant &key) { const QMetaAssociation meta = metaContainer(); - QVariant converted = key; - const void *keyData = QIterablePrivate::coerceType(converted, meta.keyMetaType()); - return iterator(QIterator(this, meta.createIteratorAtKey(mutableIterable(), keyData))); + QtPrivate::QVariantTypeCoercer coercer; + if (const void *keyData = coercer.convert(key, meta.keyMetaType())) + return iterator(QIterator(this, meta.createIteratorAtKey(mutableIterable(), keyData))); + return mutableEnd(); } /*! - Retrieves the mapped value at the given \a key, or an invalid QVariant - if the key does not exist. + Retrieves the mapped value at the given \a key, or a default-constructed + QVariant of the mapped type, if the key does not exist. If the \a key is not + convertible to the key type, the mapped value associated with the + default-constructed key is returned. */ QVariant QAssociativeIterable::value(const QVariant &key) const { const QMetaAssociation meta = metaContainer(); - QVariant converted = key; - const void *keyData = QIterablePrivate::coerceType(converted, meta.keyMetaType()); + QtPrivate::QVariantTypeCoercer coercer; QVariant result(QMetaType(meta.mappedMetaType())); - meta.mappedAtKey(constIterable(), keyData, result.data()); + meta.mappedAtKey(constIterable(), coercer.coerce(key, meta.keyMetaType()), result.data()); return result; } diff --git a/src/corelib/kernel/qassociativeiterable.h b/src/corelib/kernel/qassociativeiterable.h index 8412dc86fd..ba20af0327 100644 --- a/src/corelib/kernel/qassociativeiterable.h +++ b/src/corelib/kernel/qassociativeiterable.h @@ -163,14 +163,15 @@ inline QVariantRef::operator QVariant() const { if (m_pointer == nullptr) return QVariant(); + const auto metaAssociation = m_pointer->metaContainer(); - QMetaType metaType(metaAssociation.mappedMetaType()); + const QMetaType metaType(metaAssociation.mappedMetaType()); if (!metaType.isValid()) return m_pointer->key(); QVariant v(metaType); - void *dataPtr = metaType == QMetaType::fromType() ? &v : v.data(); - metaAssociation.mappedAtIterator(m_pointer->constIterator(), dataPtr); + metaAssociation.mappedAtIterator(m_pointer->constIterator(), + metaType == QMetaType::fromType() ? &v : v.data()); return v; } @@ -180,11 +181,15 @@ inline QVariantRef &QVariantRef::ope { if (m_pointer == nullptr) return *this; - const QMetaType metaType(m_pointer->metaContainer().mappedMetaType()); - const void *dataPtr = metaType == QMetaType::fromType() - ? &value - : value.constData(); - m_pointer->metaContainer().setMappedAtIterator(m_pointer->constIterator(), dataPtr); + + const auto metaAssociation = m_pointer->metaContainer(); + const QMetaType metaType(metaAssociation.mappedMetaType()); + if (metaType.isValid()) { + QtPrivate::QVariantTypeCoercer coercer; + metaAssociation.setMappedAtIterator(m_pointer->constIterator(), + coercer.coerce(value, metaType)); + } + return *this; } diff --git a/src/corelib/kernel/qiterable_p.cpp b/src/corelib/kernel/qiterable_p.cpp deleted file mode 100644 index ea569cebd5..0000000000 --- a/src/corelib/kernel/qiterable_p.cpp +++ /dev/null @@ -1,63 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2020 The Qt Company Ltd. -** Contact: https://www.qt.io/licensing/ -** -** This file is part of the QtCore module of the Qt Toolkit. -** -** $QT_BEGIN_LICENSE:LGPL$ -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and The Qt Company. For licensing terms -** and conditions see https://www.qt.io/terms-conditions. For further -** information use the contact form at https://www.qt.io/contact-us. -** -** GNU Lesser General Public License Usage -** Alternatively, this file may be used under the terms of the GNU Lesser -** General Public License version 3 as published by the Free Software -** Foundation and appearing in the file LICENSE.LGPL3 included in the -** packaging of this file. Please review the following information to -** ensure the GNU Lesser General Public License version 3 requirements -** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. -** -** GNU General Public License Usage -** Alternatively, this file may be used under the terms of the GNU -** General Public License version 2.0 or (at your option) the GNU General -** Public license version 3 or any later version approved by the KDE Free -** Qt Foundation. The licenses are as published by the Free Software -** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 -** included in the packaging of this file. Please review the following -** information to ensure the GNU General Public License requirements will -** be met: https://www.gnu.org/licenses/gpl-2.0.html and -** https://www.gnu.org/licenses/gpl-3.0.html. -** -** $QT_END_LICENSE$ -** -****************************************************************************/ - -#include - -QT_BEGIN_NAMESPACE - -namespace QIterablePrivate { - -const void *coerceType(QVariant &value, const QMetaType &type) -{ - if (type == QMetaType::fromType()) { - return &value; - } else if (type == value.metaType()) { - return value.constData(); - } else if (value.canConvert(type)) { - value.convert(type); - return value.constData(); - } - - value = QVariant(type); - return value.constData(); -} - -} // namespace QIterablePrivate - -QT_END_NAMESPACE diff --git a/src/corelib/kernel/qiterable_p.h b/src/corelib/kernel/qiterable_p.h index 33b190a59a..979ef49b18 100644 --- a/src/corelib/kernel/qiterable_p.h +++ b/src/corelib/kernel/qiterable_p.h @@ -58,8 +58,6 @@ QT_BEGIN_NAMESPACE namespace QIterablePrivate { -const void *coerceType(QVariant &value, const QMetaType &type); - template static QVariant retrieveElement(const QMetaType &type, Callback callback) { diff --git a/src/corelib/kernel/qsequentialiterable.cpp b/src/corelib/kernel/qsequentialiterable.cpp index 6ce41a1edf..054d97d90f 100644 --- a/src/corelib/kernel/qsequentialiterable.cpp +++ b/src/corelib/kernel/qsequentialiterable.cpp @@ -106,8 +106,8 @@ QT_BEGIN_NAMESPACE */ void QSequentialIterable::addValue(const QVariant &value, Position position) { - QVariant converted = value; - const void *valuePtr = QIterablePrivate::coerceType(converted, metaContainer().valueMetaType()); + QtPrivate::QVariantTypeCoercer coercer; + const void *valuePtr = coercer.coerce(value, metaContainer().valueMetaType()); switch (position) { case AtBegin: @@ -181,8 +181,8 @@ QVariant QSequentialIterable::at(qsizetype idx) const */ void QSequentialIterable::set(qsizetype idx, const QVariant &value) { - QVariant converted = value; - const void *dataPtr = QIterablePrivate::coerceType(converted, metaContainer().valueMetaType()); + QtPrivate::QVariantTypeCoercer coercer; + const void *dataPtr = coercer.coerce(value, metaContainer().valueMetaType()); const QMetaSequence meta = metaContainer(); if (meta.canSetValueAtIndex()) { diff --git a/src/corelib/kernel/qsequentialiterable.h b/src/corelib/kernel/qsequentialiterable.h index 5851439316..a5a22b55df 100644 --- a/src/corelib/kernel/qsequentialiterable.h +++ b/src/corelib/kernel/qsequentialiterable.h @@ -172,11 +172,11 @@ inline QVariantRef &QVariantRef::opera { if (m_pointer == nullptr) return *this; - const QMetaType metaType(m_pointer->metaContainer().valueMetaType()); - const void *dataPtr = metaType == QMetaType::fromType() - ? &value - : value.constData(); - m_pointer->metaContainer().setValueAtIterator(m_pointer->constIterator(), dataPtr); + + QtPrivate::QVariantTypeCoercer coercer; + m_pointer->metaContainer().setValueAtIterator( + m_pointer->constIterator(), + coercer.coerce(value, m_pointer->metaContainer().valueMetaType())); return *this; } diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index 09c3eee6ea..849ed6e6ef 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -2728,6 +2728,38 @@ QDebug operator<<(QDebug dbg, const QVariant::Type p) \internal */ +/*! + \internal + */ +const void *QtPrivate::QVariantTypeCoercer::convert(const QVariant &value, const QMetaType &type) +{ + if (type == QMetaType::fromType()) + return &value; + + if (type == value.metaType()) + return value.constData(); + + if (value.canConvert(type)) { + converted = value; + if (converted.convert(type)) + return converted.constData(); + } + + return nullptr; +} + +/*! + \internal + */ +const void *QtPrivate::QVariantTypeCoercer::coerce(const QVariant &value, const QMetaType &type) +{ + if (const void *result = convert(value, type)) + return result; + + converted = QVariant(type); + return converted.constData(); +} + /*! \class QVariantRef \since 6.0 diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h index 40d3d5c3b1..aa74aaa18f 100644 --- a/src/corelib/kernel/qvariant.h +++ b/src/corelib/kernel/qvariant.h @@ -605,6 +605,18 @@ template<> inline QVariant qvariant_cast(const QVariant &v) Q_CORE_EXPORT QDebug operator<<(QDebug, const QVariant::Type); #endif +namespace QtPrivate { +class Q_CORE_EXPORT QVariantTypeCoercer +{ +public: + const void *convert(const QVariant &value, const QMetaType &type); + const void *coerce(const QVariant &value, const QMetaType &type); + +private: + QVariant converted; +}; +} + template class QVariantRef { diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp index 8df3b4055b..4ff404e12a 100644 --- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp +++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp @@ -57,6 +57,8 @@ #include #include "tst_qvariant_common.h" +#include + class CustomNonQObject; class tst_QVariant : public QObject @@ -4150,11 +4152,6 @@ struct KeyGetter > }; -// We have no built-in defines to check the stdlib features. -// #define TEST_UNORDERED_MAP - -#ifdef TEST_UNORDERED_MAP -#include typedef std::unordered_map StdUnorderedMap_int_bool; Q_DECLARE_ASSOCIATIVE_CONTAINER_METATYPE(std::unordered_map) @@ -4173,7 +4170,6 @@ struct KeyGetter > return it->second; } }; -#endif template void sortIterable(QSequentialIterable *iterable) @@ -4290,6 +4286,83 @@ void testSequentialIteration() QCOMPARE(listIter.at(2), second); QCOMPARE(listIter.at(3), second); QCOMPARE(listIter.at(4), third); + + auto i = listIter.mutableBegin(); + QVERIFY(i != listIter.mutableEnd()); + + *i = QStringLiteral("17"); + if (listIter.metaContainer().valueMetaType() == QMetaType::fromType()) + QCOMPARE(listIter.at(0).toInt(), 17); + else if (listIter.metaContainer().valueMetaType() == QMetaType::fromType()) + QCOMPARE(listIter.at(0).toBool(), false); + + *i = QStringLiteral("true"); + if (listIter.metaContainer().valueMetaType() == QMetaType::fromType()) + QCOMPARE(listIter.at(0).toInt(), 0); + else if (listIter.metaContainer().valueMetaType() == QMetaType::fromType()) + QCOMPARE(listIter.at(0).toBool(), true); +} + +template +void testAssociativeIteration() +{ + using Key = typename Container::key_type; + using Mapped = typename Container::mapped_type; + + int numSeen = 0; + Container mapping; + mapping[5] = true; + mapping[15] = false; + + QVariant mappingVariant = QVariant::fromValue(mapping); + QVariantMap varMap = mappingVariant.value(); + QVariantMap varHash = mappingVariant.value(); + QAssociativeIterable mappingIter = mappingVariant.view(); + + typename Container::const_iterator containerIter = mapping.begin(); + const typename Container::const_iterator containerEnd = mapping.end(); + for ( ;containerIter != containerEnd; ++containerIter, ++numSeen) + { + Mapped expected = KeyGetter::value(containerIter); + Key key = KeyGetter::get(containerIter); + Mapped actual = qvariant_cast(mappingIter.value(key)); + QCOMPARE(qvariant_cast(varMap.value(QString::number(key))), expected); + QCOMPARE(qvariant_cast(varHash.value(QString::number(key))), expected); + QCOMPARE(actual, expected); + const QAssociativeIterable::const_iterator it = mappingIter.find(key); + QVERIFY(it != mappingIter.end()); + QCOMPARE(it.value().value(), expected); + } + QCOMPARE(numSeen, (int)std::distance(mapping.begin(), mapping.end())); + QCOMPARE(containerIter, containerEnd); + QVERIFY(mappingIter.find(10) == mappingIter.end()); + + auto i = mappingIter.mutableFind(QStringLiteral("nonono")); + QCOMPARE(i, mappingIter.mutableEnd()); + i = mappingIter.mutableFind(QStringLiteral("5")); + QVERIFY(i != mappingIter.mutableEnd()); + + *i = QStringLiteral("17"); + + if (mappingIter.metaContainer().mappedMetaType() == QMetaType::fromType()) + QCOMPARE(mappingIter.value(5).toInt(), 17); + else if (mappingIter.metaContainer().mappedMetaType() == QMetaType::fromType()) + QCOMPARE(mappingIter.value(5).toBool(), true); + + *i = QStringLiteral("true"); + if (mappingIter.metaContainer().mappedMetaType() == QMetaType::fromType()) + QCOMPARE(mappingIter.value(5).toInt(), 0); + else if (mappingIter.metaContainer().mappedMetaType() == QMetaType::fromType()) + QCOMPARE(mappingIter.value(5).toBool(), true); + + // Test that find() does not coerce + auto container = Container(); + container[0] = true; + + QVariant containerVariant = QVariant::fromValue(container); + QAssociativeIterable iter = containerVariant.value(); + auto f = iter.constFind(QStringLiteral("anything")); + QCOMPARE(f, iter.constEnd()); } void tst_QVariant::iterateContainerElements() @@ -4341,43 +4414,11 @@ void tst_QVariant::iterateContainerElements() QCOMPARE(ints, intsCopy); } -#define TEST_ASSOCIATIVE_ITERATION(CONTAINER, KEY_TYPE, MAPPED_TYPE) \ - { \ - int numSeen = 0; \ - CONTAINER mapping; \ - mapping[5] = true; \ - mapping[15] = false; \ - \ - QVariant mappingVariant = QVariant::fromValue(mapping); \ - QVariantMap varMap = mappingVariant.value(); \ - QVariantMap varHash = mappingVariant.value(); \ - QAssociativeIterable mappingIter = mappingVariant.value(); \ - \ - CONTAINER::const_iterator containerIter = mapping.begin(); \ - const CONTAINER::const_iterator containerEnd = mapping.end(); \ - for ( ; containerIter != containerEnd; ++containerIter, ++numSeen) \ - { \ - MAPPED_TYPE expected = KeyGetter >::value(containerIter); \ - KEY_TYPE key = KeyGetter >::get(containerIter); \ - MAPPED_TYPE actual = mappingIter.value(key).value(); \ - QCOMPARE(varMap.value(QString::number(key)).value(), expected); \ - QCOMPARE(varHash.value(QString::number(key)).value(), expected); \ - QCOMPARE(actual, expected); \ - const QAssociativeIterable::const_iterator it = mappingIter.find(key); \ - QVERIFY(it != mappingIter.end()); \ - QCOMPARE(it.value().value(), expected); \ - } \ - QCOMPARE(numSeen, (int)std::distance(mapping.begin(), mapping.end())); \ - QCOMPARE(containerIter, containerEnd); \ - QVERIFY(mappingIter.find(10) == mappingIter.end()); \ - } - - TEST_ASSOCIATIVE_ITERATION(QHash, int, bool) - TEST_ASSOCIATIVE_ITERATION(QMap, int, bool) - TEST_ASSOCIATIVE_ITERATION(std::map, int, bool) -#ifdef TEST_UNORDERED_MAP - TEST_ASSOCIATIVE_ITERATION(std::unordered_map, int, bool) -#endif + testAssociativeIteration>(); + testAssociativeIteration>(); + testAssociativeIteration>(); + testAssociativeIteration>(); + testAssociativeIteration>(); { QMap mapping;