[Development] QMetaType and non-const references

[Fabian Kosmale]

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-bounces at qt-project.org> im Auftrag von Thiago Macieira <thiago.macieira at intel.com>
Gesendet: Dienstag, 19. Juli 2022 00:13
An: development at 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 at qt-project.org
https://lists.qt-project.org/listinfo/development