diff options
author | Giuseppe D'Angelo <giuseppe.dangelo@kdab.com> | 2019-05-19 14:35:52 +0200 |
---|---|---|
committer | Giuseppe D'Angelo <giuseppe.dangelo@kdab.com> | 2019-05-19 16:33:56 +0200 |
commit | c2d2757bccc68e1b981df059786c2e76f2969530 (patch) | |
tree | 8f1f9fd3c82500c0366e6ac1f1043db86cfc1101 /src/corelib/tools/qbytearray.h | |
parent | c9b7cc349a13b722ecd636ec4eb8e21f9f712add (diff) |
QString/QByteArray: detach immediately in operator[]
Unlike any other implicitly shared container, QString/QByteArray
have a "lazy detach" mechanism: their operator[] returns a
special object; assignment into that object will actually
detach.
In other words:
QString a("Hello");
QCharRef c = a[0]; // does not detach
c = 'J'; // detach happens here
This allows this behavior:
QString a("Hello");
QCharRef c = a[0];
QString b = a;
c = 'J'; // detach happens here
assert(a == "Jello");
assert(b == "Hello");
Note that this happens only with operator[] -- the mutating
iterator APIs instead detach immediately, making the above code
have visible side effects in b (at the end, b == "Jello").
The reasons for this special behavior seems to have been lost in
the dawn of time: this is something present all the way back
since Qt 2, maybe even Qt 1. Holding on to a "reference" while
taking copies of a container is documented [1] to be a bad idea,
so we shouldn't double check that the users don't do it.
This patch:
1) adds an immediate detach in operator[], just like all other
containers;
2) adds a warning in debug builds in case QByteRef/QCharRef is going
to cause a detach;
3) marks operator[] as [[nodiscard]] to warn users not using
Clazy about the (unintended) detach now happening in their code.
This paves the way for removal of QCharRef/QByteRef, likely in
Qt 7.
[1] https://doc.qt.io/qt-5/containers.html#implicit-sharing-iterator-problem
[ChangeLog][QtCore][QString] QString::operator[] detaches
immediately. Previously, the detach was delayed until a
modification was made to the string through the returned
QCharRef.
[ChangeLog][QtCore][QByteArray] QByteArray::operator[] detaches
immediately. Previously, the detach was delayed until a
modification was made to the byte array through the returned
QByteRef.
Change-Id: I9f77ae36759d80dc3202426a798f5b1e5fb2c2c5
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src/corelib/tools/qbytearray.h')
-rw-r--r-- | src/corelib/tools/qbytearray.h | 23 |
1 files changed, 16 insertions, 7 deletions
diff --git a/src/corelib/tools/qbytearray.h b/src/corelib/tools/qbytearray.h index 5c7229e40d..a81051d309 100644 --- a/src/corelib/tools/qbytearray.h +++ b/src/corelib/tools/qbytearray.h @@ -209,8 +209,8 @@ public: inline char at(int i) const; inline char operator[](int i) const; inline char operator[](uint i) const; - inline QByteRef operator[](int i); - inline QByteRef operator[](uint i); + Q_REQUIRED_RESULT inline QByteRef operator[](int i); + Q_REQUIRED_RESULT inline QByteRef operator[](uint i); Q_REQUIRED_RESULT char front() const { return at(0); } Q_REQUIRED_RESULT inline QByteRef front(); Q_REQUIRED_RESULT char back() const { return at(size() - 1); } @@ -535,7 +535,12 @@ namespace DeprecatedRefClassBehavior { QCharRef, }; - Q_CORE_EXPORT Q_DECL_COLD_FUNCTION void warn(EmittingClass c); + enum class WarningType { + OutOfRange, + DelayedDetach, + }; + + Q_CORE_EXPORT Q_DECL_COLD_FUNCTION void warn(WarningType w, EmittingClass c); } // namespace DeprecatedAssignmentOperatorBehavior } // namespace QtPrivate @@ -556,7 +561,7 @@ public: if (Q_LIKELY(i < a.d->size)) return a.d->data()[i]; #ifdef QT_DEBUG - warn(EmittingClass::QByteRef); + warn(WarningType::OutOfRange, EmittingClass::QByteRef); #endif return char(0); } @@ -565,10 +570,14 @@ public: using namespace QtPrivate::DeprecatedRefClassBehavior; if (Q_UNLIKELY(i >= a.d->size)) { #ifdef QT_DEBUG - warn(EmittingClass::QByteRef); + warn(WarningType::OutOfRange, EmittingClass::QByteRef); #endif a.expand(i); } else { +#ifdef QT_DEBUG + if (Q_UNLIKELY(!a.isDetached())) + warn(WarningType::DelayedDetach, EmittingClass::QByteRef); +#endif a.detach(); } a.d->data()[i] = c; @@ -593,9 +602,9 @@ public: }; inline QByteRef QByteArray::operator[](int i) -{ Q_ASSERT(i >= 0); return QByteRef(*this, i); } +{ Q_ASSERT(i >= 0); detach(); return QByteRef(*this, i); } inline QByteRef QByteArray::operator[](uint i) -{ return QByteRef(*this, i); } +{ detach(); return QByteRef(*this, i); } inline QByteRef QByteArray::front() { return operator[](0); } inline QByteRef QByteArray::back() { return operator[](size() - 1); } inline QByteArray::iterator QByteArray::begin() |