[Development] QMetaType and non-const references

[Thiago Macieira]

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