On Tuesday, 19 July 2022 00:34:34 PDT Fabian Kosmale wrote: > 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.
Thanks. > 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 may want to add QMetaType::is{Default,Copy,Move}Constructible and isDestructible(), because it's not obvious how they work. The current flags don't match documentation, I might say they are plain *wrong* (NeedsConstruction checks if the type is not trivial...) and therefore any code currently using them is not working properly anyway for some corner cases. isRegistered() is also wrong. > 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". The PMF syntax is entirely separate from the non-PMF syntax. Meanwhile, the old syntax only does a string comparison (for the most part), so it will mach Foo& to Foo&. I don't specifically think we need to insert metatype comparisons in case Foo& is actually Namespace::Bar&, but if it is doable, sure. methodMatch() in qmetaobject.cpp is another hairy mess... But I agree on the minimum of reporting. We can use the QMetaTypeInterface structure for this. That was my plan. > 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. We already do, but the flags are wrong, see above and my reply to Ulf. > 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. Right. There should be exactly one use for Q_DECLARE_METATYPE: when the name you want to register is not the name that the C++ language recognises for your type, and this should only be done for legacy reasons. typedef QMap<int, int> IntMap; Q_DECLARE_METATYPE(IntMap); And you may have the string "IntMap" somewhere in your meta objects. Any opinion on what must happen together or not at all? -- 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
