[Development] QAnyStringView

[Marc Mutz]

Hi Thiago,

On 2020-06-24 02:36, Thiago Macieira wrote:
> On Tuesday, 23 June 2020 02:35:05 PDT Marc Mutz via Development wrote:
>> I have come to believe that QUtf8StringView without QAnyStringView 
>> won't
>> fly: Introducing QUtf8StringView without QAnyStringView will explode 
>> the
>> number of mixed-type operations we need to support.
> 
> Question, what are the "mixed-typed operations we need to support?". 
> Where do
> you see the need for this?

QString::replace(), relational operators, QXmlStream has several 
combinatorially-explosive overload sets, too.

>> The best we can do to condense this down is
>> to revoke string-ness of QByteArray and we'd be left with
>> 
>> - QStringView
>> - QLatin1String
>> - QUtf8StringView
>> - QChar
> 
> Aside from places where an exception is worth it, our string API 
> should:
> - take QString by const-ref
> - return QString by value
> 
> That condenses our four types to one for almost the entirety of Qt.

In times of QtMCU, we need to re-think whether owning containers use in 
APIs is really the way to go. In C, strings are const char*, which is a 
view. In Qt 3, the objectName ctor argument was a const char*. Things 
have gone downhill since then for no good reason (ie. no functionality 
was added), and a test like tst_qstatemachine adding 10KiB in text size 
for O(150) setObjectName() calls' forced creation of QStrings temps is 
just premature pessimisation at its best.

My Qt 5-era changes to QLatin1String (adding QL1S::arg(), enabling QL1S 
as a type for date/time formats, overloading QtJSON functions for QL1S) 
have shown how dramatic the effect of 8-bit string values without 
constructing a QString first really is, without actually limiting 
functionality.

QStringView, QStringTokenizer, QUtf8StringView and QAnyStringView are 
the result of a multi-year analysis of string processing with Qt. I'm 
fully convinced they're the way to go for ease of use and performance at 
the same time. There's one and only one technical reason to continue to 
use QString in APIs: lack of implementation bandwidth.

> For new API that benefits from the exception, I'd reduce to two:
> - QStringView
> - QUtf8StringView
> 
> But the fact that you listed QChar in the first place indicates that 
> you're
> talking about the string classes themselves. Nothing else uses QChar in 
> our
> API. In that case, yes, QLatin1String and QChar are part of the 
> overload set.
[...]

I guess you consider QStringList::join() a string class, then. But 
otherwise, yes, I agree.

>> Assuming for the sake of argument that we need those four types,
>> consider QString::replace(). Experience shows that stuff like
>> QStringBuilder expressions being passed will require an actual QString
>> overload to be present, too. Ignoring existing overloads and regexp,
>> we'd need 5x5=25 overloads. I won't enumerate them here. What I will
>> enumerate is the complete set of overloads when using QAnyStringView:
>> 
>>     QString& QString::replace(QAnyStringView, QAnyStringView,
>> Qt::CaseSensitivity);
>> 
>> That's it.
>> 
>> Unlike QStringView, QAnyStringView is a pure interface type. I won't 
>> add
>> much in the way of parsing API to it, even though I acknowledge that's 
>> a
>> slippery slope. While it would be easy to add trimmed(), and 
>> tokenize()
>> would be really interesting, QAnyStringView should not be used for
>> parsing. That's what we have the three non-variant string view types
>> for. Being a pure interface type means we can add more "dangerous"
>> conversions. QStringView can't be constructed from a QStringBuilder,
>> e.g., because it's almost impossible to make that work without
>> referencing destroyed data:
>> 
>>     QStringView s = u'c' + QString::number(x); // oops
>>     QString c = u'c' + QString::number(x);
>>     QStringView s = c; // ok
>> 
>> But QAnyStringView supports this:
>> 
>>     str.replace(name, name % "_1");
> 
> That's not the same code. In one you're creating a view object and 
> accessing
> it later outside of the same statement; in the other, it is created and
> accessed in the same statement. That is to say, the following works:
> 
>   void foo(QStringView str);
>   foo(u'c' + QString::number(x));
> 
> and the following doesn't:
> 
>  QAnyStringView s = u'c' + QString::number(x);

That's what I've been trying to say: since Q(Utf8)StringView is very 
good at parsing, QAnyStringView is very good at being an interface type 
and an interface type only. As such, we can allow ourselves some leeway 
in what (implicit) conversions we add to QAnyStringView that we actively 
rejected for QStringView.

