Hi Lars, On Tuesday 31 January 2017 10:08:35 Lars, Knoll wrote: [...] > > So, in order to give the maintainers more time for review, I'd like to > > ask for a feature freeze extension for QStringView until end of next > > week, plus a few days to deal with maintainer review comments. > > I understand that you want to have the patch series finally merged. > Actually I do as well, I believe that QStringView is the right approach > moving forward. > > But I do wonder about the approach here. The series has been around pretty > much unchanged for quite some time now without anybody working on it. You > had lots of time since summer to get this into shape an merged. Now, a > week before the feature freeze and suddenly this absolutely has to go in?
I have been sxcheduled to work on this in January, but was out with sick child and then myself afterwards for almost two weeks. That's why it's late. Then again, as you say, the main patch has been around for the better part of a year, and the basic idea has been around since the beginning. > > Why should it be granted? Because QStringView is by far the biggest > > revolution in Qt since 5.0, all the while integrating naturally and > > step-by-step into existing code, _tremendously_ simplifying it along the > > way (cf. the commits that start to make use of QStringView: > > https://codereview.qt- > > project.org/183864 https://codereview.qt-project.org/183925), esp. where, > > in private API, we can already remove the other overloads. > > I agree, that the series cleans things up and is a great improvement. But > the biggest revolution since 5.0? It's an API addition that brings > performance optimisations in Qt's string handling. No, this is not at all about performance. It's mostly about stream-lining the API, and adding flexibility: 1. Traditionally, a lot of stuff has been taking QString, and QString only. If you're lucky, you get (QChar*, int) on top, so you can use an automatic buffer to hold the string data, with all the ugliness at the call site that entails. QStringView takes all that pain and hides it (see https://codereview.qt- project.org/#/c/183925/2/src/corelib/tools/qtimezoneprivate_win.cpp,unified). It also allows for clearer interfaces, because instead of a QString and an int, you can just pass a QStringView, since creating a sub-view suddenly costs nothing: 2. QStringView gives the caller the choice of container for string data. This makes Qt much more interoperable with 3rd-party libraries. QStringView will make QStringLiteral all but obsolete, since you can now just pass u"string" or, on Windows, L"string" to any QStringView-taking function. We can trivially define a QStringViewLiteral that uses one of the other, depending on platform. I backed that feature out, though, since I fist need to verify that we support no compilers anymore that would fall back to fromUtf8() for QStringLiteral. 3. And yes, performance. Where today we have to return QString, even where we know a maximum size, and even where we don't: It will allow QLocale::toString() to return something equivalent to char16_t[N], e.g. std::array, in order to not allocate memory just to format numbers, something that has so far eluded any practical solution. For internal functions that return temporary QStrings, we can switch to QVarLengthArray, and avoid heap allocations in common cases. The last such fundamental change to QtBase was the possibility to connect signals to lambdas. Believe me, QStringView has the same caliber. > What would we really loose if we worked towards getting this into the dev > branch in the next few weeks? We don't exactly have hundreds of > users/customers who can't live without it. And pushing it into dev would > give you lots of time to make sure we make best possible use of the class > throughout all of Qt for 5.10. I have found that even with just relational operators, conversions, and iterators (ie. just the Long-live commit in the series), maybe QString::append() and QStirngBuilder integration, you can already do a lot of stuff. For us, getting QStringView into dev next week means one week of delay, but for our users, it means half a year delay. Qt is desperately in need of better integration with std C++, and QStringView is hand-down the largest such contribution. Not having it means people will continue to spend^Wwaste time porting stuff to QStringRef, which will later be replaced by QStringView. Don#t get me wrong, I love the work Anton and others have done in the area. I now just need to grep for QStringRef to find code to port to QStringView, but the real value of QStringView lies in where there was no QStringRef-overload in the first place, e.g. QColor: https://codereview.qt-project.org/184038 In the end, it's your call, of course. I personally don't care whether it goes into 5.9 or 5.10, but for our users and for other contributors, I hope that it makes it into 5.9, since, judging from 5.8, a lot of internal performance improvements come into stable branches, and we really don't need more QStringRef uses in Qt. esp. in public API, where we need to carry it all the way to Qt 6. Thanks, Marc -- Marc Mutz <[email protected]> | Senior Software Engineer KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company Tel: +49-30-521325470 KDAB - The Qt, C++ and OpenGL Experts _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
