Hi,
On 6/21/22 11:26, Giuseppe D'Angelo via Development wrote:
Hi,
On 20/06/2022 17:39, Thiago Macieira wrote:
On Monday, 20 June 2022 07:46:57 PDT Giuseppe D'Angelo via Development
wrote:
A fancy name for: "if a function/class is operating on a rvalue, should
it store a copy of it in order to keep it alive?". Consider
In other words, remove the rvalue reference and store a copy from the
const-
lvalue reference overload you already have.
Well, not necessarily. Say the input is a lvalue std::u16string; you
don't want the tokenizer to unconditionally copy it.
So it just takes a reference, and should continue to do so.
I'll just give my two cents here (and sorry if I end up re-stating the
obvious):
1. taking an rvalue can dangle, and robust code should not accept that
2. always copying can be potentially expensive.
Point number 1 trumps point number 2 IMHO, as it ensures that code will
never break, at the expense of a performance penalty in _relevant
hotpaths_. I would solve that by providing the user with a non-owning
separate API analogous to QByteArray::fromRawData(), that operates on
lvalue references.
This makes the reasoning over the API simpler IMHO - the base case is
robust, always working as intended. The user is free to pass the object
around as it's self-contained.
Should performance be an issue, the user can resort to the non-owning
version. This forces the user to reason about the code rather than
blindly and inconsequentially relying on some overload and spares a
future reader from having to figure out solely from context if a given
statement is copying or simply referencing a value.
To piggy-back on your example (Haystack&& represents an rvalue, not
universal refs, pseudo-code for illustration purposes):
1. QStringTokenizer(Haystack h, ...); // always copy
2. QStringTokenizer::makeView(const Haystack&, ...); // always reference
(only lvalues)
3. QStringTokenizer::makeView(Haystack&&, ...); // disabled/deleted -
compile error
(1) should be enough for most use-cases. (2) opens up for the
possibility of having a tokenizer that potentially carries a dangling
reference, but should only be used in optimization paths or local
scopes. (3) emits a compile error (the user should probably be using 1
in this case).
Not sure how scalable this approach is in terms of a global Qt API, though.
- Rafael
(Looking at std::views::all: this is the case where the input is a
non-view lvalue, so you'll wrap it in ref_view -- i.e. merely hold a
reference to it.)
std::u16string someLongString = u"...";
auto tokenizer = QStringTokenizer(someLongString, u"X");
For this reason QStringTokenizer moves and stores the input if it's an
rvalue, but only keeps a reference if it's an lvalue.
This means QStringTokenizer must have a QString member and a QString &
member.
The simplest implementation removes one of them.
Not always, see above.
QString s = getString();
auto tok = QStringTokenizer(s, u"x");
would take a copy (given `s` is a view after [4]), while now it only
takes a reference.
Yup.
Opinions?
My only objection is to calling this by a fancy name, "rvalue
pinning". Simply
call what it is: take all parameters by const-lvalue and never store a
reference.
It's also imprecise. We should use the C++20 range terminology which is
more accurate, but I don't want to cause even more confusion (given we
don't use that logic, and probably _can't_ use the logic just yet).
Thanks,
_______________________________________________
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development
_______________________________________________
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development
