Fix compile time type normalization code

Use a simpler constexpr to generate type name on gcc This
works around an ICE on gcc in release mode when compiling
with PCH enabled. As the type we're getting from Q_FUNC_INFO is
already in a somewhat normalized form, this requires significanlty
less processing and esp. not a recursive constexpr method which
I suspect triggers the ICE.

Fix integer type conversions to also properly normalize long long
values (to q(u)longlong. Make sure the mapping also works on
MSVC, where long long types get mapped to __int64. Also, normalize
unsigned short and unsigned char to ushort and uchar, respectively, to
follow the convention set by uint and ulong.

Add some test cases to verify the mappings.

Change-Id: I3dec5764450bf22ab6f066597803c3f46c2cd5ac
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

4 files changed, 191 insertions, 86 deletions

diff --git a/src/corelib/kernel/qmetatype.h b/src/corelib/kernel/qmetatype.h
index f25ad86255..18d911d33b 100644
--- a/src/corelib/kernel/qmetatype.h
+++ b/src/corelib/kernel/qmetatype.h
@@ -1746,13 +1746,148 @@ private:
             *output++ = x;
     }
 
+    constexpr void replaceLast(char x)
+    {
+        last = x;
+        if (output)
+            *(output-1) = x;
+    }
+
     constexpr void appendStr(const char *x)
     {
         while (*x)
             append(*x++);
     };
 
+    constexpr void normalizeIntegerTypes(const char *&begin, const char *end)
+    {
+        int numLong = 0;
+        int numSigned = 0;
+        int numUnsigned = 0;
+        int numInt = 0;
+        int numShort = 0;
+        int numChar = 0;
+        while (begin < end) {
+            if (skipToken(begin, end, "long")) {
+                numLong++;
+                continue;
+            }
+            if (skipToken(begin, end, "int")) {
+                numInt++;
+                continue;
+            }
+            if (skipToken(begin, end, "short")) {
+                numShort++;
+                continue;
+            }
+            if (skipToken(begin, end, "unsigned")) {
+                numUnsigned++;
+                continue;
+            }
+            if (skipToken(begin, end, "signed")) {
+                numSigned++;
+                continue;
+            }
+            if (skipToken(begin, end, "char")) {
+                numChar++;
+                continue;
+            }
+#ifdef Q_CC_MSVC
+            if (skipToken(begin, end, "__int64")) {
+                numLong = 2;
+                continue;
+            }
+#endif
+            break;
+        }
+        if (numLong == 2)
+            append('q'); // q(u)longlong
+        if (numSigned && numChar)
+            appendStr("signed ");
+        else if (numUnsigned)
+            appendStr("u");
+        if (numChar)
+            appendStr("char");
+        else if (numShort)
+            appendStr("short");
+        else if (numLong == 1)
+            appendStr("long");
+        else if (numLong == 2)
+            appendStr("longlong");
+        else if (numUnsigned || numSigned || numInt)
+            appendStr("int");
+    }
+
+    constexpr void skipStructClassOrEnum(const char *&begin, const char *end)
+    {
+        // discard 'struct', 'class', and 'enum'; they are optional
+        // and we don't want them in the normalized signature
+        skipToken(begin, end, "struct", true) || skipToken(begin, end, "class", true)
+                || skipToken(begin, end, "enum", true);
+    }
+
+    constexpr void skipQtNamespace(const char *&begin, const char *end)
+    {
+#ifdef QT_NAMESPACE
+        const char *nsbeg = begin;
+        if (skipToken(nsbeg, end, QT_STRINGIFY(QT_NAMESPACE)) && nsbeg + 2 < end && nsbeg[0] == ':'
+            && nsbeg[1] == ':') {
+            begin = nsbeg + 2;
+            while (begin != end && is_space(*begin))
+                begin++;
+        }
+#else
+        Q_UNUSED(begin);
+        Q_UNUSED(end);
+#endif
+    }
+
 public:
