[Development] Qt 5.5.0 header diff: QtCore.diff

[Olivier Goffart]

On Monday 8. June 2015 17:57:23 Thiago Macieira wrote:
> On Monday 08 June 2015 15:58:23 Olivier Goffart wrote:
> > > > There is no reason to stop improving qmetatype.
> > > 
> > > The qFatal was there for a good reason.
> > 
> > It was there for a good reason for the existing flags.
> > But for new flags of course it does not make sens.
> 
> It did make sense: the idea was that registering new flags would cause the
> very incompatibility we're seeing here. I'm not entirely convinced that we
> discussed all scenarios at QtCS, so I'm still skeptical about allowing the
> IsGadget flag. I insist that we \omitvalue for now, until we understand the
> consequences better.

No, the qFatal is there because a change in the other flags like the 
MovableType is binary incompatible.
A change in the IsGadget flag is fine.

Old code did not need this flag. And code that relies on it requires anyway 
that the code that register the types register the isGadget flag.

Even if one could build a complicated use case that breaks, it would be very 
unlikely to happen.
And any change is dangerous. When we fix any small bug, there can always be 
applications that breaks because it was relying on the bug for something.


Anyway, let us summarize the "problem" in that case.

In Qt <= 5.4, QtLocation was using QML Private API to basically creates 
QObject wrapper around QGeoCoordinate so it can be exposed in QML. But in Qt 
5.5, one does not need private API, everybody can expose value types to QML 
just by adding Q_GADGET to it and registering Q_PROPERTY or Q_INVOKABLE.
So QGeoCoordinate became a Q_GADGET.  But for QML to be able to take advantage 
of it, the type needs to be registered with the QMetaType::IsGadget flag. This 
is automatic. But if the metatype is registered by code compiled with Qt 5.4 
or before, the IsGadget flag is not present.
This is what is happening in a application that was compiled against Qt 5.4,  
QGeoCoordinate was used in signal and slot and registered as a metatype by the 
application. QMetaType will detect the difference in the flags on the second 
registration and do a qFatal. 
The solution is obviously not to do a qFatal, but take the new flags in 
addition. And then everything works as expected.

Now the problem is: could there be code that relies in IsGadget that is run 
and that the type is only registered by "old" code but not by new code. i.e: 
could loading an old plugin compiled with an old code suddenly break a new 
application using these new feature. 
In theory this is possible, but in practice I don't think this will ever 
happen. 

Let's suppose an application called App 1.0 which has this code:

mystruct.h:
 struct MyStruct {
    int myValue;
 };
 Q_DECLARE_METATYPE(MyStruct);

myitem_p.h
 class MyItem : public QObject {
   Q_OBJECT
   Q_INVOKABLE extractValue(const QVariant &v) {
      return qvariant_cast<MyStruct>(v).myValue; 
   } 
 };
 
foo.qml:
 MyItem {
   function foobar(someObject) {
      return extractValue(someObject.myStructProperty);
   }
 }

someObject is an object coming from a plugin which has a property 
myStructProperty of type MyStruct.

Now, App 1.1 gets released and they simplify the code:

mystruct.h:
 struct MyStruct {
    Q_GADGET;
    Q_PROPERTY(int myValue MEMBER myValue)
  public:
    int myValue;
 };
 
foo.qml:
 MyItem {
   function foobar(someObject) {
      // now that myStruct is a Q_GADGET i don't need the helper
      return someObject.myStructProperty.myValue;
   }
 }

Then yes, if the MyStruct was only registered by the plugin we get a binary 
compatibility problem in the application.  But this is a problem for the 
application, not Qt. And the solution is to make sure that MyStruct is 
registered by manually registering it using qRegisterMetaType
 
> And it's not just the flag. I'm not convinced about the template detection
> either. You had to apply two late fixes to the detection so that we wouldn't
> break source compatibility or create unnecessary warnings.

Yes, I had to apply fixes after the beta was released and it got tested in the 
wild. But is that not what beta releases are for?

> > > The freeze stays: no new flags in QMetaType until Qt 6, no more messing
> > > with the template black magic.
> > 
> > You can't mandate that.
> 
> Yes, I can. As the maintainer, I have the authority and mandate to oversee
> the changes to the module I maintain and that includes blocking changes I
> am unsatisfied with.
> 
> A mailing list consensus can overrule me, as can the Chief Maintainer.
> 
> We stay frozen until further notice. If you have new flags you want to
> propose, you can do it, but we'll need a mailing list discussion before the
> change is allowed.

Well, then it will be reviewed as usual.
There are few improvements that can be done to QMetaType that we were 
discussing at the summit, like the ability to modify list types (append and 
such) or including some of the C++11 features. I bet one can simplify 
qmetatype.h when we require C++11's decltype and proper SFINAE rules