On Wed, 4 Dec 2019 at 13:05, Thiago Macieira <thiago.macie...@intel.com> wrote: > > IMHO, it is legit to write code like: > > const QUrl url(urlString); > > const auto appId = url.host() + "." + url.path().mid(1); > > doSomeThing(appId); // void doSomeThing(const QString &stuff); > > > > Yet it results in crashes, apparently because of QStringBuilder > > holding references to temporary implicitly shared Qt object. I would > > expect QStringBuilder to somehow grab a reference to this implicitely > > shared object (eg. by incrementing a ref counter). > > We'd lose performance on the common case by doing atomic reference counting up > and down. QStringBuilder would need to hold (shallow, implicitly-shared) > copies of the QString values returned by QUrl::host() and QString::mid(). One > can argue that the cost is reasonable and worth the extra safety it gives.
My point is that this sort of bugs are really hard to detect during code review. This is subtle & nasty 'feature' of QStringBuilder (a choice in its implementation). QStringBuilder has no documentation (that i could find), and there are no 'BIG WARNING' mention in https://doc.qt.io/qt-5/qstring.html#more-efficient-string-construction BTW, Clazy has a check for that very specific case, which just show how dangerous is this QStringBuilder. https://github.com/KDE/clazy/blob/master/docs/checks/README-auto-unexpected-qstringbuilder.md > > AFAIU, QT_USE_QSTRINGBUILDER is a dangerous build option, don't use it. > > It's optional. optional and dangerous are orthogonal concepts! :) Chris _______________________________________________ Interest mailing list Interest@qt-project.org https://lists.qt-project.org/listinfo/interest