+#ifndef Q_CC_MSVC
+    // this is much simpler than the full type normalization below
+    // the reason is that the signature returned by Q_FUNC_INFO is already
+    // normalized to the largest degree, and we need to do only small adjustments
+    constexpr int normalizeTypeFromSignature(const char *begin, const char *end)
+    {
+        while (begin < end) {
+            if (*begin == ' ') {
+                if (last == ',' || last == '>' || last == '<' || last == '*' || last == '&') {
+                    ++begin;
+                    continue;
+                }
+            }
+            if (last == ' ') {
+                if (*begin == '*' || *begin == '&') {
+                    replaceLast(*begin);
+                    ++begin;
+                    continue;
+                }
+            }
+            if (!is_ident_char(last)) {
+                skipStructClassOrEnum(begin, end);
+                if (begin == end)
+                    break;
+
+                skipQtNamespace(begin, end);
+                if (begin == end)
+                    break;
+
+                normalizeIntegerTypes(begin, end);
+                if (begin == end)
+                    break;
+            }
+            append(*begin);
+            ++begin;
+        }
+        return len;
+    }
+#else
+    // MSVC needs the full normalization, as it puts the const in a different
+    // place than we expect
+    constexpr int normalizeTypeFromSignature(const char *begin, const char *end)
+    { return normalizeType(begin, end); }
+#endif
+
     constexpr int normalizeType(const char *begin, const char *end, bool adjustConst = true)
     {
         // Trim spaces
@@ -1835,20 +1970,8 @@ public:
             }
         }
 
-        // discard 'struct', 'class', and 'enum'; they are optional
-        // and we don't want them in the normalized signature
-        skipToken(begin, end, "struct", true) || skipToken(begin, end, "class", true)
-                || skipToken(begin, end, "enum", true);
-
-#ifdef QT_NAMESPACE
-        const char *nsbeg = begin;
-        if (skipToken(nsbeg, end, QT_STRINGIFY(QT_NAMESPACE)) && nsbeg + 2 < end && nsbeg[0] == ':'
-            && nsbeg[1] == ':') {
-            begin = nsbeg + 2;
-            while (begin != end && is_space(*begin))
-                begin++;
-        }
-#endif
+        skipStructClassOrEnum(begin, end);
+        skipQtNamespace(begin, end);
 
         if (skipToken(begin, end, "QVector")) {
             // Replace QVector by QList
@@ -1860,66 +1983,9 @@ public:
             appendStr("std::pair");
         }
 
