[Development] qMoveToConst helper for rvalue references to movable Qt containers?

[Elvis Stansvik]

Den lör 20 okt. 2018 kl 18:53 skrev Giuseppe D'Angelo via Development
<development at qt-project.org>:
>
> 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 at 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 at 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 | giuseppe.dangelo at 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 at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development