moc: Allow NOTIFY signals defined in parent classes

Limitation is that the signal needs to be parameter-less

[ChangeLog][moc] moc now supports NOTIFY signals of parent classes in Q_PROPERTY

Change-Id: Iad64c96c3ec65d4be8ad9ff1a9f889938ab9bf45
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
Reviewed-by: Brett Stottlemyer <bstottle@ford.com>

9 files changed, 161 insertions, 15 deletions

diff --git a/src/corelib/kernel/qmetaobject.cpp b/src/corelib/kernel/qmetaobject.cpp
index cdc605d33b..f07b463482 100644
--- a/src/corelib/kernel/qmetaobject.cpp
+++ b/src/corelib/kernel/qmetaobject.cpp
@@ -3347,7 +3347,21 @@ int QMetaProperty::notifySignalIndex() const
     if (hasNotifySignal()) {
         int offset = priv(mobj->d.data)->propertyData +
                      priv(mobj->d.data)->propertyCount * 3 + idx;
-        return mobj->d.data[offset] + mobj->methodOffset();
+        int methodIndex = mobj->d.data[offset];
+        if (methodIndex & IsUnresolvedSignal) {
+            methodIndex &= ~IsUnresolvedSignal;
+            const QByteArray signalName = stringData(mobj, methodIndex);
+            const QMetaObject *m = mobj;
+            const int idx = indexOfMethodRelative<MethodSignal>(&m, signalName, 0, nullptr);
+            if (idx >= 0) {
+                return idx + m->methodOffset();
+            } else {
+                qWarning("QMetaProperty::notifySignal: cannot find the NOTIFY signal %s in class %s for property '%s'",
+                         signalName.constData(), objectClassName(mobj), name());
+                return -1;
+            }
+        }
+        return methodIndex + mobj->methodOffset();
     } else {
         return -1;
     }
diff --git a/src/corelib/kernel/qmetaobject_p.h b/src/corelib/kernel/qmetaobject_p.h
index e247c48703..434ef84808 100644
--- a/src/corelib/kernel/qmetaobject_p.h
+++ b/src/corelib/kernel/qmetaobject_p.h
@@ -111,7 +111,8 @@ enum MetaObjectFlags {
 
 enum MetaDataFlags {
     IsUnresolvedType = 0x80000000,
-    TypeNameIndexMask = 0x7FFFFFFF
+    TypeNameIndexMask = 0x7FFFFFFF,
+    IsUnresolvedSignal = 0x70000000
 };
 
 enum EnumFlags {
diff --git a/src/tools/moc/generator.cpp b/src/tools/moc/generator.cpp
index c4184929ef..0b45776b88 100644
--- a/src/tools/moc/generator.cpp
+++ b/src/tools/moc/generator.cpp
@@ -218,6 +218,7 @@ void Generator::generateCode()
     registerFunctionStrings(cdef->slotList);
     registerFunctionStrings(cdef->methodList);
     registerFunctionStrings(cdef->constructorList);
+    registerByteArrayVector(cdef->nonClassSignalList);
     registerPropertyStrings();
     registerEnumStrings();
 
@@ -603,6 +604,19 @@ void Generator::generateCode()
 // Generate plugin meta data
 //
     generatePluginMetaData();
+
+//
+// Generate function to make sure the non-class signals exist in the parent classes
+//
+    if (!cdef->nonClassSignalList.isEmpty()) {
+        fprintf(out, "// If you get a compile error in this function it can be because either\n");
+        fprintf(out, "//     a) You are using a NOTIFY signal that does not exist. Fix it.\n");
+        fprintf(out, "//     b) You are using a NOTIFY signal that does exist (in a parent class) but has a non-empty parameter list. This is a moc limitation.\n");
+        fprintf(out, "Q_DECL_UNUSED static void checkNotifySignalValidity_%s(%s *t) {\n", qualifiedClassNameIdentifier.constData(), cdef->qualified.constData());
+        for (const QByteArray &nonClassSignal : cdef->nonClassSignalList)
+            fprintf(out, "    t->%s();\n", nonClassSignal.constData());
+        fprintf(out, "}\n");
+    }
 }
 
 
@@ -648,6 +662,12 @@ void Generator::registerFunctionStrings(const QVector<FunctionDef>& list)
     }
 }
 
