Re: [Development] Views in APIs
On 15/05/2020 06.39, Edward Welbourne wrote: Thiago Macieira (15 May 2020 02:30) replied On Thursday, 14 May 2020 12:34:54 PDT Matthew Woehlke wrote: On 14/05/2020 14.58, Thiago Macieira wrote: Which is: b) misspelling "iteratable" Be that as it may, that ship has sailed: https://www.google.com/search?q=iteratable Even Google thinks so, and if you insist otherwise, 12k results instead of 2M. The majority of the results in the first and second pages are about a company called Iterable. ...but also on those pages are references to the Java and Javascript concepts. Then... It remains that "iterable" is the standard form in python's extensive documentation and is the form I am used to. ...there's Python. Lisp seems to be the least obscure language using "iteratable". Even Wiktionary¹ gives "iterable" as the "correct" spelling, and doesn't even have an entry for "iteratable". (¹ https://en.wiktionary.org/wiki/iterable) I'm *not* arguing that you (Thiago) aren't, pedantically, correct. I'm just pointing out that, as Edward notes, you're fighting the tide. -- Matthew ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 14/05/2020 14.58, Thiago Macieira wrote: >>> Which is: >>> b) misspelling "iteratable" On Thursday, 14 May 2020 12:34:54 PDT Matthew Woehlke wrote: >> Be that as it may, that ship has sailed: >> >>https://www.google.com/search?q=iteratable >> >> Even Google thinks so, and if you insist otherwise, 12k results instead >> of 2M. Thiago Macieira (15 May 2020 02:30) replied > The majority of the results in the first and second pages are about a company > called Iterable. That'll be because a company's marketing department works hard to get its pages at the top of Google's search results. It remains that "iterable" is the standard form in python's extensive documentation and is the form I am used to. English evolves according to the way it's mostly used, regardless of anyone's attempts to impose rules. Feel free to resist (that, also, is part of the evolutionary process), but remember that you may be trying to stop the tide. Eddy. ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On Thursday, 14 May 2020 12:34:54 PDT Matthew Woehlke wrote: > On 14/05/2020 14.58, Thiago Macieira wrote: > > Which is: > > b) misspelling "iteratable" > > Be that as it may, that ship has sailed: > >https://www.google.com/search?q=iteratable > > Even Google thinks so, and if you insist otherwise, 12k results instead > of 2M. The majority of the results in the first and second pages are about a company called Iterable. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel System Software Products ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
> To have type-erased containers, we need to go back to something like
java.lang.Collection.
C++20 offers coroutines which allows the same things without java-like type
hierarchies though
(and with much more flexibility as you don't even need to have an actual
container existing somewhere - your function's stack
can serve as one just fine :p).
see e.g. https://godbolt.org/z/cEv7Gp - in simple cases it optimizes as
well as as a std::vector.
Well, for Qt 7 maybe :-)
On Thu, May 14, 2020 at 9:00 PM Thiago Macieira
wrote:
> On Thursday, 14 May 2020 02:33:43 PDT Marc Mutz via Development wrote:
> > On 2020-05-14 02:15, Thiago Macieira wrote:
> > > On quarta-feira, 13 de maio de 2020 01:41:26 PDT Иван Комиссаров wrote:
> > >> std::span Project::productsSpan() const & { return
> > >> d->products; }
> > >> std::span Project::productsSpan() const && = delete;
> > >
> > > Now implement either or both of these functions if internally you have
> > > a
> > > QSet.
> >
> > Now implement QHash roleNames() when
> > internally you have a std::pmr::unordered_map > std::pmr::string>, with all the data allocated from one
> > std::pmr::monotonic_buffer_resource.
> >
> > This does both ways.
>
> No, it doesn't. In both you created a copy on demand.
>
> But you can only *return* said copy if the return type is an owning
> container.
> std::span and the views are not.
>
> > And: you can also have views over associated data structures, they just
> > can't have inline functions because they need to type-erase the
> > container. For an extreme example, consider QAssociativeIterable.
>
> Which is:
> a) a horrible API
> b) misspelling "iteratable"
>
> To have type-erased containers, we need to go back to something like
> java.lang.Collection.
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
> Software Architect - Intel System Software Products
>
>
>
> ___
> Development mailing list
> Development@qt-project.org
> https://lists.qt-project.org/listinfo/development
>
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 14/05/2020 14.58, Thiago Macieira wrote: Which is: b) misspelling "iteratable" Be that as it may, that ship has sailed: https://www.google.com/search?q=iteratable Even Google thinks so, and if you insist otherwise, 12k results instead of 2M. -- Matthew ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
On Thursday, 14 May 2020 04:20:43 PDT Иван Комиссаров wrote: > I don’t see the problem here. Ranges library operates on views only, so > there *should be* (I am just not aware of it’s name) the type that > corresponds to the «forward access iterator» (well, in case of QSet, that > would be bidirectional but not random access iterator) range. > > If there’s no such view, it’s quite easy to implement it - it’s simply a > pair of iterators. You're missing the point. I want you to implement it without changing your return type. Or, in different words: choose one return type for your non-inline function that you can be sure to support for the next 7 years, regardless of internal modifications to your data store. > The disadvantage here is that such a view doesn’t gives the same flexibility > as std::span does. i.e. it is still not possible to replace QSet with > std::unordered_set, so we don’t have much gain compared to returning QSet / > const QSet &. > > So, as I said, I do not see the problem. Can we always return views? Yes. Do > they gives us benefits in every case? No. Should we do it? Almost never. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel System Software Products ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On Thursday, 14 May 2020 02:33:43 PDT Marc Mutz via Development wrote:
> On 2020-05-14 02:15, Thiago Macieira wrote:
> > On quarta-feira, 13 de maio de 2020 01:41:26 PDT Иван Комиссаров wrote:
> >> std::span Project::productsSpan() const & { return
> >> d->products; }
> >> std::span Project::productsSpan() const && = delete;
> >
> > Now implement either or both of these functions if internally you have
> > a
> > QSet.
>
> Now implement QHash roleNames() when
> internally you have a std::pmr::unordered_map std::pmr::string>, with all the data allocated from one
> std::pmr::monotonic_buffer_resource.
>
> This does both ways.
No, it doesn't. In both you created a copy on demand.
But you can only *return* said copy if the return type is an owning container.
std::span and the views are not.
> And: you can also have views over associated data structures, they just
> can't have inline functions because they need to type-erase the
> container. For an extreme example, consider QAssociativeIterable.
Which is:
a) a horrible API
b) misspelling "iteratable"
To have type-erased containers, we need to go back to something like
java.lang.Collection.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel System Software Products
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On Thu, May 14, 2020 at 11:46:29AM +0200, Marc Mutz via Development wrote: > > Please stop with this crusade of yours to end all CoW, get rid of > > QList, etc. It is misguided and harmful to the ecosystem at large. > > You are entitled to your opinions just as I am. The difference is that I put > time (much more than KDAB pays me for, esp. when it comes to my "crusade") > where my mouth is. The problem here is not what personal or company time you invest yourself. You can spent as much of that as you wish or are allowed to. The problem is the effort these activities cause for others, especially for those that do not share your opinions, and are still forced to adapt their code to your preferences. Andre' ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
I don’t see the problem here. Ranges library operates on views only, so there
*should be* (I am just not aware of it’s name) the type that corresponds to the
«forward access iterator» (well, in case of QSet, that would be bidirectional
but not random access iterator) range.
If there’s no such view, it’s quite easy to implement it - it’s simply a pair
of iterators.
The disadvantage here is that such a view doesn’t gives the same flexibility as
std::span does. i.e. it is still not possible to replace QSet with
std::unordered_set, so we don’t have much gain compared to returning QSet /
const QSet &.
So, as I said, I do not see the problem. Can we always return views? Yes. Do
they gives us benefits in every case? No.
Ivan
> 14 мая 2020 г., в 02:15, Thiago Macieira
> написал(а):
>
> On quarta-feira, 13 de maio de 2020 01:41:26 PDT Иван Комиссаров wrote:
>> std::span Project::productsSpan() const & { return d->products; }
>> std::span Project::productsSpan() const && = delete;
>
> Now implement either or both of these functions if internally you have a
> QSet.
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
> Software Architect - Intel System Software Products
>
>
>
> ___
> Development mailing list
> Development@qt-project.org
> https://lists.qt-project.org/listinfo/development
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 2020-05-13 17:27, Matthew Woehlke wrote: On 12/05/2020 12.59, Marc Mutz via Development wrote: AsidE: If you think that CoW is still a thing today: no. SSO is a thing these days, and it seems that QString will not have it in Qt 6, either. NOI favours SSO, QString-everywhere cements the naïve CoW world of the 1990s for yet another decade. I am really, *really* sick of this. Okay, for "most" *strings*, you may have a point. I thought this thread was about strings. Did I miss something? However, CoW is *absolutely* still a useful tool for a lot of other applications, and will continue to be so; *Optional* *explicit* sharing, as per shared_ptr is a thing, yes. But that's completely different from the broken Qt "implicit sharing" that we have now. Have you read the 22year-old Sutter articles? After having read the second, have you checked Qt's implementation for the pitfalls mentioned therein? Continue discussing after you did. the combination of implicit value semantics and "cheap" copies (an atomic increment may be relatively expensive, but so are memory allocations, especially for large data structures) is not going away any time soon. Please stop with this crusade of yours to end all CoW, get rid of QList, etc. It is misguided and harmful to the ecosystem at large. You are entitled to your opinions just as I am. The difference is that I put time (much more than KDAB pays me for, esp. when it comes to my "crusade") where my mouth is. Thanks, Marc ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 2020-05-14 02:15, Thiago Macieira wrote:
On quarta-feira, 13 de maio de 2020 01:41:26 PDT Иван Комиссаров wrote:
std::span Project::productsSpan() const & { return
d->products; }
std::span Project::productsSpan() const && = delete;
Now implement either or both of these functions if internally you have
a
QSet.
Now implement QHash roleNames() when
internally you have a std::pmr::unordered_mapstd::pmr::string>, with all the data allocated from one
std::pmr::monotonic_buffer_resource.
This does both ways.
And: you can also have views over associated data structures, they just
can't have inline functions because they need to type-erase the
container. For an extreme example, consider QAssociativeIterable.
Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
On quarta-feira, 13 de maio de 2020 01:41:26 PDT Иван Комиссаров wrote:
> std::span Project::productsSpan() const & { return d->products; }
> std::span Project::productsSpan() const && = delete;
Now implement either or both of these functions if internally you have a
QSet.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel System Software Products
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 13/05/2020 03.04, Lars Knoll wrote: I don’t see a need to take a QStringView in those cases. With the exception of some low level APIs, Qt’s APIs should be safe to use. An overload taking a QStringView would either need to copy the data (in which case we could just as well only offer the QString version), or it holds an ‘unsecured’ pointer to the data, and lifetime is out of control of the class. The second option is something I’m only willing to accept in a very limited number of cases, and the API naming should make it clear that this is what’s happening. I don't see a need for that. An API that might retain a copy of a string should take a QString. An API that needs to *look at* a string, but will not *directly*¹ copy that data, should take a QStringView. If this isn't completely transparent to the user of the API, we're doing something wrong. (¹ An API that must always make a deep copy, e.g. by storing a modified version of an input string, isn't making a "direct" copy.) That said, you can run into issues if the implementation of the API changes. You might call CoW naive, but I do believe that the fact that Qt does use containers that own their data in most of our APIs is what makes Qt significantly simpler and safer to use than many other APIs. No question. On of the projects I work with uses std::shared_ptr all over the place, because large data structures get passed around a lot. One of the developers is absolutely fanatical about avoiding deep copies, although I personally have not seen hard data whether this is merited (but my suspicion is that it is). Shared ownership is a pain point, because users have to be careful to make a copy (which, due to polymorphism, is already "hard") if they need to change data and aren't sure if anyone else is using the data. Unfortunately, all this was designed before I became involved, or I would have insisted on Qt-style CoW value-like semantics, which would have made the whole mess *much* safer and easier to use. Qt did something right. Let's please not throw that away because it "isn't fashionable". -- Matthew ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 12/05/2020 12.59, Marc Mutz via Development wrote: AsidE: If you think that CoW is still a thing today: no. SSO is a thing these days, and it seems that QString will not have it in Qt 6, either. NOI favours SSO, QString-everywhere cements the naïve CoW world of the 1990s for yet another decade. I am really, *really* sick of this. Okay, for "most" *strings*, you may have a point. However, CoW is *absolutely* still a useful tool for a lot of other applications, and will continue to be so; the combination of implicit value semantics and "cheap" copies (an atomic increment may be relatively expensive, but so are memory allocations, especially for large data structures) is not going away any time soon. Please stop with this crusade of yours to end all CoW, get rid of QList, etc. It is misguided and harmful to the ecosystem at large. -- Matthew ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 2020-05-13 09:56, Philippe wrote: On Tue, 12 May 2020 18:59:35 +0200 Marc Mutz via Development wrote: QString-everywhere cements the naïve CoW world of the 1990s for yet another decade. CoW naive? Really not, this method is at the heart of many strategic technologies. eg.: * virtual memory used by most of operating system * some databases * principle behind any immutable data structure The keyword being 'immutable'. QString is not immutable. I'm all for going more immer-style, but you can't possibly claim that's what's happening in Qt 6. Immer is modern, the crude CoW implementations used in Qt (incl. 6) _are_ naïve. ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On Wed, 13 May 2020 at 12:46, Marc Mutz via Development wrote: > In the not so hypothetical case that Qt is used to visualize results of > some business calculations, chances are that thrid-party libraries will > use std::string or std::u16string, and not QString, requiring the use of > QString::fromStdString() to pass these to a QString API. Had the API > taken QStringView, no extra code would have been necessary. Wait, what? How can QStringView view a std::string? ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 2020-05-13 11:31, Marco Bubke wrote:
auto view3 = myObject.getCopy()[42].getView(); // not safe, copy is
destroyed
AFAIK you can say that a rvalue should not return a reference by using
ref-qualified member functions.
View getView() const & { return v;}
Qt is so many years behind the mainstream... This is a big problem in
ranges-v3, do you really believe no-one works on this?
https://github.com/isocpp/CppCoreGuidelines/blob/master/docs/Lifetime.pdf
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
Nope, that would not be C++ if it wasn’t broken:
class C1
{
public:
std::string_view getMember() const & { return member; }
std::string member;
};
std::string_view m = C1().getMember(); // compiles!
You have to delete the method explicitly:
std::string_view getMember() const && = delete;
Now compilation fails:
call to deleted member function 'getMember'
std::string_view m = C1().getMember();
~^
> 13 мая 2020 г., в 11:31, Marco Bubke написал(а):
>
>
> auto view3 = myObject.getCopy()[42].getView(); // not safe, copy is destroyed
>
> AFAIK you can say that a rvalue should not return a reference by using
> ref-qualified member functions.
>
> View getView() const & { return v;}
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
On Wed, 13 May 2020 at 12:37, Marco Bubke wrote:
> auto view3 = myObject.getCopy()[42].getView(); // not safe, copy is destroyed
> AFAIK you can say that a rvalue should not return a reference by using
> ref-qualified member functions.
> View getView() const & { return v;}
However,
whatever_function(myObject.getCopy()[42].getView()); // never dangles,
so always safe
Disabling reference returns for rvalues prevents such forwarding cases
from working, and you need
to declare a separate lvalue to make your code work.
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs
On 2020-05-13 09:04, Lars Knoll wrote:
On 12 May 2020, at 18:59, Marc Mutz wrote:
[...]
Most other classes:
* Only take and return QString
This is wrong. By taking a QString in the API of non-string-related
APIs, you expose QString as an implementation detail of the class.
Think QRegion, which, before I added begin()/end(), had a SSO-like
internal container (1 QRect or QVector) which forced most users
to code around the owning-container API like this:
I don’t think you can compare those cases. QString is the container
our users use in 99% of the cases to hold a string. And this won’t
change (and I don’t think we should advocate any changes here).
So then, an API taking and returning a QString is the most logical,
easiest and most convenient to use.
There are two independent issues here: taking and returning.
For taking I don't see how "taking QString is most logical, easiest,
most convenient" follows. It's simply not true that 99% of the cases to
hold a string use QString. In the common case of applications which are
not localized, all the nice QString APIs are fed with const char*
literals, causing QString creation and destruction in user code. Even if
you change all these to const char16_t* and a QString that doesn't
allocate in this case, you still have all these complex (in the
QTypeInfo sense) QString objects created and destroyed at the call site.
This bloats user code. If all those shiny QString APIs would take
QStringView instead, the construction and destruction of which is
trivial (in the C++ sense), the compiler can remove all those calls and
the QString construction (if any) happens centrally inside the library
function (O(1) instead of O(N), N = #callers).
In the not so hypothetical case that Qt is used to visualize results of
some business calculations, chances are that thrid-party libraries will
use std::string or std::u16string, and not QString, requiring the use of
QString::fromStdString() to pass these to a QString API. Had the API
taken QStringView, no extra code would have been necessary.
So, I have shown that taking QString is neither the most convenient, nor
the most easy, choice, as it requires users with other string types as
data sources to jump through hoops to pass their data. Only if you
employ a very narrow focus where both efficiency and the existence of
3rd-party code are all ignored, can you still maintain that a function
taking an owning container is more convenient and easier than one taking
a view. The only logic in such an API that _I_ can find, however, is
"MUST ... NOT ... BREAK ... COW logic", which has been proven as flawed
over twenty years ago:
http://www.gotw.ca/gotw/043.htm
http://www.gotw.ca/gotw/044.htm
http://www.gotw.ca/gotw/045.htm
(and CoW isn't even correctly implemented in Qt, ever since unsharable
data was removed).
In case the internal storage _is_ QString, then providing a QString&&
overload to avoid the copy is a good idea, if you're willing to impart
the details of the implementation that way.
Second, returning.
You talk about QString getting an SSO buffer, maybe. Then returning a
QString will become even more expensive. It already got more expensive,
since instead of sizeof(void*) it's now two or three (didn't check)
words, but adding SSO will not make it better. And if we don't get SSO,
classes can decide to store u16string instead, which _has_ SSO already.
So, efficiency wins here, too.
if (r.rectCount() == 1) {
use(r.boundingRect());
} else {
const auto rects = r.rects();
for (const QRect &rect : rects)
use(rect);
}
If rects() had been a view (today, we'd use gsl/std::spanQRect>), all users could just do
I don’t get that argument. The region is a list of rectangles today,
so you could simply add a rects() method that returns them and the
code below would work.
I think you should take a look at QRegionPrivate::begin() (in Qt 6 or Qt
5). And in Qt 5's version of QRegion::rects().
for (const QRect &rect : r.rects())
use(rect);
This is objectively a better API, for two reasons: a) the user doesn't
need to care about some weird idiosyncrasies of the class to avoid
performance penalties and b) the class author is now free to extend
the SSO buffer from one to, say, four, without changing the API, not
even those affected by Hyrum.
It _seems_ your solution is to fold views into owning containers, and
while that may seem to work, it's dangerous:
Assume QRegion::rects() returned a QVector-acting-as-view. Then this
would silently fail:
QRegion region();
for (const QRect &rect : region().rects())
use(rect);
because, clearly, QVector is an owning container, so we don't care
that the QRegion went out of scope. Whereas with a view, it will be
immediately obvious (to a tool like Clazy, at least) that this can't
work.
The above example is rather weirdly constructed.
But anyhow, those data lifetime issues when using views as
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
auto view3 = myObject.getCopy()[42].getView(); // not safe, copy is destroyed
AFAIK you can say that a rvalue should not return a reference by using
ref-qualified member functions.
View getView() const & { return v;}
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
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{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 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 Project::productsSpan() const & { return d->products; }
std::span Project::productsSpan() const && = delete;
auto results = createProject().products() | std::views::transform(…); //
compile error, no need for Clazy!
Ivan
> 13 мая 2020 г., в 10:14, Иван Комиссаров написал(а):
>
>
>
>> 13 мая 2020 г., в 09:04, Lars Knoll > > написал(а):
>>
>> 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 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 and so on).
>
> Ivan
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
> 13 мая 2020 г., в 09:04, Lars Knoll написал(а):
>
> 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 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 and so on).
Ivan___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
On Tue, 12 May 2020 18:59:35 +0200 Marc Mutz via Development wrote: > QString-everywhere cements the naïve CoW world of the > 1990s for yet another decade. CoW naive? Really not, this method is at the heart of many strategic technologies. eg.: * virtual memory used by most of operating system * some databases * principle behind any immutable data structure Philippe ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Views in APIs (was: Re: QString and related changes for Qt 6)
> On 12 May 2020, at 18:59, Marc Mutz wrote:
>
> Hi Lars,
>
> Let me pick important points one per email here:
>
> On 2020-05-12 09:49, Lars Knoll wrote:
> [...]
>> For String related classes:
>> * All methods not taking ownership of the passed arguments take a
>> QStringView
>
> Almost (see below).
>
>> * If the method stores a pointer to the passed data it should take a
>> QString to not surprise users.
>
> ... in addition ... (though see below).
I don’t see a need to take a QStringView in those cases. With the exception of
some low level APIs, Qt’s APIs should be safe to use. An overload taking a
QStringView would either need to copy the data (in which case we could just as
well only offer the QString version), or it holds an ‘unsecured’ pointer to the
data, and lifetime is out of control of the class.
The second option is something I’m only willing to accept in a very limited
number of cases, and the API naming should make it clear that this is what’s
happening.
>> Exceptions can be done where it makes
>> sense, but then the method naming has to give clear indications that
>> this happens (like e.g. fromRawData())
>> * Return a QString in QString itself and when doing conversions,
>> return QStringView from QStringView
>> * No QStringRef!
>> * QLatin1String for backwards compatibility, can be disabled with a
>> macro (similar to QT_NO_CAST_FROM_ASCII)
>> * Remove or deprecate overloads taking a (const Char *, length) pair.
>> Replace them with QStringView
>
> +1 to the above
>
>> Most other classes:
>> * Only take and return QString
>
> This is wrong. By taking a QString in the API of non-string-related APIs, you
> expose QString as an implementation detail of the class. Think QRegion,
> which, before I added begin()/end(), had a SSO-like internal container (1
> QRect or QVector) which forced most users to code around the
> owning-container API like this:
I don’t think you can compare those cases. QString is the container our users
use in 99% of the cases to hold a string. And this won’t change (and I don’t
think we should advocate any changes here).
So then, an API taking and returning a QString is the most logical, easiest and
most convenient to use.
>
> if (r.rectCount() == 1) {
> use(r.boundingRect());
> } else {
> const auto rects = r.rects();
> for (const QRect &rect : rects)
> use(rect);
> }
>
> If rects() had been a view (today, we'd use gsl/std::span), all
> users could just do
I don’t get that argument. The region is a list of rectangles today, so you
could simply add a rects() method that returns them and the code below would
work.
>
> for (const QRect &rect : r.rects())
> use(rect);
>
> This is objectively a better API, for two reasons: a) the user doesn't need
> to care about some weird idiosyncrasies of the class to avoid performance
> penalties and b) the class author is now free to extend the SSO buffer from
> one to, say, four, without changing the API, not even those affected by Hyrum.
>
> It _seems_ your solution is to fold views into owning containers, and while
> that may seem to work, it's dangerous:
>
> Assume QRegion::rects() returned a QVector-acting-as-view. Then this would
> silently fail:
>
>QRegion region();
>for (const QRect &rect : region().rects())
>use(rect);
>
> because, clearly, QVector is an owning container, so we don't care that the
> QRegion went out of scope. Whereas with a view, it will be immediately
> obvious (to a tool like Clazy, at least) that this can't work.
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.
>
> So, I'd argue for the complete opposite here: we would increase encapsulation
> of our APIs if they stopped trafficking in owning container types. I call
> this the NOI pattern (Non-Owning Interface). By not having to serve QString
> everywhere, we'll be much more free to use alternative storage types in the
> implementation (e.g. QVarLengthArray, or - the horror! -
> std::pmr::u16string). Handling-wise, QStringView makes all these choices
> equal, so the implementation of a class can use whatever is objectively
> optimal instead of being bound to QString.
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.
>
> AsidE: If you think that CoW is still a thing today: no. SSO is a thing these
> days, and it seems that QString will not have it in Qt 6, either. NOI favours
[Development] Views in APIs (was: Re: QString and related changes for Qt 6)
Hi Lars,
Let me pick important points one per email here:
On 2020-05-12 09:49, Lars Knoll wrote:
[...]
For String related classes:
* All methods not taking ownership of the passed arguments take a
QStringView
Almost (see below).
* If the method stores a pointer to the passed data it should take a
QString to not surprise users.
... in addition ... (though see below).
Exceptions can be done where it makes
sense, but then the method naming has to give clear indications that
this happens (like e.g. fromRawData())
* Return a QString in QString itself and when doing conversions,
return QStringView from QStringView
* No QStringRef!
* QLatin1String for backwards compatibility, can be disabled with a
macro (similar to QT_NO_CAST_FROM_ASCII)
* Remove or deprecate overloads taking a (const Char *, length) pair.
Replace them with QStringView
+1 to the above
Most other classes:
* Only take and return QString
This is wrong. By taking a QString in the API of non-string-related
APIs, you expose QString as an implementation detail of the class. Think
QRegion, which, before I added begin()/end(), had a SSO-like internal
container (1 QRect or QVector) which forced most users to code
around the owning-container API like this:
if (r.rectCount() == 1) {
use(r.boundingRect());
} else {
const auto rects = r.rects();
for (const QRect &rect : rects)
use(rect);
}
If rects() had been a view (today, we'd use gsl/std::span),
all users could just do
for (const QRect &rect : r.rects())
use(rect);
This is objectively a better API, for two reasons: a) the user doesn't
need to care about some weird idiosyncrasies of the class to avoid
performance penalties and b) the class author is now free to extend the
SSO buffer from one to, say, four, without changing the API, not even
those affected by Hyrum.
It _seems_ your solution is to fold views into owning containers, and
while that may seem to work, it's dangerous:
Assume QRegion::rects() returned a QVector-acting-as-view. Then this
would silently fail:
QRegion region();
for (const QRect &rect : region().rects())
use(rect);
because, clearly, QVector is an owning container, so we don't care that
the QRegion went out of scope. Whereas with a view, it will be
immediately obvious (to a tool like Clazy, at least) that this can't
work.
So, I'd argue for the complete opposite here: we would increase
encapsulation of our APIs if they stopped trafficking in owning
container types. I call this the NOI pattern (Non-Owning Interface). By
not having to serve QString everywhere, we'll be much more free to use
alternative storage types in the implementation (e.g.
QVarLengthArray, or - the horror! - std::pmr::u16string).
Handling-wise, QStringView makes all these choices equal, so the
implementation of a class can use whatever is objectively optimal
instead of being bound to QString.
AsidE: If you think that CoW is still a thing today: no. SSO is a thing
these days, and it seems that QString will not have it in Qt 6, either.
NOI favours SSO, QString-everywhere cements the naïve CoW world of the
1990s for yet another decade.
Learn from QRegion!
I have spoken on many conferences (at least QtWS, Meeting C++, emBO++)
on this, if anyone wants to learn more.
Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
