Den lör 20 okt. 2018 kl 18:53 skrev Giuseppe D'Angelo via Development <[email protected]>: > > Il 20/10/18 14:43, Elvis Stansvik ha scritto: > > In our application we added a helper like > > > > template <typename T> > > 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<Object> 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 <QSharedData> #include <QSharedDataPointer> #include <QVector> #include <QtDebug> class FooPrivate : public QSharedData { public: FooPrivate() { qDebug() << "FooPrivate default constr"; }; FooPrivate(const FooPrivate &other) : QSharedData(other) { qDebug() << "FooPrivate copy constr"; }; explicit FooPrivate(const QVector<int> &v) : v(v) { qDebug() << "FooPrivate constr from vector"; }; ~FooPrivate() { qDebug() << "FooPrivate destr"; }; QVector<int> 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<int> &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<int>::iterator begin() { qDebug() << "Foo begin" << this; return d->v.begin(); }; QVector<int>::const_iterator begin() const { qDebug() << "Foo begin const" << this; return d->v.begin(); }; QVector<int>::const_iterator cbegin() const { qDebug() << "Foo cbegin" << this; return d->v.cbegin(); }; QVector<int>::iterator end() { qDebug() << "Foo end" << this; return d->v.end(); }; QVector<int>::const_iterator end() const { qDebug() << "Foo end const" << this; return d->v.end(); }; QVector<int>::const_iterator cend() { qDebug() << "Foo cend" << this; return d->v.cend(); }; private: QSharedDataPointer<FooPrivate> d; }; template <typename T> const T moveToConst(T &&t) { return std::move(t); } Foo f() { return Foo({1, 2, 3}); } int main(int argc, char *argv[]) { Q_UNUSED(argc); Q_UNUSED(argv); qDebug() << "without moveToConst:"; for (auto v : f()) { Q_UNUSED(v); } qDebug() << ""; qDebug() << "with moveToConst:"; for (auto v : moveToConst(f())) { Q_UNUSED(v); } /* * Potential unsafe use suggested by Guiseppe D'Angelo * https://lists.qt-project.org/pipermail/development/2018-October/033808.html * * Gives: * * move-to-const-test.cpp:93:23: error: cannot bind non-const lvalue reference of type ‘Foo&’ to an rvalue of type ‘std::remove_reference<Foo&>::type {aka Foo}’ * return std::move(t); * ^ */ //Foo baz; //for (auto &v : moveToConst(baz)) { Q_UNUSED(v); }; /* * Potential unsafe use from https://stackoverflow.com/a/39051612/252857 * * Gives: * * move-to-const-test.cpp:118:42: error: cannot bind rvalue reference of type ‘const int&&’ to lvalue of type ‘const int’ * for (auto const &&v : moveToConst(f())) { Q_UNUSED(v); } // whoops! */ //for (auto const &&v : moveToConst(f())) { Q_UNUSED(v); } // whoops! return 0; } > > > My 2 c, > -- > Giuseppe D'Angelo | [email protected] | 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 > [email protected] > http://lists.qt-project.org/mailman/listinfo/development _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
