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<QString> 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<QString/QByteArray> 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<QString> 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 <thiago.macieira@intel.com>
(cherry picked from commit 88b1c9623903d54bd64cc92aa2dd61ee4ff4389b)
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
This commit is contained in:
Marc Mutz 2024-05-24 14:48:45 +02:00
parent cff1142732
commit 229ed81c72
2 changed files with 21 additions and 8 deletions

View File

@ -61,10 +61,13 @@ private:
};
template <typename Char>
using if_compatible_char = std::enable_if_t<std::disjunction_v<
using is_compatible_char = std::disjunction<
QtPrivate::IsCompatibleCharType<Char>,
QtPrivate::IsCompatibleChar8Type<Char>
>, bool>;
>;
template <typename Char>
using if_compatible_char = std::enable_if_t<is_compatible_char<Char>::value, bool>;
template <typename Pointer>
using if_compatible_pointer = std::enable_if_t<std::disjunction_v<
@ -86,6 +89,7 @@ private:
std::is_same<q20::remove_cvref_t<T>, QAnyStringView::Tag>,
std::is_same<q20::remove_cvref_t<T>, QAnyStringView>, // don't make a copy/move ctor
std::is_pointer<std::decay_t<T>>, // const char*, etc
is_compatible_char<T>, // don't create a QString/QByteArray, we have a ctor
std::is_same<q20::remove_cvref_t<T>, QByteArray>,
std::is_same<q20::remove_cvref_t<T>, 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<Container, QByteArray> &&capacity = {})
//noexcept(std::is_nothrow_constructible_v<QByteArray, Container>)
: QAnyStringView(capacity = std::forward<Container>(c)) {}
template <typename Char, if_compatible_char<Char> = true>
constexpr QAnyStringView(const Char &c) noexcept
: QAnyStringView{&c, 1} {}
constexpr QAnyStringView(const QChar &c) noexcept
: QAnyStringView{&c, 1} {}
template <typename Char, if_convertible_to<QChar, Char> = true>
constexpr QAnyStringView(Char ch, QCharContainer &&capacity = QCharContainer()) noexcept
: QAnyStringView{&(capacity.ch = ch), 1} {}
template <typename Char, typename Container = decltype(QChar::fromUcs4(U'x')),
std::enable_if_t<std::is_same_v<Char, char32_t>, 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

View File

@ -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) {