[Development] Views in APIs (was: Re: QString and related changes for Qt 6)

[Иван Комиссаров]

> 13 мая 2020 г., в 09:04, Lars Knoll <lars.knoll at qt.io> написал(а):
> 
> The above example is rather weirdly constructed. 
> 
> But anyhow, those data lifetime issues when using views as return values extensively are a lot less obvious to spot when a human reads the code. APIs should be safe to use wherever possible, so that our users don’t have to worry about those things. This will lead to fewer bugs in the resulting code and faster development times. Trading that for some saved CPU cycles is in almost all cases (and yes there are exceptions in low level classes) a very bad deal.

Well, I can’t say that returning COW container is an easy thing to do. It’s a trade-off between «safety» and «some CPU cycles». And I’d say it’s much better if app crashes compared to the case where I have spent ages in profiler fixing performance bugs introduced by the developers who doesn’t really know how COW works (e.g. use operator[] instead of .at(), hidden detaches)

There's already a «-Wclazy-range-loop» warning in Clazy about detaching COWs:

From Qbs code (which is written by the developers who are supposed to know COW problems): 

1. for (const QString &abi: rhs.split(QLatin1Char(' '))) // attempt to detach

2. QList<ProductData> ProjectData::products() const { return d->products; }
    for (const ProductData &p : project.products()) // oops, deep copy

And I can’t say that creating a variable on the stack before every for-loop is an easier way for users (note, qAsConst doesn’t work for temporaries and it should not).

The point is - there’s already a check in Clazy that does the similar check - on can add a check for using a views from temporary.

And the lifetime issues only come into play when mixing views and non-views approach:

Object myObject;
auto view1 = myObject.getView()[42].getView(); // safe!
auto view2 = myObject.getReference()[42].getView(); // safe!
auto view3 = myObject.getCopy()[42].getView(); // not safe, copy is destroyed

> You can just as well argue the other way round. Returning a view requires the class to store the data in continuous memory. It can’t create it on the fly if asked.

How often is such a use-case, when you realize that you need to change the simple getter to a container?

Why cannot you simply add a method with a new name in that case?

Am not really advocating for returning views, but it’s not that black and white as you described, that’s a trade-off.
What I am advocating for is that functions should take views where possible - this is perfectly safe (you can pass temporaries into a view) and leads to much easier code in some cases (users can pass unicode literals, std::wstring, QVector<QChar> and so on).

Ivan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20200513/8ee2002f/attachment.html>