> On 24. Nov 2019, at 18:57, Olivier Goffart <[email protected]> wrote: > > 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)
_If_ you “deprecate all toXxx”, please keep qvariant_cast (without deprecation). If you also replace qvariant_cast with QVariant::cast, the proposed replacement for the deprecated methods is introduced late in the process, which makes porting harder, and as Olivier says, for no obvious reason. qvariant_cast also is consistent with the other “casts” in Qt and C++. > >>> 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 -- Eike Ziller Principal Software Engineer The Qt Company GmbH Erich-Thilo-Straße 10 D-12489 Berlin [email protected] http://qt.io Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B _______________________________________________ Development mailing list [email protected] https://lists.qt-project.org/listinfo/development
