[Development] Feature Freeze Exception: QStringView

[Marc Mutz]

On Tuesday 31 January 2017 16:18:59 Thiago Macieira wrote:
> Em terça-feira, 31 de janeiro de 2017, às 12:07:52 PST, Marc Mutz escreveu:
> > 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,u
> >    nified)
> > 
> > . 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:
>
> But we still have to have QString overloads in public functions, otherwise
> creating full copies would now suddenly cost something.

Using QStringView level 3 we'll be able to fact-check that. And I was under 
the impression that Q6String would use SSO...?
 
> > 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.
> 
> There's an interesting advanage for using QStringView on data alloated by
> CoreFramework / Cocoa or by Win32, but since we don't own the data, we have
> to be very careful. In fact, unlike everything else in Qt, QStringView
> never owns the data and that is a major change in how we think.
> 
> It was the cause of Thierry's bug yesterday
> https://codereview.qt-project.org/184004

This is a bug caused by unclear ownership semantics of QByteArray. QStringView 
has clear ownership semantics, so bugs are much less likely. Yes, you have 
problem with stability of references, but you cannot avoid that. Even in Java, 
an int into a container has the problem that the int may get out with the 
resp. index in the container...

> > 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.
> 
> I'll study this when the time comes, but first reaction to:
> 
> 	using NumberString = char16_t[32];
> 	NumberString toString(int value, bool *ok = nullptr);
> 
> Sounds like an awfully ugly API and prone to errors.

This does not compile. I was thinking about a QFormattedNumber<T>, which would 
contain a buffer of sufficient size to hold the result, and implicit 
converstions to QString, QStringView.

> > The last such fundamental change to QtBase was the possibility to connect
> > signals to lambdas. Believe me, QStringView has the same caliber.
> 
> I don't doubt it. Which is why I need more time to review this.
> 
> > 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.
> 
> Sorry, but I have yet to see any codebase using std::u16string, so I really
> doubt that QStringView allows us better integration with standard library-
> based C++.

u"string". The only reason we don't use that, yet, is that MSVC < 2015 doesn't 
support it. Let's drop 2013 in 5.10 and we can replace all QStringLiterals 
with char16_t string literals.

> > 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
> 
> Indeed, and it's unfortunate.

I reviewed additions to public (QtBase) API between 5.8 and 5.9 and the only 
problem is the addition of QStringRef::isRightToLeft() as an out-of-line 
function. I expect that I'll de-out-line most QString/QStringRef functions 
that become backed by a QStringView implementation, qstring_compat.cpp-style, 
and since we won't have QStringView in 5.9, there's another out-of-line 
function to deal with. But what's one in dozens...?

> > 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.
> 
> Our users have used QString for 25 years and QStringRef for 10, so we
> another 6 months is not too big a deal.

But we only have had C++11 for five years...

It's not a problem, but an inconvenience. I don't know when Qt6 will come 
around, but by then we should have all of QtBase equipped with QStringView 
overloads to be able to judge which level to keep in Qt 6. If we try to do the 
remaining overloads in the frenzy leading to 6.0.0, we'll fail to achieve that 
goal.

But since it's decided to postpone to 5.10 anyway, let's at least make sure we 
start to merge the series asap after the final dev→5.9 downmerge.

Thanks,
Marc

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts