Btw, I actually have an argument for returning a view=)
The ongoing c++20 ranges have exactly the same problem with the lifetime
issues. For instance, this won’t compile:
auto results = std::vector<int>{1, 2, 3, 4, 5, 6}
| std::views::filter([](int n){ return n % 2 == 0; })
| std::views::transform([](int n){ return n * 2; });
But this will:
std::vector<int> numbers = {1, 2, 3, 4, 5, 6};
auto results = numbers
| std::views::filter([](int n){ return n % 2 == 0; })
| std::views::transform([](int n){ return n * 2; });
So, we’re returning back to the products and range-for-loop example:
const auto products = project->productsList();
auto results = products | std::views::transform(…);
==OR==
auto results = project->productsSpan() | std::views::transform(...);
To me, second looks much more intuitive than the first one.
And yes, it is possible to disable the && overload for a method that returns a
view so the chained calls won’t compile
(doesn’t solve all the other issues, though and is a bit verbose):
std::span<Product> Project::productsSpan() const & { return d->products; }
std::span<Product> Project::productsSpan() const && = delete;
auto results = createProject().products() | std::views::transform(…); //
compile error, no need for Clazy!
Ivan
> 13 мая 2020 г., в 10:14, Иван Комиссаров <[email protected]> написал(а):
>
>
>
>> 13 мая 2020 г., в 09:04, Lars Knoll <[email protected]
>> <mailto:[email protected]>> написал(а):
>>
>> 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
_______________________________________________
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development
