Re: [Development] qMoveToConst helper for rvalue references to movable Qt containers?
Den sön 21 okt. 2018 kl 16:15 skrev Elvis Stansvik :
>
> Den lör 20 okt. 2018 kl 18:53 skrev Giuseppe D'Angelo via Development
> :
> >
> > Il 20/10/18 14:43, Elvis Stansvik ha scritto:
> > > In our application we added a helper like
> > >
> > > template
> > > const T moveToConst(T &&t)
> > > {
> > > return std::move(t);
> > > }
> > >
> > > that we use for these cases. It just moves the argument to a const
> > > value and returns it. With that we can do for (auto foo :
> > > moveToConst(returnsQtContainer()) { ... }.
> > >
> > > Since it's such a common pattern to want to iterate a returned Qt
> > > container, what would you think of having such a helper
> > > (qMoveToConst?) in Qt?
> >
> > Possibly... Note however that such a thing was already proposed for
> > qAsConst itself, and shut down to avoid having qAsConst diverge from
> > std::as_const (and I approve of not having Qt-specific behaviours). I
> > can't find the relevant discussion on the mailing list right now.
> >
> >
> > For reference, std::as_const's original authors had doubts about the
> > lifetime issues around this, saying that more investigations were needed:
> >
> > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0007r0.html#9.0
> >
> >
> > After a LEWG meeting it was reworded like this:
> >
> > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0007r1.html#8.0
> >
> >
> > I'm not entirely sure of what prompted the change for as_const -- if
> > just a safety net while waiting for more investigation, or if something
> > more profound was raised.
> >
> >
> > I can easily imagine however a scenario where moveToConst() can lead to
> > worse code than the current solution.
> >
> > Current solution:
> >
> > // GCE may get applied, 0 moves
> > const QVector v = getVector();
> > for (auto &obj : v) ~~~
> >
> > vs.
> >
> > // Always 2 moves
> > for (auto &obj : qMoveToConst(getVector()) ~~~
> >
> >
> > And if the type is not even cheap to move (e.g. QVLA, std::array),
> > qMoveToConst becomes a really unpleasant surprise.
These two points of yours of course still stands, and I can understand
why you wouldn't want this as a helper in Qt because of it.
Elvis
> >
> > How about asking LEWG about their reasoning too?
>
> I couldn't find a way to contact them.
>
> In order to try out the unsafe usage you suggested in your other mail,
> and also another unsafe usage pointed out in an SO question
> (https://stackoverflow.com/questions/39051460/why-does-as-const-forbid-rvalue-arguments/39051612#39051612),
> I made the following test program.
>
> The output when running is:
>
> [estan@newton move-to-const-test]$ ./move-to-const-test
> without moveToConst:
> FooPrivate constr from vector
> Foo constr with arg 0x7fffdb627200
> Foo begin 0x7fffdb627200
> Foo end 0x7fffdb627200
> Foo destr 0x7fffdb627200
> FooPrivate destr
>
> with moveToConst:
> FooPrivate constr from vector
> Foo constr with arg 0x7fffdb627208
> Foo move constr 0x7fffdb627210
> Foo destr 0x7fffdb627208
> Foo begin const 0x7fffdb627210
> Foo end const 0x7fffdb627210
> Foo destr 0x7fffdb627210
> FooPrivate destr
> [estan@newton move-to-const-test]$
>
> Which just shows it's working as intended.
>
> The two unsafe usages are commented out, because they wouldn't compile
> (good!).
>
> Note that the version suggested under Further Discussion in rev 0 is
> different from our helper, we return std::move(t) while they return t.
>
> With the version from rev 0, your example unsafe usage will compile
> (the one from SO still won't).
>
> Elvis
>
> #include
> #include
> #include
> #include
>
> class FooPrivate : public QSharedData {
> public:
> FooPrivate() {
> qDebug() << "FooPrivate default constr";
> };
>
> FooPrivate(const FooPrivate &other) : QSharedData(other) {
> qDebug() << "FooPrivate copy constr";
> };
>
> explicit FooPrivate(const QVector &v) : v(v) {
> qDebug() << "FooPrivate constr from vector";
> };
>
> ~FooPrivate() {
> qDebug() << "FooPrivate destr";
> };
>
> QVector v;
> };
>
> class Foo {
> public:
> Foo() : d(new FooPrivate) {
> qDebug() << "Foo default constr" << this;
> };
>
>
Re: [Development] qMoveToConst helper for rvalue references to movable Qt containers?
Den lör 20 okt. 2018 kl 18:53 skrev Giuseppe D'Angelo via Development
:
>
> Il 20/10/18 14:43, Elvis Stansvik ha scritto:
> > In our application we added a helper like
> >
> > template
> > const T moveToConst(T &&t)
> > {
> > return std::move(t);
> > }
> >
> > that we use for these cases. It just moves the argument to a const
> > value and returns it. With that we can do for (auto foo :
> > moveToConst(returnsQtContainer()) { ... }.
> >
> > Since it's such a common pattern to want to iterate a returned Qt
> > container, what would you think of having such a helper
> > (qMoveToConst?) in Qt?
>
> Possibly... Note however that such a thing was already proposed for
> qAsConst itself, and shut down to avoid having qAsConst diverge from
> std::as_const (and I approve of not having Qt-specific behaviours). I
> can't find the relevant discussion on the mailing list right now.
>
>
> For reference, std::as_const's original authors had doubts about the
> lifetime issues around this, saying that more investigations were needed:
>
> > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0007r0.html#9.0
>
>
> After a LEWG meeting it was reworded like this:
>
> > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0007r1.html#8.0
>
>
> I'm not entirely sure of what prompted the change for as_const -- if
> just a safety net while waiting for more investigation, or if something
> more profound was raised.
>
>
> I can easily imagine however a scenario where moveToConst() can lead to
> worse code than the current solution.
>
> Current solution:
>
> // GCE may get applied, 0 moves
> const QVector v = getVector();
> for (auto &obj : v) ~~~
>
> vs.
>
> // Always 2 moves
> for (auto &obj : qMoveToConst(getVector()) ~~~
>
>
> And if the type is not even cheap to move (e.g. QVLA, std::array),
> qMoveToConst becomes a really unpleasant surprise.
>
> How about asking LEWG about their reasoning too?
I couldn't find a way to contact them.
In order to try out the unsafe usage you suggested in your other mail,
and also another unsafe usage pointed out in an SO question
(https://stackoverflow.com/questions/39051460/why-does-as-const-forbid-rvalue-arguments/39051612#39051612),
I made the following test program.
The output when running is:
[estan@newton move-to-const-test]$ ./move-to-const-test
without moveToConst:
FooPrivate constr from vector
Foo constr with arg 0x7fffdb627200
Foo begin 0x7fffdb627200
Foo end 0x7fffdb627200
Foo destr 0x7fffdb627200
FooPrivate destr
with moveToConst:
FooPrivate constr from vector
Foo constr with arg 0x7fffdb627208
Foo move constr 0x7fffdb627210
Foo destr 0x7fffdb627208
Foo begin const 0x7fffdb627210
Foo end const 0x7fffdb627210
Foo destr 0x7fffdb627210
FooPrivate destr
[estan@newton move-to-const-test]$
Which just shows it's working as intended.
The two unsafe usages are commented out, because they wouldn't compile (good!).
Note that the version suggested under Further Discussion in rev 0 is
different from our helper, we return std::move(t) while they return t.
With the version from rev 0, your example unsafe usage will compile
(the one from SO still won't).
Elvis
#include
#include
#include
#include
class FooPrivate : public QSharedData {
public:
FooPrivate() {
qDebug() << "FooPrivate default constr";
};
FooPrivate(const FooPrivate &other) : QSharedData(other) {
qDebug() << "FooPrivate copy constr";
};
explicit FooPrivate(const QVector &v) : v(v) {
qDebug() << "FooPrivate constr from vector";
};
~FooPrivate() {
qDebug() << "FooPrivate destr";
};
QVector v;
};
class Foo {
public:
Foo() : d(new FooPrivate) {
qDebug() << "Foo default constr" << this;
};
Foo(const Foo &other) : d(other.d) {
qDebug() << "Foo copy constr" << this;
};
Foo(Foo &&other) : d(other.d) {
other.d = nullptr;
qDebug() << "Foo move constr" << this;
};
explicit Foo(const QVector &v) : d(new FooPrivate(v)) {
qDebug() << "Foo constr with arg" << this;
};
~Foo() {
qDebug() << "Foo destr" << this;
};
Foo &operator=(const Foo &other) {
qDebug() << "Foo copy ass op" << this;
d = other.d;
return *this;
};
QVector::iterator begin() {
qDebug() << "Foo begin" << this;
return d->v.begin();
};
QVector::const_iterator begin() const {
qDebu
Re: [Development] qMoveToConst helper for rvalue references to movable Qt containers?
(Adding back the mailing list)
Den lör 20 okt. 2018 kl 23:54 skrev Elvis Stansvik :
>
> Den lör 20 okt. 2018 kl 23:50 skrev Giuseppe D'Angelo
> :
> >
> > Il 20/10/18 19:37, Elvis Stansvik ha scritto:
> > > If the C++ wizards considered this but were hesitant, then I think
> > > it's right that Qt is hesitant as well. I hadn't really considered
> > > potential life-time issues, so I guess what we're doing might possibly
> > > be unsafe (?).
> >
> > Probably in the sense that the function you pasted can be applied like this:
> >
> > QVector v;
> > for (auto &o : qMoveAsConst(v)) ~~~; // compiles with an lvalue!
> > use(v); // unspecified behavior
>
> Ah yes, it may be that the standards folks simply didn't want this
> because of foot-shooting potential like this.
Another potential foot-shooter I found on an SO question
(https://stackoverflow.com/a/39051612/252857):
for (auto const &&value : as_const(getQString())) // whoops!
{
}
Elvis
>
> >
> >
> > > What's GCE? Some optimization? (Google "c++ gce" didn't give me anything).
> >
> > Guaranteed Copy Elision:
> >
> > > https://en.cppreference.com/w/cpp/language/copy_elision
>
> Thanks!
>
> Elvis
>
> >
> > My 2 c,
> >
> > --
> > Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer
> > KDAB (France) S.A.S., a KDAB Group company
> > Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
> > KDAB - The Qt, C++ and OpenGL Experts
> >
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] qMoveToConst helper for rvalue references to movable Qt containers?
Den lör 20 okt. 2018 kl 18:53 skrev Giuseppe D'Angelo via Development
:
>
> Il 20/10/18 14:43, Elvis Stansvik ha scritto:
> > In our application we added a helper like
> >
> > template
> > const T moveToConst(T &&t)
> > {
> > return std::move(t);
> > }
> >
> > that we use for these cases. It just moves the argument to a const
> > value and returns it. With that we can do for (auto foo :
> > moveToConst(returnsQtContainer()) { ... }.
> >
> > Since it's such a common pattern to want to iterate a returned Qt
> > container, what would you think of having such a helper
> > (qMoveToConst?) in Qt?
>
> Possibly... Note however that such a thing was already proposed for
> qAsConst itself, and shut down to avoid having qAsConst diverge from
> std::as_const (and I approve of not having Qt-specific behaviours). I
> can't find the relevant discussion on the mailing list right now.
>
>
> For reference, std::as_const's original authors had doubts about the
> lifetime issues around this, saying that more investigations were needed:
>
> > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0007r0.html#9.0
>
>
> After a LEWG meeting it was reworded like this:
>
> > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0007r1.html#8.0
>
>
> I'm not entirely sure of what prompted the change for as_const -- if
> just a safety net while waiting for more investigation, or if something
> more profound was raised.
>
>
> I can easily imagine however a scenario where moveToConst() can lead to
> worse code than the current solution.
>
> Current solution:
>
> // GCE may get applied, 0 moves
> const QVector v = getVector();
> for (auto &obj : v) ~~~
>
> vs.
>
> // Always 2 moves
> for (auto &obj : qMoveToConst(getVector()) ~~~
>
>
> And if the type is not even cheap to move (e.g. QVLA, std::array),
> qMoveToConst becomes a really unpleasant surprise.
>
> How about asking LEWG about their reasoning too?
>
Thanks a lot for the pointers and back story Giuseppe. I definitely
think it's good that qAsConst mirrors what std::as_const did, so
wouldn't want it expanded.
If the C++ wizards considered this but were hesitant, then I think
it's right that Qt is hesitant as well. I hadn't really considered
potential life-time issues, so I guess what we're doing might possibly
be unsafe (?).
What's GCE? Some optimization? (Google "c++ gce" didn't give me anything).
Elvis
>
> My 2 c,
> --
> Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer
> KDAB (France) S.A.S., a KDAB Group company
> Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
> KDAB - The Qt, C++ and OpenGL Experts
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
[Development] qMoveToConst helper for rvalue references to movable Qt containers?
Hi all (first post),
In Qt 5.7+ there's qAsConst, an std::as_const implementation for those
who are not on C++17 yet, which is convenient for iterating over Qt
containers using range-based for loops without causing a detach.
For good reasons there's no version of qAsConst that takes an rvalue
reference, so you can't do e.g. for (auto foo :
qAsConst(returnsQtContainer()) { ... }. Instead you must do const
auto stuff = returnsQtContainer(); for (auto foo : stuff) { ... }.
In our application we added a helper like
template
const T moveToConst(T &&t)
{
return std::move(t);
}
that we use for these cases. It just moves the argument to a const
value and returns it. With that we can do for (auto foo :
moveToConst(returnsQtContainer()) { ... }.
Since it's such a common pattern to want to iterate a returned Qt
container, what would you think of having such a helper
(qMoveToConst?) in Qt?
Cheers,
Elvis
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
