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. > > > 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. > > > 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. > > > 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
