From 838f66be51103b8567ee22b71bd8e6102297b681 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Mon, 13 Mar 2023 17:59:35 +0100
Subject: [PATCH] moc: Handle attributes after meta-method tag

We so far only handled them if they came at the very start of the method
declaration.
This patch ensures that we also handle them after the meta-method tag
(but before the actual type).
Unifying parseFunction and parseMaybeFunction to avoid the need to
munally keep them in sync is left for another day.

Fixes: QTBUG-111330
Change-Id: Ic94edb69f04b9150aea2c8e6d004a8b9e5cf12ec
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 44b5ad01f0da55a351e0855e1173acfbef77221d)
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
---
 src/tools/moc/moc.cpp            | 21 +++++++++++++++++----
 src/tools/moc/moc.h              |  1 +
 tests/auto/tools/moc/tst_moc.cpp | 28 ++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/src/tools/moc/moc.cpp b/src/tools/moc/moc.cpp
index d4973b87ac6..09bc1e26a10 100644
--- a/src/tools/moc/moc.cpp
+++ b/src/tools/moc/moc.cpp
@@ -409,8 +409,7 @@ bool Moc::parseFunction(FunctionDef *def, bool inMacro)
     def->isVirtual = false;
     def->isStatic = false;
     //skip modifiers and attributes
-    while (test(INLINE) || (test(STATIC) && (def->isStatic = true) == true) ||
-        (test(VIRTUAL) && (def->isVirtual = true) == true) //mark as virtual
+    while (testForFunctionModifiers(def)
         || skipCxxAttributes() || testFunctionAttribute(def) || testFunctionRevision(def)) {}
     bool templateFunction = (lookup() == TEMPLATE);
     def->type = parseType();
@@ -426,6 +425,10 @@ bool Moc::parseFunction(FunctionDef *def, bool inMacro)
         scopedFunctionName = def->type.isScoped;
         def->type = Type("int");
     } else {
+        // we might have modifiers and attributes after a tag
+        // note that testFunctionAttribute is handled further below,
+        // and revisions and attributes must come first
+        while (testForFunctionModifiers(def)) {}
         Type tempType = parseType();;
         while (!tempType.name.isEmpty() && lookup() != LPAREN) {
             if (testFunctionAttribute(def->type.firstToken, def))
@@ -509,14 +512,20 @@ bool Moc::parseFunction(FunctionDef *def, bool inMacro)
     return true;
 }
 
+bool Moc::testForFunctionModifiers(FunctionDef *def)
+{
+    return test(EXPLICIT) || test(INLINE) ||
+            (test(STATIC) && (def->isStatic = true)) ||
+            (test(VIRTUAL) && (def->isVirtual = true));
+}
+
 // like parseFunction, but never aborts with an error
 bool Moc::parseMaybeFunction(const ClassDef *cdef, FunctionDef *def)
 {
     def->isVirtual = false;
     def->isStatic = false;
     //skip modifiers and attributes
-    while (test(EXPLICIT) || test(INLINE) || (test(STATIC) && (def->isStatic = true) == true) ||
-        (test(VIRTUAL) && (def->isVirtual = true) == true) //mark as virtual
+    while (testForFunctionModifiers(def)
         || skipCxxAttributes() || testFunctionAttribute(def) || testFunctionRevision(def)) {}
     bool tilde = test(TILDE);
     def->type = parseType();
@@ -534,6 +543,10 @@ bool Moc::parseMaybeFunction(const ClassDef *cdef, FunctionDef *def)
             def->type = Type("int");
         }
     } else {
+        // ### TODO: The condition before testForFunctionModifiers shoulnd't be necessary,
+        // but otherwise we end up with misparses
+        if (def->isSlot || def->isSignal || def->isInvokable)
+            while (testForFunctionModifiers(def)) {}
         Type tempType = parseType();;
         while (!tempType.name.isEmpty() && lookup() != LPAREN) {
             if (testFunctionAttribute(def->type.firstToken, def))
diff --git a/src/tools/moc/moc.h b/src/tools/moc/moc.h
index 512428356e2..67777c7a43a 100644
--- a/src/tools/moc/moc.h
+++ b/src/tools/moc/moc.h
@@ -280,6 +280,7 @@ public:
 
     void checkSuperClasses(ClassDef *def);
     void checkProperties(ClassDef* cdef);
+    bool testForFunctionModifiers(FunctionDef *def);
 };
 
 inline QByteArray noRef(const QByteArray &type)
diff --git a/tests/auto/tools/moc/tst_moc.cpp b/tests/auto/tools/moc/tst_moc.cpp
index 26522e94efd..a509c0ddae0 100644
--- a/tests/auto/tools/moc/tst_moc.cpp
+++ b/tests/auto/tools/moc/tst_moc.cpp
@@ -83,6 +83,19 @@ Q_DECLARE_METATYPE(const QMetaObject*);
 
 #define TESTEXPORTMACRO Q_DECL_EXPORT
 
+#if !defined(Q_MOC_RUN) && !defined(Q_NOREPLY)
+# define Q_NOREPLY
+#endif
+
+struct TagTest : QObject {
+    Q_OBJECT
+
+    Q_INVOKABLE Q_NOREPLY inline int test() {return 0;}
+public slots:
+    Q_NOREPLY virtual inline void pamOpen(int){}
+};
+
+
 namespace TestNonQNamespace {
 
 struct TestGadget {
@@ -787,6 +800,7 @@ private slots:
     void privateQPropertyShim();
     void readWriteThroughBindable();
     void invokableCtors();
+    void virtualInlineTaggedSlot();
 
 signals:
     void sigWithUnsignedArg(unsigned foo);
@@ -4575,6 +4589,20 @@ void tst_Moc::invokableCtors()
     QCOMPARE(result2.m_thing, 17);
 }
 
+void tst_Moc::virtualInlineTaggedSlot()
+{
+    auto mo = TagTest::staticMetaObject;
+    auto idx = mo.indexOfMethod("pamOpen(int)");
+    auto method = mo.method(idx);
+    QVERIFY(method.isValid()); // fails!
+    QCOMPARE(method.tag(), "Q_NOREPLY");
+    idx = mo.indexOfMethod("test()");
+    method = mo.method(idx);
+    QVERIFY(method.isValid());
+    QCOMPARE(method.tag(), "Q_NOREPLY");
+    QCOMPARE(method.returnMetaType(), QMetaType::fromType<int>());
+}
+
 QTEST_MAIN(tst_Moc)
 
 // the generated code must compile with QT_NO_KEYWORDS