>> QAnyStringView solves this in the sense that one overload can replace
>> many overloads. The complexity is still there, a binary visitation of 
>> a
>> QAnyStringView produces nine instantiations of the visitor (though 
>> that
>> can be reduced to six in many cases), but many implementations fall 
>> into
>> one of just two classes: 1) a function would just call toString() on 
>> the
>> any-string-view, anyway, in which case the QString construction is 
>> taken
>> out of user code and centralized in the library. If you think that
>> doesn't matter, look at the tst_qstatemachine numbers in
>> 
>>    https://codereview.qt-project.org/c/qt/qtbase/+/301595 (-10KiB just
>> from temporary QString creation and destruction)
> 
> I'm leaning towards agreeing to use QAnyStringView in the string 
> classes.

The part you're replying to is, however, a case of a traditional 
QString-only function: setObjectName().

> I'll remove my -2.

Thanks for that!

>> 2) the complexity is already there and QAnyStringView helps in 
>> reducing
>> it:
>> 
>>    https://codereview.qt-project.org/c/qt/qtbase/+/303483 (QCalendar)
>>    https://codereview.qt-project.org/c/qt/qtbase/+/303512 (QColor)
>>    https://codereview.qt-project.org/c/qt/qtbase/+/303707 (arg())
>>    https://codereview.qt-project.org/c/qt/qtbase/+/303708 (QUuid)
> 
> Agreed on arg(), it's a great clean-up and performance improvement.
> 
> But it's part of QString itself. The other ones, however, are the 
> slippery
> slope. I agree they improve performance for sink-only functions, but we 
> don't
> *need* QAnyStringView for them. For example, for QCalendar, they could 
> be the
> QStringView/QUtf8StringView pair.

Remember that a great deal of performance improvement already came from 
adding QLatin1String::arg() and QStringView::arg(). You can say this is 
string-classes, but it really isn't. It's formatting: you take a format 
string, parse it, and produce some result based on it. We have tons of 
these in our API: date/time come to mind. If QAnyStringView for arg() is 
a good idea, so it is for any format string, and by a ever-so-slight 
extension, for any parser input. Which brings us to:

> My problem is not with the clean up that it provides, it's adding yet 
> another
> class to our API.

We seems to have agreement on using QAnyStringView for "string classes". 
If we do, this argument is moot, as the class will _be_ there. It's then 
only a little extra step to my proposal, since a) using QAnyStringView 
more widely makes for a more consistent string story in Qt 6 and b) 
meeting my proposed minimal step would eradicate QLatin1String from our 
APIs, reducing the newcomer-need-to-know API by one class. 
+QAnyStringView -QLatin1String = same number of classes.

That said, I'll happily repeat my mantra that fewer classes don't make 
an API easier to use, it's fewer responsibilities per class that does. 
And my design is very clean here:

- QAnyStringView is the interface type (and only that)
- Q(Utf8)StringView are the parse types (via QAnyStringView::visit())
- QString is a (possible) storage type (std::u16string and 
QVAL<char16_t> are others,
   and I expect a lot more QVLA<char16_t> to be used in implementation as 
we move forward).

One class - exactly one responsibility. _That_ makes an API easy to use.

>> Now that I hopefully have convinced you that we need QAnyStringView,
>> where to go from here?
>> 
>> Given the lack of time until Qt 6.0, I'd like to propose to just 
>> replace
>> all overload sets that contain QL1S with one overload taking
>> QAnyStringView
> 
> Agreed for the string classes themselves.

I hope I've made my point about not stopping there.

>> The implementation usually contains the optimized handling of L1 data
>> already, and can often be easily extended to UTF-8, too, cf. QColor,
>> QUuid, arg().
> 
> Those are likely candidates, yes.
> 
> I just don't want to give blanket approval for everything. There may be 
> places
> where the correct solution is to delete the QLatin1String overload and 
> keep
> only QString.

I disagree here. If there's already a QL1S overload, we must never go 
back to just QString.

What instead we should look into is whether we can make a 
QString&&/QAnyStringView overload set work meaningfully (ie. no 
ambiguities for whatever the user passes). That would allow classes that 
actually store QStrings to allow transfer-of-ownership, at the cost of 
exposing an implementation detail. The main problem I see here is 
QStringBuilder.

Thanks,
Marc