-        if (!hasMiddleConst) {
+        if (!hasMiddleConst)
             // Normalize the integer types
-            int numLong = 0;
-            int numSigned = 0;
-            int numUnsigned = 0;
-            int numInt = 0;
-            int numShort = 0;
-            int numChar = 0;
-            while (begin < end) {
-                if (skipToken(begin, end, "long")) {
-                    numLong++;
-                    continue;
-                }
-                if (skipToken(begin, end, "int")) {
-                    numInt++;
-                    continue;
-                }
-                if (skipToken(begin, end, "short")) {
-                    numShort++;
-                    continue;
-                }
-                if (skipToken(begin, end, "unsigned")) {
-                    numUnsigned++;
-                    continue;
-                }
-                if (skipToken(begin, end, "signed")) {
-                    numSigned++;
-                    continue;
-                }
-                if (skipToken(begin, end, "char")) {
-                    numChar++;
-                    continue;
-                }
-                break;
-            }
-            if (numChar || numShort) {
-                if (numSigned && numChar)
-                    appendStr("signed ");
-                if (numUnsigned)
-                    appendStr("unsigned ");
-                if (numChar)
-                    appendStr("char");
-                else
-                    appendStr("short");
-            } else if (numLong) {
-                if (numLong == 1) {
-                    if (numUnsigned)
-                        append('u');
-                    appendStr("long");
-                } else {
-                    if (numUnsigned)
-                        appendStr("unsigned ");
-                    appendStr("long long");
-                }
-            } else if (numUnsigned || numSigned || numInt) {
-                if (numUnsigned)
-                    append('u');
-                appendStr("int");
-            }
-        }
+            normalizeIntegerTypes(begin, end);
 
         bool spaceSkiped = true;
         while (begin != end) {
@@ -2024,21 +2090,20 @@ constexpr auto typenameHelper()
         constexpr int suffix = sizeof("]");
 #endif
 
-#if !(defined(Q_CC_GNU) && !defined(Q_CC_INTEL) && !defined(Q_CC_CLANG))
-        constexpr auto func = Q_FUNC_INFO;
-        constexpr const char *begin = func + prefix;
-        constexpr const char *end = func + sizeof(Q_FUNC_INFO) - suffix;
-        constexpr int len = qNormalizeType(begin, end, nullptr);
-#else // GCC < 8.1 did not have Q_FUNC_INFO as constexpr, and GCC 9 has a precompiled header bug
+#if (defined(Q_CC_GNU) && !defined(Q_CC_INTEL) && !defined(Q_CC_CLANG) && Q_CC_GNU < 804)
         auto func = Q_FUNC_INFO;
         const char *begin = func + prefix;
         const char *end = func + sizeof(Q_FUNC_INFO) - suffix;
         // This is an upper bound of the size since the normalized signature should always be smaller
-        // (Unless there is a QList -> QVector change, but that should not happen)
         constexpr int len = sizeof(Q_FUNC_INFO) - suffix - prefix;
+#else
+        constexpr auto func = Q_FUNC_INFO;
+        constexpr const char *begin = func + prefix;
+        constexpr const char *end = func + sizeof(Q_FUNC_INFO) - suffix;
+        constexpr int len = QTypeNormalizer{ nullptr }.normalizeTypeFromSignature(begin, end);
 #endif
         std::array<char, len + 1> result {};
-        qNormalizeType(begin, end, result.data());
+        QTypeNormalizer{ result.data() }.normalizeTypeFromSignature(begin, end);
         return result;
     }
 }
diff --git a/tests/auto/corelib/kernel/qmetaobject/tst_qmetaobject.cpp b/tests/auto/corelib/kernel/qmetaobject/tst_qmetaobject.cpp
index c073e2d77f..08219d45d2 100644
--- a/tests/auto/corelib/kernel/qmetaobject/tst_qmetaobject.cpp
+++ b/tests/auto/corelib/kernel/qmetaobject/tst_qmetaobject.cpp
@@ -1402,15 +1402,15 @@ void tst_QMetaObject::normalizedType_data()
     QTest::newRow("QVector") << "QVector<int>" << "QList<int>";
     QTest::newRow("refref") << "X const*const&&" << "const X*const&&";
     QTest::newRow("refref2") << "const X<T const&&>&&" << "const X<const T&&>&&";
-    QTest::newRow("long1") << "long unsigned int long" << "unsigned long long";
+    QTest::newRow("long1") << "long unsigned int long" << "qulonglong";
     QTest::newRow("long2") << "int signed long" << "long";
     QTest::newRow("long3") << "long unsigned" << "ulong";
     QTest::newRow("long double") << " long  double" << "long double";
     QTest::newRow("signed char") << "char signed" << "signed char";
-    QTest::newRow("unsigned char") << "char unsigned" << "unsigned char";
+    QTest::newRow("unsigned char") << "char unsigned" << "uchar";
     QTest::newRow("signed short") << "short signed" << "short";
-    QTest::newRow("unsigned shot") << "short unsigned" << "unsigned short";
-    QTest::newRow("unsigned shot") << "short unsigned" << "unsigned short";
+    QTest::newRow("unsigned short") << "unsigned short" << "ushort";
+    QTest::newRow("short unsigned") << "short unsigned" << "ushort";
     QTest::newRow("array1") << "unsigned int [4]" << "uint[4]";
     QTest::newRow("array2") << "unsigned int const [4][5]" << "const uint[4][5]";
     QTest::newRow("array3") << "unsigned[] const" << "uint[]";
diff --git a/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.cpp b/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.cpp
index f116684f5b..caef716656 100644
--- a/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.cpp
+++ b/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.cpp
@@ -235,6 +235,7 @@ private slots:
     void operatorEq();
     void typesWithInaccessibleDTors();
     void voidIsNotUnknown();
