[Development] Feature Freeze Exception: QStringView

Lars, Knoll lars.knoll at qt.io
Tue Jan 31 10:08:35 CET 2017


Hi Marc,

The downside of any exceptions to the feature freeze (as we've seen before) is added risk to our schedules. We should use this sparingly (and I've been probably giving exceptions too often in the past).

> On 30 Jan 2017, at 20:03, Marc Mutz <marc.mutz at kdab.com> wrote:
> 
> QStringView is ready to be merged, but the maintainer has asked for another 
> week before he can properly review the class.
> 
> In case you don't remember, here's the discussion thread from 2015: 
> http://lists.qt-project.org/pipermail/development/2015-October/023358.html
> 
> Here's the evolving patch series:
> https://codereview.qt-
> project.org/#/q/status:open+project:qt/qtbase+topic:qstringview,n,z
> 
> I feel the patch addresses all concerns raised in the discussion:
> 
> 1. size_type: we agreed on on ssize_t equivalent to strike a balance between
>   the Qt preference of 32-bit signed types over the STL preference for 64-bit
>   unsigned types.
> 2. Do this in Qt vs. do this in QtCreator. I believe that a QStringView
>   without deeply-routed support in QString would be a still-born. We also
>   don't have time come Qt 6 to port all of Qt to a QStringView developed
>   elsewhere. The current patches just scratch the surface. Everywhere
>   QStringRef is currently used, QStringView can be used. And a lot of places
>   that traditionally only took QString should take QStringView, too, e.g.
>   QFile::encodeName(). So I added it to Qt.
> 3. Keeping existing QString overloads for implicit sharing.
>   The patch does not remove anything. It will, as it progresses, implement
>   QT_STRINGVIEW_LEVELs 2 and 3 which remove existing QString and QStringRef
>   overloads (2: only where the function read from the string, 3: also where
>   it stores the string), so that at some later point in time we can just flip
>   a preprocessor switch and compare executable size and execution speed of
>   both sets of overloads, everything else being equal.
> 
> 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? 

> 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.

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.

Cheers,
Lars




More information about the Development mailing list