From 229ed81c728892abcd0732fb6ff630bf95f6cb4b Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 24 May 2024 14:48:45 +0200 Subject: [PATCH] QAnyStringView: fix char-ish ctors to not allocate memory Extending the fromCharacter() tests to check for the construction to be noexcept and implicit turned up a few bugs: The constructor from char32_t was simply not marked as noexcept, even though all its operations were. So just mark it noexcept. The constructor from QChar, otoh, was never called, (certainly not for rvalues), because QString happens to be constructible from QChar (and QLatin1Char and QChar::SpecialCharactor), so due to perfect forwarding, the if_convertible_to ctor won, allocating a QString. This is a regression of Qt 6.5 compared to Qt 6.4. To fix this, exclude arguments that convert to QChar from matching the if_convertible_to ctors, taking care to not match those arguments that are already compatible_char<>s. This, in turn, creates a problem for implicit QASV construction from QLatin1Char and QChar::SpecialCharacter, because now that we've excluded them from the if_convertible_to ctor, calling the existing QChar or QString non-template constructors for these types would require two user-defined conversions (from said type to QChar/QString and from QChar/QString to QAnyStringView). That works for explicit construction, but we need implicit convertability. So bring out the big guns once more and add a perfectly-forwarding ctor for anything convertible to QChar. QChar itself is actually already handled by compatible_char<>, so the old QChar non-template ctor can go. The extra copy of the QChar argument will be optimized away by the compiler. [ChangeLog][QtCore][QAnyStringView] Fixed a regression where constructing a QAnyStringView from a char-like type (QChar, QLatin1Char, ...) would first construct a QString and only then convert that to QAnyStringView. Amends 812a0d3125cb89e340c59aa92cdc946862fb009d. This change is forward and backward BC and SC, but not behavior-compatible: certain operations detectably change noexcept'ness, and some arguments may now cause the resulting QAnyStringView to have a different encoding (though I really tried hard to avoid that). Since it's a regression, I proposed to pick this to the affected branches, 6.7 and 6.5 (6.6 is already closed at this point). Pick-to: 6.5 Fixes: QTBUG-125735 Change-Id: I86f37df5d80bee36db27e529c017cb73995a6831 Reviewed-by: Thiago Macieira (cherry picked from commit 88b1c9623903d54bd64cc92aa2dd61ee4ff4389b) Reviewed-by: Ahmad Samir --- src/corelib/text/qanystringview.h | 21 +++++++++++++------ .../qanystringview/tst_qanystringview.cpp | 8 +++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/corelib/text/qanystringview.h b/src/corelib/text/qanystringview.h index 4a5e80fa18e..e18fb854699 100644 --- a/src/corelib/text/qanystringview.h +++ b/src/corelib/text/qanystringview.h @@ -61,10 +61,13 @@ private: }; template - using if_compatible_char = std::enable_if_t, QtPrivate::IsCompatibleChar8Type - >, bool>; + >; + + template + using if_compatible_char = std::enable_if_t::value, bool>; template using if_compatible_pointer = std::enable_if_t, QAnyStringView::Tag>, std::is_same, QAnyStringView>, // don't make a copy/move ctor std::is_pointer>, // const char*, etc + is_compatible_char, // don't create a QString/QByteArray, we have a ctor std::is_same, QByteArray>, std::is_same, QString> >>, @@ -143,6 +147,11 @@ private: static QChar toQChar(QChar ch) noexcept { return ch; } static QChar toQChar(QLatin1Char ch) noexcept { return ch; } + struct QCharContainer { // private, so users can't pass their own + explicit QCharContainer() = default; + QChar ch; + }; + explicit constexpr QAnyStringView(const void *d, qsizetype n, std::size_t sizeAndType) noexcept : m_data{d}, m_size{std::size_t(n) | (sizeAndType & TypeMask)} {} public: @@ -192,16 +201,16 @@ public: constexpr QAnyStringView(Container &&c, QtPrivate::wrapped_t &&capacity = {}) //noexcept(std::is_nothrow_constructible_v) : QAnyStringView(capacity = std::forward(c)) {} - template = true> constexpr QAnyStringView(const Char &c) noexcept : QAnyStringView{&c, 1} {} - constexpr QAnyStringView(const QChar &c) noexcept - : QAnyStringView{&c, 1} {} + template = true> + constexpr QAnyStringView(Char ch, QCharContainer &&capacity = QCharContainer()) noexcept + : QAnyStringView{&(capacity.ch = ch), 1} {} template , bool> = true> - constexpr QAnyStringView(Char c, Container &&capacity = {}) + constexpr QAnyStringView(Char c, Container &&capacity = {}) noexcept : QAnyStringView(capacity = QChar::fromUcs4(c)) {} constexpr QAnyStringView(QStringView v) noexcept diff --git a/tests/auto/corelib/text/qanystringview/tst_qanystringview.cpp b/tests/auto/corelib/text/qanystringview/tst_qanystringview.cpp index a6a1f0499ea..0f3567eb4b9 100644 --- a/tests/auto/corelib/text/qanystringview/tst_qanystringview.cpp +++ b/tests/auto/corelib/text/qanystringview/tst_qanystringview.cpp @@ -702,13 +702,17 @@ void tst_QAnyStringView::fromCharacter(Char arg, qsizetype expectedSize) const // Need to re-create a new QASV(arg) each time, QASV(Char).data() dangles // after the end of the Full Expression: + static_assert(noexcept(QAnyStringView(arg)), + "If this fails, we may be creating a temporary QString/QByteArray"); + QCOMPARE(QAnyStringView(arg).size(), expectedSize); // QCOMPARE(QAnyStringView(arg), arg); // not all pairs compile, so do it manually: + // Check implicit conversion: const QChar chars[] = { - QAnyStringView(arg).front(), - QAnyStringView(arg).back(), + [](QAnyStringView v) { return v.front(); }(arg), + [](QAnyStringView v) { return v.back(); }(arg), }; switch (expectedSize) {