+    void typeNameNormalization();
 };
 
 struct BaseGenericType
@@ -2715,6 +2716,43 @@ void tst_QMetaType::voidIsNotUnknown()
     QVERIFY(voidType != QMetaType(QMetaType::UnknownType));
 }
 
+void tst_QMetaType::typeNameNormalization()
+{
+    // check the we normalize types the right way
+
+#define CHECK_TYPE_NORMALIZATION(Normalized, ...) \
+    do { \
+        /*QCOMPARE(QtPrivate::typenameHelper<Type>(), Normalized);*/ \
+        QByteArray typeName = QMetaObject::normalizedType(#__VA_ARGS__); \
+        QCOMPARE(typeName, Normalized); \
+        typeName = QMetaType::fromType<__VA_ARGS__>().name(); \
+        QCOMPARE(typeName, Normalized); \
+    } while (0)
+
+    CHECK_TYPE_NORMALIZATION("QList<QString*const>", QList<QString * const>);
+    CHECK_TYPE_NORMALIZATION("QList<const QString*>", QList<const QString * >);
+    CHECK_TYPE_NORMALIZATION("QList<const QString*const>", QList<const QString * const>);
+    CHECK_TYPE_NORMALIZATION("QList<const QString*>", QList<QString const *>);
+    CHECK_TYPE_NORMALIZATION("QList<signed char>", QList<signed char>);
+    CHECK_TYPE_NORMALIZATION("QList<uint>", QList<unsigned>);
+    CHECK_TYPE_NORMALIZATION("uint", uint);
+    CHECK_TYPE_NORMALIZATION("QList<QHash<uint,QString*>>", QList<QHash<unsigned, QString *>>);
+    CHECK_TYPE_NORMALIZATION("QList<qlonglong>", QList<qlonglong>);
+    CHECK_TYPE_NORMALIZATION("QList<qulonglong>", QList<qulonglong>);
+    CHECK_TYPE_NORMALIZATION("QList<qlonglong>", QList<long long>);
+    CHECK_TYPE_NORMALIZATION("QList<qulonglong>", QList<unsigned long long>);
+    CHECK_TYPE_NORMALIZATION("QList<qulonglong*>", QList<unsigned long long *>);
+    CHECK_TYPE_NORMALIZATION("QList<ulong>", QList<long unsigned >);
+#ifdef Q_CC_MSVC
+    CHECK_TYPE_NORMALIZATION("qulonglong", __int64 unsigned);
+#endif
+    CHECK_TYPE_NORMALIZATION("std::pair<const QString&&,short>", QPair<const QString &&, signed short>);
+
+    // The string based normalization doesn't handle aliases, QMetaType::fromType() does
+//    CHECK_TYPE_NORMALIZATION("qulonglong", quint64);
+    QCOMPARE(QMetaType::fromType<quint64>().name(), "qulonglong");
+}
+
 // Compile-time test, it should be possible to register function pointer types
 class Undefined;
 
diff --git a/tests/auto/tools/moc/tst_moc.cpp b/tests/auto/tools/moc/tst_moc.cpp
index d3af266e74..d6eb76feb7 100644
--- a/tests/auto/tools/moc/tst_moc.cpp
+++ b/tests/auto/tools/moc/tst_moc.cpp
@@ -872,7 +872,9 @@ void tst_Moc::uLongLong()
     QVERIFY(idx != -1);
     idx = mobj->indexOfSlot("slotWithULongLong(unsigned long long)");
     QVERIFY(idx != -1);
-    idx = mobj->indexOfSlot("slotWithULongLongP(unsigned long long*)");
+    idx = mobj->indexOfSlot("slotWithULongLong(qulonglong)");
+    QVERIFY(idx != -1);
+    idx = mobj->indexOfSlot("slotWithULongLongP(qulonglong*)");
     QVERIFY(idx != -1);
 
     idx = mobj->indexOfSlot("slotWithLong(long)");