Fix regression in connectNotify(const char *) emission

Reimplementations of connectNotify() and disconnectNotify() can
assume that the signal argument is in normalized form, but after the
introduction of the Qt5 meta-object format, it could happen that it's
not.

The problem is that the internal QArgumentType class, which attempts
to resolve a typename to a type id, was calling QMetaType::type().
QMetaType::type() falls back to trying the normalized form of the
typename if the original argument can't be resolved as a type (this
behavior isn't documented, but that's how it works). This means that
e.g. QMetaType::type("const QString &") returns QMetaType::QString.

Since QMetaObjectPrivate::indexOfMethodRelative() (more specifically,
the methodMatch() helper function) prefers to compare type ids
over typenames (since the type ids are stored directly in the meta-
object data for built-in types), the method lookup would *succeed*
for signatures with non-normalized built-in typenames as parameters.
QObject::connect() would then think that it did not have to
normalize the signature (see "// check for normalized signatures").
The consequence was that the original, non-normalized form got
passed to connectNotify().

This commit introduces an internal typename-to-type function that
is the same as QMetaType::type(), except it doesn't try to normalize
the name. This way, the only place where normalization can occur in
the signature-to-meta-method processing is through the calls to
QMetaObject::normalizedSignature() in QObject::connect() itself.

The implication is that there are now cases where the method
signature will be decoded and processed twice, where processing it
once was sufficient before. On the other hand, it is consistent with
the pre-Qt5-meta-object behavior, where we predict that the
signature is already normalized, and only perform (comparatively
costly) normalization if the initial lookup fails.

Change-Id: Ie6b60f60b0f9a57ebd378d980329dac62d57bbd9
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@nokia.com>

diff --git a/src/corelib/kernel/qmetaobject.cpp b/src/corelib/kernel/qmetaobject.cpp
index a8639e2..4e05b63 100644 (file)
--- a/src/corelib/kernel/qmetaobject.cpp
+++ b/src/corelib/kernel/qmetaobject.cpp
@@ -652,6 +652,7 @@ int QMetaObject::indexOfMethod(const char *method) const
 }
 
 // Parses a string of comma-separated types into QArgumentTypes.
+// No normalization of the type names is performed.
 static void argumentTypesFromString(const char *str, const char *end,
                                     QArgumentTypeArray &types)
 {

diff --git a/src/corelib/kernel/qmetaobject_p.h b/src/corelib/kernel/qmetaobject_p.h
index e78a920..5877ab1 100644 (file)
--- a/src/corelib/kernel/qmetaobject_p.h
+++ b/src/corelib/kernel/qmetaobject_p.h
@@ -110,6 +110,8 @@ enum MetaDataFlags {
     TypeNameIndexMask = 0x7FFFFFFF
 };
 
+extern int qMetaTypeTypeInternal(const char *);
+
 class QArgumentType
 {
 public:
@@ -117,7 +119,7 @@ public:
         : _type(type)
     {}
     QArgumentType(const QByteArray &name)
-        : _type(QMetaType::type(name.constData())), _name(name)
+        : _type(qMetaTypeTypeInternal(name.constData())), _name(name)
     {}
     QArgumentType()
         : _type(0)

diff --git a/src/corelib/kernel/qmetatype.cpp b/src/corelib/kernel/qmetatype.cpp
index 5ff8281..809d3bf 100644 (file)
--- a/src/corelib/kernel/qmetatype.cpp
+++ b/src/corelib/kernel/qmetatype.cpp
@@ -606,26 +606,26 @@ bool QMetaType::isRegistered(int type)
 }
 
 /*!
-    Returns a handle to the type called \a typeName, or QMetaType::UnknownType if there is
-    no such type.
+    \internal
 
-    \sa isRegistered(), typeName(), Type
+    Implementation of QMetaType::type().
 */
-int QMetaType::type(const char *typeName)
+template <int tryNormalizedType>
+static inline int qMetaTypeTypeImpl(const char *typeName)
 {
     int length = qstrlen(typeName);
     if (!length)
-        return UnknownType;
+        return QMetaType::UnknownType;
     int type = qMetaTypeStaticType(typeName, length);
-    if (type == UnknownType) {
+    if (type == QMetaType::UnknownType) {
         QReadLocker locker(customTypesLock());
         type = qMetaTypeCustomType_unlocked(typeName, length);
 #ifndef QT_NO_QOBJECT
-        if (type == UnknownType) {
+        if ((type == QMetaType::UnknownType) && tryNormalizedType) {
             const NS(QByteArray) normalizedTypeName = QMetaObject::normalizedType(typeName);
             type = qMetaTypeStaticType(normalizedTypeName.constData(),
                                        normalizedTypeName.size());
-            if (type == UnknownType) {
+            if (type == QMetaType::UnknownType) {
                 type = qMetaTypeCustomType_unlocked(normalizedTypeName.constData(),
                                                     normalizedTypeName.size());
             }
@@ -635,6 +635,29 @@ int QMetaType::type(const char *typeName)
     return type;
 }
 
+/*!
+    Returns a handle to the type called \a typeName, or QMetaType::UnknownType if there is
+    no such type.
+
+    \sa isRegistered(), typeName(), Type
+*/
+int QMetaType::type(const char *typeName)
+{
+    return qMetaTypeTypeImpl</*tryNormalizedType=*/true>(typeName);
+}
+
+/*!
+    \a internal
+
+    Similar to QMetaType::type(); the only difference is that this function
+    doesn't attempt to normalize the type name (i.e., the lookup will fail
+    for type names in non-normalized form).
+*/
+int qMetaTypeTypeInternal(const char *typeName)
+{
+    return qMetaTypeTypeImpl</*tryNormalizedType=*/false>(typeName);
+}
+
 #ifndef QT_NO_DATASTREAM
 /*!
     Writes the object pointed to by \a data with the ID \a type to

diff --git a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
index fefec56..60b92fc 100644 (file)
--- a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
+++ b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
@@ -172,6 +172,7 @@ signals:
     void signal4();
     QT_MOC_COMPAT void signal5();
     void signal6(void);
+    void signal7(int, const QString &);
 
 public slots:
     void aPublicSlot() { aPublicSlotCalled++; }
@@ -833,6 +834,7 @@ void tst_QObject::connectDisconnectNotify_data()
     QTest::newRow("combo4") << SIGNAL(  signal4( void )  )<< SLOT(  slot4( void )  );
     QTest::newRow("combo5") << SIGNAL( signal6( void ) )  << SLOT( slot4() );
     QTest::newRow("combo6") << SIGNAL( signal6() )        << SLOT( slot4() );
+    QTest::newRow("combo7") << SIGNAL( signal7( int , const QString & ) ) << SLOT( slot4() );
 }
 
 void tst_QObject::connectDisconnectNotify()