Qt 5.7: rvalue references

[Erik Lundin]

When building for Debian stable which has Qt 5.7 (running the generator myself), I get errors like this:

moc_com_trolltech_qt_gui_builtin0.cpp:152:134: error: call of overloaded ‘operator_assign(QBitmap*&, QBitmap&)’ is ambiguous
         case 11: { QBitmap* _r = _t->operator_assign((*reinterpret_cast< QBitmap*(*)>(_a[1])),(*reinterpret_cast< QBitmap(*)>(_a[2])));
                                                                                                                                      ^
In file included from moc_com_trolltech_qt_gui_builtin0.cpp:9:0:
../../generated_cpp/com_trolltech_qt_gui_builtin/com_trolltech_qt_gui_builtin0.h:83:14: note: candidate: QBitmap* PythonQtWrapper_QBitmap::operator_assign(QBitmap*, QBitmap)
    QBitmap*  operator_assign(QBitmap* theWrappedObject, QBitmap  arg__1);
              ^~~~~~~~~~~~~~~
../../generated_cpp/com_trolltech_qt_gui_builtin/com_trolltech_qt_gui_builtin0.h:84:14: note: candidate: QBitmap* PythonQtWrapper_QBitmap::operator_assign(QBitmap*, const QBitmap&)
    QBitmap*  operator_assign(QBitmap* theWrappedObject, const QBitmap&  other);
              ^~~~~~~~~~~~~~~

The reason is that the QBitmap arg__1 argument actually should be QBitmap&& other, but the generator currently doesn't handle rvalue references.

I have added support for rvalue references in the generator based on commits (f7fb80ae, 80fef170 and 454b2eef) by Friedemann Kleint in the pyside-shiboken project (took me all summer...). The changes are found in handle-rvalue-refs.diff. With this patch applied the generated code looks fine, but compiling still fails:

moc_com_trolltech_qt_gui_builtin0.cpp:4103:96: error: cannot bind ‘QMatrix’ lvalue to ‘QMatrix&&’
         case 31: { QMatrix* _r = _t->operator_assign((*reinterpret_cast< QMatrix*(*)>(_a[1])),(*reinterpret_cast< QMatrix(*)>(_a[2])));
                                                                                               ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from moc_com_trolltech_qt_gui_builtin0.cpp:9:0:
../../generated_cpp/com_trolltech_qt_gui_builtin/com_trolltech_qt_gui_builtin0.h:689:14: note:   initializing argument 2 of ‘QMatrix* PythonQtWrapper_QMatrix::operator_assign(QMatrix*, QMatrix&&)’
    QMatrix*  operator_assign(QMatrix* theWrappedObject, QMatrix&&  other);
              ^~~~~~~~~~~~~~~

I haven't managed to understand what's the problem here, but probably the wrapping made by PythonQt is not working out really, so for now my suggestion is to simply skip functions with rvalue reference arguments, see ignore-functions-with-rvalue-ref-args.diff. Applying this patch in addition to the first one makes the compilation work for me.

[Erik Lundin]

Here are the patches (the ones ending with "unix" have unix line endings, the other ones have Windows line endings.

[Florian Link]

I will have a look, it might take a while. I guess rvalues don't make sense together with qmake slots.

[Erik Lundin]

Rvalue references seem to be used for move assignment operators etc., like here for QBitmap: http://doc.qt.io/qt-5/qbitmap.html#operator-eq-1

[Florian Link]

Right, but they don't make sense from Python wrappers, so it's fine to ignore the,.

[Florian Link]

I am sorry, I can't take that patch because it is taken from PySide and thus copyrighted by the LGPL v3 while PythonQt is LGPL v2.1.

I think rvalue references don't need wrapping, so the best approach will be to remove the functions containing rvalue refs as early as possible.

[Erik Lundin]

Oh, that's unfortunate. I'll have a look at it, trying to come up with something. I guess long-term it would be nice to use some more recent version of the generator that's used by other projects and easier to maintain?

[Florian Link]

I think PySide considers moving to a clang based parser/generator since it is really hard to maintain a cpp parser. So in the long run, we might switch to a different generator. Right now, the licencing is in the way...

I think all rvalues are inside of

Q_COMPILER_RVALUE_REFS

and I ask myself why the generator makes the Qt headers think that is supports rvalues.

If we manage to undefine Q_COMPILER_RVALUE_REFS or to trick
http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/global/qcompilerdetection.h
into thinking that the generator does not support rvalues, then they should be gone.

I did not have these problems with 5.7 on Windows, maybe unsetting the INCLUDE env variable helps?

[Erik Lundin]

I'm not sure how Q_COMPILER_RVALUE_REFS is meant to be used. Not all rvalue references in the headers are checked for that. This is an excerpt from the definition of the class QRegion in QtGui/qregion.h:

    QRegion(QRegion &&other) Q_DECL_NOTHROW
        : d(other.d) { other.d = const_cast<QRegionData*>(&shared_empty); }
    QRegion(const QBitmap &bitmap);
    ~QRegion();
    QRegion &operator=(const QRegion &);
#ifdef Q_COMPILER_RVALUE_REFS
    inline QRegion &operator=(QRegion &&other) Q_DECL_NOEXCEPT
    { qSwap(d, other.d); return *this; }
#endif

The other places causing problems (none of them within Q_COMPILER_RVALUE_REFS ifdefs):

// QtGui/qbitmap.h (in definition of class QBitmap):
    QBitmap &operator=(QBitmap &&other) Q_DECL_NOTHROW { QPixmap::operator=(std::move(other)); return *this; }

// QtGui/qmatrix.h (in definition of class QMatrix):
    QMatrix &operator=(QMatrix &&other) Q_DECL_NOTHROW // = default
    { memcpy(this, &other, sizeof(QMatrix)); return *this; }

// QtGui/qtransform.h (in definition of class QTransform):
    QTransform(QTransform &&other) Q_DECL_NOTHROW // = default
        : affine(Qt::Uninitialized)
    { memcpy(this, &other, sizeof(QTransform)); }

[Florian Link]

I see, so the define is no option.

[Erik Lundin]

I have now made a new implementation that's focused on skipping functions with rvalue reference arguments as early as possible. The filtering is now made while building the abstract syntax tree. The code is written by me, but for example in the parser, it's difficult not to make it similar to other's implementations, because there are not so many ways to do certain things.

[Florian Link]

Thanks, I will have a look on Monday.

[Florian Link]

Thanks for your effort, I integrated the patch into svn trunk.

[Erik Lundin]

Great, thanks! This was an important step towards a build that works for the folks at Debian. :)