From 6bc227a06a0d1392d220aa79ddb1cdc145d4f76e Mon Sep 17 00:00:00 2001 From: Sona Kurazyan Date: Tue, 28 Jun 2022 10:01:53 +0200 Subject: [PATCH] Port QXmlStremReader to QAnyStringView Port the constructor and addData() method to QAnyStringView, but keep the overloads taking a QByteArray to avoid extra copies when actual QByteArray is passed. These overlaods need to be Q_WEAK_OVERLOADs, to avoid ambiguities (e.g. for const char * arguments). Additionally, add a test to make sure the patch doesn't break parsing from a QLatin1StringView input. [ChangeLog][QtCore][QXmlStremReader] Added constructor and addData() overloads taking QAnyStringView. Change-Id: I0efaab82a2123271c88407e380f3c67d1099a4a6 Reviewed-by: Edward Welbourne --- src/corelib/compat/removed_api.cpp | 35 ++++ src/corelib/serialization/qxmlstream.cpp | 150 +++++++++++------- src/corelib/serialization/qxmlstream.h | 15 ++ .../qxmlstream/tst_qxmlstream.cpp | 22 +++ 4 files changed, 161 insertions(+), 61 deletions(-) diff --git a/src/corelib/compat/removed_api.cpp b/src/corelib/compat/removed_api.cpp index d56ea9f43a..ba844715d8 100644 --- a/src/corelib/compat/removed_api.cpp +++ b/src/corelib/compat/removed_api.cpp @@ -278,3 +278,38 @@ QT_WARNING_POP // order sections alphabetically to reduce chances of merge conflicts #endif // QT_CORE_REMOVED_SINCE(6, 4) + +#if QT_CORE_REMOVED_SINCE(6, 5) + +#include "qxmlstream.h" + +QXmlStreamReader::QXmlStreamReader(const QByteArray &data) + : QXmlStreamReader(data, PrivateConsructorTag{}) +{ +} + +QXmlStreamReader::QXmlStreamReader(const QString &data) + : QXmlStreamReader(qToAnyStringViewIgnoringNull(data)) +{ +} + +QXmlStreamReader::QXmlStreamReader(const char *data) + : QXmlStreamReader(QAnyStringView(data)) +{ +} + +void QXmlStreamReader::addData(const QByteArray &data) +{ + addData<>(data); +} +void QXmlStreamReader::addData(const QString &data) +{ + addData(qToAnyStringViewIgnoringNull(data)); +} + +void QXmlStreamReader::addData(const char *data) +{ + addData(QAnyStringView(data)); +} + +#endif // QT_CORE_REMOVED_SINCE(6, 5) diff --git a/src/corelib/serialization/qxmlstream.cpp b/src/corelib/serialization/qxmlstream.cpp index a6a2bc41af..28e927d7da 100644 --- a/src/corelib/serialization/qxmlstream.cpp +++ b/src/corelib/serialization/qxmlstream.cpp @@ -360,43 +360,57 @@ QXmlStreamReader::QXmlStreamReader(QIODevice *device) } /*! - Creates a new stream reader that reads from \a data. + \overload - \sa addData(), clear(), setDevice() - */ -QXmlStreamReader::QXmlStreamReader(const QByteArray &data) + \fn QXmlStreamReader::QXmlStreamReader(const QByteArray &data) + + Creates a new stream reader that reads from \a data. + + \sa addData(), clear(), setDevice() +*/ + +/*! + Creates a new stream reader that reads from \a data. + + \note In Qt versions prior to 6.5, this constructor was overloaded + for QString and \c {const char*}. + + \sa addData(), clear(), setDevice() +*/ +QXmlStreamReader::QXmlStreamReader(QAnyStringView data) + : d_ptr(new QXmlStreamReaderPrivate(this)) +{ + Q_D(QXmlStreamReader); + data.visit([d](auto data) { + if constexpr (std::is_same_v) { + d->dataBuffer = data.toUtf8(); + d->decoder = QStringDecoder(QStringDecoder::Utf8); + d->lockEncoding = true; + } else if constexpr (std::is_same_v) { + // Conversion to a QString is required, to avoid breaking + // pre-existing (before porting to QAnyStringView) behavior. + d->dataBuffer = QString::fromLatin1(data).toUtf8(); + d->decoder = QStringDecoder(QStringDecoder::Utf8); + d->lockEncoding = true; + } else { + d->dataBuffer = QByteArray(data.data(), data.size()); + } + }); +} + +/*! + \internal + + Creates a new stream reader that reads from \a data. + Used by the weak constructor taking a QByteArray. +*/ +QXmlStreamReader::QXmlStreamReader(const QByteArray &data, PrivateConsructorTag) : d_ptr(new QXmlStreamReaderPrivate(this)) { Q_D(QXmlStreamReader); d->dataBuffer = data; } -/*! - Creates a new stream reader that reads from \a data. - - \sa addData(), clear(), setDevice() - */ -QXmlStreamReader::QXmlStreamReader(const QString &data) - : d_ptr(new QXmlStreamReaderPrivate(this)) -{ - Q_D(QXmlStreamReader); - d->dataBuffer = data.toUtf8(); - d->decoder = QStringDecoder(QStringDecoder::Utf8); - d->lockEncoding = true; -} - -/*! - Creates a new stream reader that reads from \a data. - - \sa addData(), clear(), setDevice() - */ -QXmlStreamReader::QXmlStreamReader(const char *data) - : d_ptr(new QXmlStreamReaderPrivate(this)) -{ - Q_D(QXmlStreamReader); - d->dataBuffer = QByteArray(data); -} - /*! Destructs the reader. */ @@ -443,14 +457,54 @@ QIODevice *QXmlStreamReader::device() const return d->device; } +/*! + \overload + + \fn void QXmlStreamReader::addData(const QByteArray &data) + + Adds more \a data for the reader to read. This function does + nothing if the reader has a device(). + + \sa readNext(), clear() +*/ /*! - Adds more \a data for the reader to read. This function does - nothing if the reader has a device(). + Adds more \a data for the reader to read. This function does + nothing if the reader has a device(). - \sa readNext(), clear() - */ -void QXmlStreamReader::addData(const QByteArray &data) + \note In Qt versions prior to 6.5, this function was overloaded + for QString and \c {const char*}. + + \sa readNext(), clear() +*/ +void QXmlStreamReader::addData(QAnyStringView data) +{ + Q_D(QXmlStreamReader); + data.visit([=](auto data) { + if constexpr (std::is_same_v) { + d->lockEncoding = true; + if (!d->decoder.isValid()) + d->decoder = QStringDecoder(QStringDecoder::Utf8); + addDataImpl(data.toUtf8()); + } else if constexpr (std::is_same_v) { + // Conversion to a QString is required, to avoid breaking + // pre-existing (before porting to QAnyStringView) behavior. + if (!d->decoder.isValid()) + d->decoder = QStringDecoder(QStringDecoder::Utf8); + addDataImpl(QString::fromLatin1(data).toUtf8()); + } else { + addDataImpl(QByteArray(data.data(), data.size())); + } + }); +} + +/*! + \internal + + Adds more \a data for the reader to read. This function does + nothing if the reader has a device(). +*/ +void QXmlStreamReader::addDataImpl(const QByteArray &data) { Q_D(QXmlStreamReader); if (d->device) { @@ -460,32 +514,6 @@ void QXmlStreamReader::addData(const QByteArray &data) d->dataBuffer += data; } -/*! - Adds more \a data for the reader to read. This function does - nothing if the reader has a device(). - - \sa readNext(), clear() - */ -void QXmlStreamReader::addData(const QString &data) -{ - Q_D(QXmlStreamReader); - d->lockEncoding = true; - if (!d->decoder.isValid()) - d->decoder = QStringDecoder(QStringDecoder::Utf8); - addData(data.toUtf8()); -} - -/*! - Adds more \a data for the reader to read. This function does - nothing if the reader has a device(). - - \sa readNext(), clear() - */ -void QXmlStreamReader::addData(const char *data) -{ - addData(QByteArray(data)); -} - /*! Removes any device() or data from the reader and resets its internal state to the initial state. diff --git a/src/corelib/serialization/qxmlstream.h b/src/corelib/serialization/qxmlstream.h index 366d357b37..8ba24f430e 100644 --- a/src/corelib/serialization/qxmlstream.h +++ b/src/corelib/serialization/qxmlstream.h @@ -197,16 +197,27 @@ public: QXmlStreamReader(); explicit QXmlStreamReader(QIODevice *device); +#if QT_CORE_REMOVED_SINCE(6, 5) explicit QXmlStreamReader(const QByteArray &data); explicit QXmlStreamReader(const QString &data); explicit QXmlStreamReader(const char * data); +#endif // QT_CORE_REMOVED_SINCE(6, 5) + Q_WEAK_OVERLOAD + explicit QXmlStreamReader(const QByteArray &data) + : QXmlStreamReader(data, PrivateConsructorTag{}) { } + explicit QXmlStreamReader(QAnyStringView data); ~QXmlStreamReader(); void setDevice(QIODevice *device); QIODevice *device() const; +#if QT_CORE_REMOVED_SINCE(6, 5) void addData(const QByteArray &data); void addData(const QString &data); void addData(const char *data); +#endif // QT_CORE_REMOVED_SINCE(6, 5) + Q_WEAK_OVERLOAD + void addData(const QByteArray &data) { addDataImpl(data); } + void addData(QAnyStringView data); void clear(); @@ -293,6 +304,10 @@ public: QXmlStreamEntityResolver *entityResolver() const; private: + struct PrivateConsructorTag { }; + QXmlStreamReader(const QByteArray &data, PrivateConsructorTag); + void addDataImpl(const QByteArray &data); + Q_DISABLE_COPY(QXmlStreamReader) Q_DECLARE_PRIVATE(QXmlStreamReader) QScopedPointer d_ptr; diff --git a/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp b/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp index 2799e7a999..3cad03641b 100644 --- a/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp +++ b/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp @@ -16,6 +16,8 @@ #include "qc14n.h" +using namespace Qt::StringLiterals; + Q_DECLARE_METATYPE(QXmlStreamReader::ReadElementTextBehaviour) static const char *const catalogFile = "XML-Test-Suite/xmlconf/finalCatalog.xml"; @@ -558,6 +560,7 @@ private slots: void setEntityResolver(); void readFromQBuffer() const; void readFromQBufferInvalid() const; + void readFromLatin1String() const; void readNextStartElement() const; void readElementText() const; void readElementText_data() const; @@ -1099,6 +1102,25 @@ void tst_QXmlStream::readFromQBufferInvalid() const QVERIFY(reader.hasError()); } +void tst_QXmlStream::readFromLatin1String() const +{ + const auto in = "M\xE5rten"_L1; + { + QXmlStreamReader reader(in); + QVERIFY(reader.readNextStartElement()); + QString text = reader.readElementText(); + QCOMPARE(text, "M\xE5rten"_L1); + } + // Same as above, but with addData() + { + QXmlStreamReader reader; + reader.addData(in); + QVERIFY(reader.readNextStartElement()); + QString text = reader.readElementText(); + QCOMPARE(text, "M\xE5rten"_L1); + } +} + void tst_QXmlStream::readNextStartElement() const { QLatin1String in("text");