+void Generator::registerByteArrayVector(const QVector<QByteArray> &list)
+{
+    for (const QByteArray &ba : list)
+        strreg(ba);
+}
+
 void Generator::generateFunctions(const QVector<FunctionDef>& list, const char *functype, int type, int &paramsIndex)
 {
     if (list.isEmpty())
@@ -841,12 +861,17 @@ void Generator::generateProperties()
         fprintf(out, "\n // properties: notify_signal_id\n");
         for (int i = 0; i < cdef->propertyList.count(); ++i) {
             const PropertyDef &p = cdef->propertyList.at(i);
-            if(p.notifyId == -1)
+            if (p.notifyId == -1) {
                 fprintf(out, "    %4d,\n",
                         0);
-            else
+            } else if (p.notifyId > -1) {
                 fprintf(out, "    %4d,\n",
                         p.notifyId);
+            } else {
+                const int indexInStrings = strings.indexOf(p.notify);
+                fprintf(out, "    %4d,\n",
+                        indexInStrings | IsUnresolvedSignal);
+            }
         }
     }
     if (cdef->revisionedProperties) {
@@ -1401,13 +1426,15 @@ void Generator::generateStaticMetacall()
                             prefix.constData(), p.member.constData(), p.type.constData());
                     fprintf(out, "                %s%s = *reinterpret_cast< %s*>(_v);\n",
                             prefix.constData(), p.member.constData(), p.type.constData());
-                    if (!p.notify.isEmpty() && p.notifyId != -1) {
+                    if (!p.notify.isEmpty() && p.notifyId > -1) {
                         const FunctionDef &f = cdef->signalList.at(p.notifyId);
                         if (f.arguments.size() == 0)
                             fprintf(out, "                Q_EMIT _t->%s();\n", p.notify.constData());
                         else if (f.arguments.size() == 1 && f.arguments.at(0).normalizedType == p.type)
                             fprintf(out, "                Q_EMIT _t->%s(%s%s);\n",
                                     p.notify.constData(), prefix.constData(), p.member.constData());
+                    } else if (!p.notify.isEmpty() && p.notifyId < -1) {
+                        fprintf(out, "                Q_EMIT _t->%s();\n", p.notify.constData());
                     }
                     fprintf(out, "            }\n");
                     fprintf(out, "            break;\n");
diff --git a/src/tools/moc/generator.h b/src/tools/moc/generator.h
index 3833148fb3..8b80138302 100644
--- a/src/tools/moc/generator.h
+++ b/src/tools/moc/generator.h
@@ -46,6 +46,7 @@ private:
     void registerClassInfoStrings();
     void generateClassInfos();
     void registerFunctionStrings(const QVector<FunctionDef> &list);
+    void registerByteArrayVector(const QVector<QByteArray> &list);
     void generateFunctions(const QVector<FunctionDef> &list, const char *functype, int type, int &paramsIndex);
     void generateFunctionRevisions(const QVector<FunctionDef> &list, const char *functype);
     void generateFunctionParameters(const QVector<FunctionDef> &list, const char *functype);
diff --git a/src/tools/moc/moc.cpp b/src/tools/moc/moc.cpp
index 36d84a61d8..f7de57fae3 100644
--- a/src/tools/moc/moc.cpp
+++ b/src/tools/moc/moc.cpp
@@ -1727,9 +1727,13 @@ void Moc::checkProperties(ClassDef *cdef)
             }
             p.notifyId = notifyId;
             if (notifyId == -1) {
-                QByteArray msg = "NOTIFY signal '" + p.notify + "' of property '" + p.name
-                        + "' does not exist in class " + cdef->classname + ".";
-                error(msg.constData());
+                int index = cdef->nonClassSignalList.indexOf(p.notify);
+                if (index == -1) {
+                    cdef->nonClassSignalList << p.notify;
+                    p.notifyId = -1 - cdef->nonClassSignalList.count();
+                } else {
+                    p.notifyId = -2 - index;
+                }
             }
         }
     }
