Hi, I think the most uncontroversial part is not treating T& as T; I fully agree with it. I think it is fine to reject them in QMetaType::fromType. It is a SC breaking change, but I expect it to only affect template code, which can rather easily fix it by explicitly using remove_ref. Can be done for 6.5, but I'd argue it should be done even for 6.4.
I also agree with changing QVariant to only handle types that are default constructible, copyalbe and destructible => https://codereview.qt-project.org/c/qt/qtbase/+/422249 when it gets a check for destructible. I think we do need a way to correctly represent non-const references in the metatype system for QMetaObject. We support references in connect (see tst_QObject::connectWithReference()), at least when using PMF syntax. If we want to properly handle this even when using QMetaMethods to connect, I expect that we need the ability to tell T& apart from T. That also can be used at runtime to give sensible warnings about things that are wonky, like creating a queued connection involving reference tpyes. If we simply were to use invalid metatypes, it would be hard to tell "type was incomplete, please include its header" apart from "you're using a reference here, don't do that". I'm in favour of allowing non-default-constructible, non-destructible and/or non-copyable types as metatypes. Well, I don't see a use case for non-destructible yet, but I can see uses-cases for supporting move-only types. Lastly, an aside: Q_DECLARE_METATYPE being effectively unnecessary was the goal for Qt 6, but is not the case yet. I consider that a bug that should finally be fixed in 6.5. Fabian ________________________________________ Von: Development <development-boun...@qt-project.org> im Auftrag von Thiago Macieira <thiago.macie...@intel.com> Gesendet: Dienstag, 19. Juli 2022 00:13 An: development@qt-project.org Betreff: [Development] QMetaType and non-const references Re: https://codereview.qt-project.org/c/qt/qtbase/+/422128 tl;dr: QMetaTypes currently allow T&, but the implementation looks b0rked. Need input from people who may want that (read: QML) to know what to do. I was doing some work on QMetaObject that spilled over to fixing some things in QMetaType. I realised that since 6.0 we *do* allow meta types for types that aren't default-constructible or copiable, which includes references. This is due to the rewrite by Olivier in 6.0 times, which effectively made Q_DECLARE_METATYPE a no-op (yes, you've read me right: the macro is unnecessary). But as a result we've introduced subtle issues where a non-const reference was used as a metatype where it maybe shouldn't (and QMetaType::fromType allowed it, which is what the change above fixes). For example, for the purposes of the metatype system -- as used in the signal- slot connection -- a const T& is interpreted as a T and normalizeType performs that change. But a T& is not and has never been the same as a T, and you can't make that connection with invokeMethod+Q_ARG either. In https://codereview.qt-project.org/c/qt/qtbase/+/422132, I'm making QMetaType::from<T&> explicitly fail. So the question to the dev community is what we should do to non-const references as metatypes. And as a side question, if we should support non- default-constructible, non-destructible and/or non-copyable types as metatypes in the first place, or reject them like Qt 4 & 5 did. My personal feeling is that we should: - reject them in QMetaType::fromType, since they're usually not what you wanted. See https://codereview.qt-project.org/c/qt/qtquick3d/+/422197 - reject non-default constructible, non-copyable or non-destructible types in QVariant but not QMetaType -- this includes non-const references => corollary: need a way to form them, but not QMetaType::fromType. - do use them in QMetaObject for reporting purposes - add a way to link a T& metatype back to T's However, please note I cannot guarantee all of the tasks above can be done in time for 6.5. I actually want some of the bug-fixing in 6.4, like the change at the top of this email. So what's mandatory as a unit? There's a question on what to do to rvalue references, but please address that separately, as we don't support them at all right now, not even for signal- slot connections in the new syntax I believe. The QMetaType code has accumulated a hairy mess of templates developed by different people at different times (Harry back in Qt 4 days when we supported pre-C++98 compilers, Stephen in early 5.x days for pre-C++11, additional work by some others for late 5.x with C++11, Olivier for 6.0 with C++17) to the point that I as the maintainer cannot follow what goes where and the sequence of SFINAE detections. Even after 6.0 some changes assumed that Q_DECLARE_METATYPE was required, when it isn't, so QMetaType behaves subtly different whether you use it or not. The code needs to be cleaned to the point that I understand it before I will allow any new feature work for it. -- Thiago Macieira - thiago.macieira (AT) intel.com Cloud Software Architect - Intel DCAI Cloud Engineering _______________________________________________ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development _______________________________________________ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
