From 287234704b57e171c786ba8df0b93c49edb903c5 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 24 Dec 2024 13:34:25 +0100 Subject: [PATCH] QSpan: don't detach Qt containers When converting an implicitly-shared Qt container to QSpan, the QSpan ctor would call .data(), which detaches said Qt container (if it's not const). This is what must happen for mutable spans (QSpan), but is not necessary for non-mutable spans (QSpan). Fix by copying the potential const from QSpan::element_type to the Range object in the resp. QSpan ctor. This makes a QSpan over a const element_type mark the range const before passing it to adl_data() (= member-data()). For a non-const element_type, nothing changes. [ChangeLog][QtCore][QSpan] No longer detaches implicitly-shared Qt containers converted to QSpan. Note that std::span will, however, detach such containers, so we recommend to use std::as_const() with implcitly-shared Qt containers, as always. Fixes: QTBUG-132133 Pick-to: 6.9 Change-Id: I9fdae20994d2c900bc5b45b44db3901d10f8838a Reviewed-by: Thiago Macieira --- src/corelib/tools/qspan.h | 8 ++++++-- src/corelib/tools/qspan.qdoc | 1 + tests/auto/corelib/tools/qspan/tst_qspan.cpp | 2 -- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/corelib/tools/qspan.h b/src/corelib/tools/qspan.h index 6e4bf2dd16c..36a34b56edb 100644 --- a/src/corelib/tools/qspan.h +++ b/src/corelib/tools/qspan.h @@ -44,6 +44,10 @@ QT_END_INCLUDE_NAMESPACE namespace QSpanPrivate { +template +std::conditional_t, const To &, To &> // like [forward]/6.1 COPY_CONST +const_propagated(To &in) { return in; } + template class QSpanBase; template @@ -208,7 +212,7 @@ public: template = true> Q_IMPLICIT constexpr QSpanBase(Range &&r) - : QSpanBase(QSpanPrivate::adl_data(r), // no forward<>() here (std doesn't have it, either) + : QSpanBase(QSpanPrivate::adl_data(QSpanPrivate::const_propagated(r)), // no forward<>() here (std doesn't have it, either) qsizetype(QSpanPrivate::adl_size(r))) // ditto, no forward<>() {} @@ -280,7 +284,7 @@ public: template = true> Q_IMPLICIT constexpr QSpanBase(Range &&r) - : QSpanBase(QSpanPrivate::adl_data(r), // no forward<>() here (std doesn't have it, either) + : QSpanBase(QSpanPrivate::adl_data(QSpanPrivate::const_propagated(r)), // no forward<>() here (std doesn't have it, either) qsizetype(QSpanPrivate::adl_size(r))) // ditto, no forward<>() {} diff --git a/src/corelib/tools/qspan.qdoc b/src/corelib/tools/qspan.qdoc index e7b5fe7dc0a..a2d93cbde68 100644 --- a/src/corelib/tools/qspan.qdoc +++ b/src/corelib/tools/qspan.qdoc @@ -104,6 +104,7 @@ \list \li QSpan is using the signed qsizetype as \c{size_type} whereas \c{std::span} uses \c{size_t}. + \li (since Qt 6.9) \c{QSpan} doesn't detach Qt containers, \c{std::span} does. \li All QSpan constructors are implicit; many \c{std::span} ones are \c{explicit}. \li QSpan can be constructed from rvalue owning containers, \c{std::span} can not. diff --git a/tests/auto/corelib/tools/qspan/tst_qspan.cpp b/tests/auto/corelib/tools/qspan/tst_qspan.cpp index 2d63b0a45a2..fa2b81c8d34 100644 --- a/tests/auto/corelib/tools/qspan/tst_qspan.cpp +++ b/tests/auto/corelib/tools/qspan/tst_qspan.cpp @@ -478,7 +478,6 @@ void tst_QSpan::constQSpansDontDetachQtContainers() const [[maybe_unused]] const QList copy = li; QVERIFY(!li.isDetached()); [[maybe_unused]] QSpan cvspan = li; // should not detach (QTBUG-132133) - QEXPECT_FAIL("", "QTBUG-132133", Continue); QVERIFY(!li.isDetached()); [[maybe_unused]] QSpan mvspan = li; // this _has_ to detach, though QVERIFY(li.isDetached()); @@ -489,7 +488,6 @@ void tst_QSpan::constQSpansDontDetachQtContainers() const [[maybe_unused]] const QList copy = li; QVERIFY(!li.isDetached()); [[maybe_unused]] QSpan cfspan = li; // should not detach (QTBUG-132133) - QEXPECT_FAIL("", "QTBUG-132133", Continue); QVERIFY(!li.isDetached()); [[maybe_unused]] QSpan mfspan = li; // this _has_ to detach, though QVERIFY(li.isDetached());