On 24.11.19 12:36, Lars Knoll wrote:
Hi Olivier,
Thanks for looking through this and coming up with a proposal. I like the
direction.
On 22 Nov 2019, at 14:32, Olivier Goffart <[email protected]> wrote:
Hi,
This is a follow-up on what was discussed in the (second part of the) QtCore
session in the QtCS.
Lars and others have been mentioning that they dislike implicit conversions
within QVariant. Creating a new class (QAny) has been suggested, that would be
like QVariant but without the conversions.
I am personally not in favor of this change because we are using QVariant all
over the place in our API and so we cannot really deprecate QVariant. It will
cause much confusion to user to have two similar classes. And the difference is
not big enough to force a new class.
So here is what I suggest we do in Qt6. None of this is implemented yet, it is
only proposed on this list for feedback.
1. operator==
In Qt6, QVariant::operator==() will no longer do any conversions.
If two QVariant does not have exactly the same, they will no longer be
considered equal.
Also, in Qt6, QMetatype will gain ability to register the operator==, and
therefore it will work for any type (and not only for builtin type as
currently).
So right now,
QVariant(QByteArray("Hello")) == QVariant(QString("Hello"))
is true, but in Qt6 it will be false.
This is a behavior change, but I believe this is something we can afford to do.
I do not have data on how much code will break with this change, but i feel most
use of operator== are there for optimisations: i.e: setFoo(const QVariant
&foo) { if (m_foo == foo) return; ... }
Maybe we'll have more data once we actually implement the change and see if too
many things breaks.
This should be relatively uncontroversial. The current behaviour is at the very
least unexpected by users.
2. operator< and other comparison operator
Deprecate in Qt 5.15, remove in Qt 6
It is not possible to implement it correctly with a total order.
I could not find direct use of the operator in the code indexed on
https://code.woboq.org/qt5 (only in QmlDesigner::operator< which is itself not
used)
Sorting on variant does not really make sense. Even code that does, like
QAbstractItemModelPrivate::isVariantLessThan does not use operator<.
Where this is used may be the use of QVariant as a key in a QMap. This is
problematic because the operator< does not implement a total order, so you can
have funny results.
I could not find instances of that in Qt or QtCreator, but Github search for
"QMap<QVariant," shows many result :-(
I'd still want to deprecate it. User could wrap QVariant in their own
MySortedVariant with their own operators that does what they need for their use
case.
+1. Total ordering will only work in special cases, as we do not control the types of
data stored in a QVariant. Using a QMap<QVariant, …> sounds like a design
mistake in any case. Let’s deprecate and remove in Qt 6.
Deprecation MR: https://codereview.qt-project.org/282432
Other things came up, QMetaType::registerComparators also register the
operator<, but i suggest leaving it as it in 5.15 (as it is usefull to register
operator==) and make it a deprecated no-op in Qt6
There is also QMetaType::compare which has then to be removed in Qt6. I
couldn't find any use of this function on github that was not some automaticaly
generated bindings, or the tests within Qt itself. I guess it's Ok to deprecate
it in Qt 5.15 as well even if there is no replacement. Or would it be ok to
just remove it.
3. conversions in QVariant::value
We would like to avoid having automatic conversion in QVariant::value.
So Qt6 would be
std::optional<T> QVariant::value() const;
And we could deprecate the current one in Qt5.15 in favor of qvariant_cast
which is explicit.
This one is a bit more controversial maybe. Because there are thousands of call to
QVariant::value all over the place. But "value()" is the ideal name for the
non-converting variant.
A clazy script to replace QVariant::value with qvariant_cast will be in order.
I like this idea. value() is the perfect API to return the value of the variant
without implicit conversions. The advantage of the above approach is that it
offers a way to already migrate in Qt 5 and will give compile errors in Qt6 for
code that hasn’t been migrated to use qvariant_cast.
One more idea here: qvariant_cast was done as a free standing function because of
limitations in VC++ 6. We could also add a template<typename T>
QVariant::cast<T>() (or maybe to() or convertedTo()). IMO that would make the code
look a bit nicer than using the freestanding qvariant_cast method.
The VC++6 workaround was called qVariantValue (and qVariantFromValue,
qVariantSetValue). I think qvariant_cast was done on purpose independently.
Adding a cast<T> function does not sound like a bad idea? But should it be done
in Qt5.15 or Qt6. And should it return T or optional<T> (in case the conversion
did not work) or have a bool*ok=nullptr parameter?
4. All the implicit constructors for builtin types.
QVariant has many implicit constructors for all the builtin types.
I suggest to replace them all with a template<typename T> QVariant(T&&)
constructor. (same as std::any.) So builtin types are no longer special.
+1. This should be source compatible.
5. All the method toXxx (where Xxx is a builtin type)
Leave them as-is?
However some of them are for types that may go outside of QtCore, these should
be deprecated in Qt 5.15 and removed in Qt6
They do basically the same as qvariant_cast, so we could simply deprecate them
all and replace by the cast() method mentioned above.
The issue is that there is lots of use of these (esp. the most common ones like
toString and toInt.) Removing all uses be a huge work for no obvious reasons.
(And i was told they are now recommended by clazy)
6. QVariant::Type and QMetaType::Type enums
QVariant::Type is already marked as obsolete in the documentation, but not yet
marked as deprecated.
So we can remove it in Qt6, and we should try to mark it as deprecated in Qt
5.15. But that's hard because it is used all over the place.
QMetaType::Type will be marked as deprecated in Qt6, but i'm afraid we cannot
simply remove it.
In general, all the integer id API for QMetaType will be deprecated in Qt6,
one should use QMetaType by value. The integer id will stay in Qt6. This means
that there will still be a central registry of types but it would only be there
for the types for which we ask the id (and for the builtin types)
Sounds ok to me.
Cheers,
Lars
_______________________________________________
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development