diff --git a/src/tools/moc/moc.h b/src/tools/moc/moc.h
index 6040f944f3..5f8cdfcf2c 100644
--- a/src/tools/moc/moc.h
+++ b/src/tools/moc/moc.h
@@ -121,7 +121,7 @@ struct PropertyDef
 {
     PropertyDef():notifyId(-1), constant(false), final(false), gspec(ValueSpec), revision(0){}
     QByteArray name, type, member, read, write, reset, designable, scriptable, editable, stored, user, notify, inPrivateClass;
-    int notifyId;
+    int notifyId; // -1 means no notifyId, >= 0 means signal defined in this class, < -1 means signal not defined in this class
     bool constant;
     bool final;
     enum Specification  { ValueSpec, ReferenceSpec, PointerSpec };
@@ -179,6 +179,7 @@ struct ClassDef : BaseDef {
 
     QVector<FunctionDef> constructorList;
     QVector<FunctionDef> signalList, slotList, methodList, publicList;
+    QVector<QByteArray> nonClassSignalList;
     int notifyableProperties = 0;
     QVector<PropertyDef> propertyList;
     int revisionedMethods = 0;
diff --git a/tests/auto/corelib/kernel/qmetaobject/tst_qmetaobject.cpp b/tests/auto/corelib/kernel/qmetaobject/tst_qmetaobject.cpp
index c884edde04..35e14b7dbf 100644
--- a/tests/auto/corelib/kernel/qmetaobject/tst_qmetaobject.cpp
+++ b/tests/auto/corelib/kernel/qmetaobject/tst_qmetaobject.cpp
@@ -147,6 +147,77 @@ namespace MyNamespace {
     {
         Q_OBJECT
     };
+
+    class ClassWithSetterGetterSignals : public QObject
+    {
+        Q_OBJECT
+
+        public:
+            int value1() const { return m_value1; }
+            void setValue1(int v) {
+                if (v != m_value1) {
+                    m_value1 = v;
+                    Q_EMIT value1Changed();
+                }
+            }
+
+            int value2() const { return m_value2; }
+            void setValue2(int v) {
+                if (v != m_value2) {
+                    m_value2 = v;
+                    Q_EMIT value2Changed();
+                }
+            }
+
+        Q_SIGNALS:
+            void value1Changed();
+            void value2Changed();
+
+        private:
+            int m_value1 = 0;
+            int m_value2 = 0;
+    };
+
+    class ClassWithSetterGetterSignalsAddsProperties : public ClassWithSetterGetterSignals
+    {
+        Q_OBJECT
+        Q_PROPERTY(int value1 READ value1 WRITE setValue1 NOTIFY value1Changed)
+        Q_PROPERTY(int value2 READ value2 WRITE setValue2 NOTIFY value2Changed)
+    };
+
+    class ClassWithChangedSignal : public QObject
+    {
+        Q_OBJECT
+
+        public:
+            int value1() const { return m_value1; }
+            void setValue1(int v) {
+                if (v != m_value1) {
+                    m_value1 = v;
+                    Q_EMIT propertiesChanged();
+                }
+            }
+
+            void thisIsNotASignal() { }
+
+        Q_SIGNALS:
+            void propertiesChanged();
+
+        private:
+            int m_value1 = 0;
+    };
+
+    class ClassWithChangedSignalNewValue : public ClassWithChangedSignal
+    {
+        Q_OBJECT
+
+        Q_PROPERTY(int value2 MEMBER m_value2 NOTIFY propertiesChanged)
+        Q_PROPERTY(int value3 MEMBER m_value3 NOTIFY thisIsNotASignal)
+
+        private:
+            int m_value2 = 0;
+            int m_value3 = 0;
+    };
 }
 
 
@@ -245,6 +316,8 @@ private slots:
     void inherits_data();
     void inherits();
 
+    void notifySignalsInParentClass();
+
 signals:
     void value6Changed();
     void value7Changed(const QString &);
@@ -1672,5 +1745,18 @@ void tst_QMetaObject::inherits()
     QCOMPARE(derivedMetaObject->inherits(baseMetaObject), inheritsResult);
 }
 
+void tst_QMetaObject::notifySignalsInParentClass()
+{
+    MyNamespace::ClassWithSetterGetterSignalsAddsProperties obj;
+    QCOMPARE(obj.metaObject()->property(obj.metaObject()->indexOfProperty("value1")).notifySignal().name(), QByteArray("value1Changed"));
+    QCOMPARE(obj.metaObject()->property(obj.metaObject()->indexOfProperty("value2")).notifySignal().name(), QByteArray("value2Changed"));
+
+    MyNamespace::ClassWithChangedSignalNewValue obj2;
+    QCOMPARE(obj2.metaObject()->property(obj2.metaObject()->indexOfProperty("value2")).notifySignal().name(), QByteArray("propertiesChanged"));
+
+    QTest::ignoreMessage(QtWarningMsg, "QMetaProperty::notifySignal: cannot find the NOTIFY signal thisIsNotASignal in class MyNamespace::ClassWithChangedSignalNewValue for property 'value3'");
+    obj2.metaObject()->property(obj2.metaObject()->indexOfProperty("value3")).notifySignal();
+}
+
 QTEST_MAIN(tst_QMetaObject)
 #include "tst_qmetaobject.moc"
diff --git a/tests/auto/tools/moc/error-on-wrong-notify.h b/tests/auto/tools/moc/error-on-wrong-notify.h
index 11f5e954db..9e6b27bff3 100644
--- a/tests/auto/tools/moc/error-on-wrong-notify.h
+++ b/tests/auto/tools/moc/error-on-wrong-notify.h
@@ -28,7 +28,7 @@
 
 #ifndef ERROR_ON_WRONG_NOTIFY_H
 #define ERROR_ON_WRONG_NOTIFY_H
-#include <QObject>
+#include <QtCore/QObject>
 
 class ClassWithWrongNOTIFY : public QObject
 {
diff --git a/tests/auto/tools/moc/tst_moc.cpp b/tests/auto/tools/moc/tst_moc.cpp
index 8189227cbe..55e8054a70 100644
--- a/tests/auto/tools/moc/tst_moc.cpp
+++ b/tests/auto/tools/moc/tst_moc.cpp
@@ -1830,13 +1830,25 @@ void tst_Moc::notifyError()
     const QString header = m_sourceDirectory + QStringLiteral("/error-on-wrong-notify.h");
     proc.start(m_moc, QStringList(header));
     QVERIFY(proc.waitForFinished());
-    QCOMPARE(proc.exitCode(), 1);
+    QCOMPARE(proc.exitCode(), 0);
     QCOMPARE(proc.exitStatus(), QProcess::NormalExit);
     QByteArray mocOut = proc.readAllStandardOutput();
-    QVERIFY(mocOut.isEmpty());
-    QString mocError = QString::fromLocal8Bit(proc.readAllStandardError());
-    QCOMPARE(mocError, header +
-        QString(":42: Error: NOTIFY signal 'fooChanged' of property 'foo' does not exist in class ClassWithWrongNOTIFY.\n"));
+    QVERIFY(!mocOut.isEmpty());
+    QCOMPARE(proc.readAllStandardError(), QByteArray());
+
+    QStringList args;
+    args << "-c" << "-x" << "c++" << "-I" << "."
+         << "-I" << qtIncludePath << "-o" << "/dev/null" << "-fPIC" << "-std=c++11" << "-";
+    proc.start("gcc", args);
+    QVERIFY(proc.waitForStarted());
+    proc.write(mocOut);
+    proc.closeWriteChannel();
+
+    QVERIFY(proc.waitForFinished());
+    QCOMPARE(proc.exitCode(), 1);
+    const QString gccOutput = QString::fromLocal8Bit(proc.readAllStandardError());
+    QVERIFY(gccOutput.contains(QLatin1String("error")));
+    QVERIFY(gccOutput.contains(QLatin1String("fooChanged")));
 #else
     QSKIP("Only tested on linux/gcc");
 #endif