qmltc: fix handlers for c++-signals

Allow qmltc to generate handlers for c++-defined signals with const
parameters by changing the safeguard to avoid type mismatch between
slots and signals.

First, remove the qOverload in the generated QObject::connect call to
be able the connect slots and signals with different types (namely,
pass by const references and pass by value should be interchangeable but
is not allowed by qOverload).

Second, save in QQmlJSMetaParameter when types are passed by pointer.
Like this, qqmljsimportvisitor can check if a value type is indeed
passed by value or const reference in a C++ signal. The same for reference
types that need to be passed by (const and non-const) pointer.
Print a message when an type is passed by argument in an incompatible
way instead of letting qmltc generate uncompilable code, which makes the
compiler print out cryptical messages.

Third, add a qqmlcpptypehelpers template that decides if value types
should be passed by value or reference, by letting the c++ compiler
check if sizeof(T) > 3*sizeof(void*).

Fixes: QTBUG-107625
Fixes: QTBUG-107622
Change-Id: I1a00532df591d10f74c1fd00dff5b7fccf40cb22
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

8 files changed, 103 insertions, 5 deletions

diff --git a/src/qml/CMakeLists.txt b/src/qml/CMakeLists.txt
index 5781b03573..686c9dfb24 100644
--- a/src/qml/CMakeLists.txt
+++ b/src/qml/CMakeLists.txt
@@ -359,6 +359,7 @@ qt_internal_add_qml_module(Qml
         qmltc/supportlibrary/qqmlcppbinding.cpp
         qmltc/supportlibrary/qqmlcpponassignment_p.h
         qmltc/supportlibrary/qqmlcpponassignment.cpp
+        qmltc/supportlibrary/qqmlcpptypehelpers_p.h
     DEFINES
         BUILDING_QT__
         ENABLE_ASSEMBLER_WX_EXCLUSIVE=1
diff --git a/src/qml/doc/snippets/qmltc/special/HelloWorld.qml.cpp b/src/qml/doc/snippets/qmltc/special/HelloWorld.qml.cpp
index 732d915b99..eda9009bb7 100644
--- a/src/qml/doc/snippets/qmltc/special/HelloWorld.qml.cpp
+++ b/src/qml/doc/snippets/qmltc/special/HelloWorld.qml.cpp
@@ -31,7 +31,7 @@ public:
     void setHello(const QString& hello_);
     QString hello();
     QBindable<QString> bindableHello();
-    Q_INVOKABLE void printHello(QString prefix, QString suffix);
+    Q_INVOKABLE void printHello(passByConstRefOrValue<QString> prefix, passByConstRefOrValue<QString> suffix);
 
     // ...
 };
diff --git a/src/qml/qmltc/supportlibrary/qqmlcpptypehelpers_p.h b/src/qml/qmltc/supportlibrary/qqmlcpptypehelpers_p.h
new file mode 100644
index 0000000000..2470d87efe
--- /dev/null
+++ b/src/qml/qmltc/supportlibrary/qqmlcpptypehelpers_p.h
@@ -0,0 +1,28 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+#ifndef QQMLCPPTYPEHELPERS_H
+#define QQMLCPPTYPEHELPERS_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API.  It exists purely as an
+// implementation detail.  This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+
+#include <type_traits>
+
+/*! \internal
+    Used by Qmltc to decide when value types should be passed by value or reference.
+ */
+template<typename T>
+using passByConstRefOrValue =
+        std::conditional_t<((sizeof(T) > 3 * sizeof(void *)) || !std::is_trivial_v<T>), const T &,
+                           T>;
+
+#endif // QQMLCPPTYPEHELPERS_H
diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp
index 8ce58cdcca..b44550cade 100644
--- a/src/qmlcompiler/qqmljsimportvisitor.cpp
+++ b/src/qmlcompiler/qqmljsimportvisitor.cpp
@@ -986,8 +986,69 @@ void QQmlJSImportVisitor::checkSignals()
 
             const auto signalParameters = signalMethod->parameters();
             QHash<QString, qsizetype> parameterNameIndexes;
-            for (int i = 0; i < signalParameters.size(); i++)
-                parameterNameIndexes[signalParameters[i].name()] = i;
+            // check parameter positions and also if signal is suitable for onSignal handler
+            for (int i = 0, end = signalParameters.size(); i < end; ++i) {
+                auto &p = signalParameters[i];
+                parameterNameIndexes[p.name()] = i;
+
+                auto signalName = [&]() {
+                    if (signal)
+                        return u" called %1"_s.arg(*signal);
+                    return QString();
+                };
+                auto type = p.type();
+                if (!type) {
+                    m_logger->log(
+                            QStringLiteral(
+                                    "Type %1 of parameter %2 in signal%3 was not found, but is "
+                                    "required to compile %4. Did you add all import paths?")
+                                    .arg(p.typeName(), p.name(), signalName(), pair.first),
+                            qmlSignalParameters, location);
+                    continue;
+                }
+
+                if (type->isComposite())
+                    continue;
+
+                // only accept following parameters for non-composite types:
+                // * QObjects by pointer (nonconst*, const*, const*const,*const)
+                // * Value types by value (QFont, int)
+                // * Value types by const ref (const QFont&, const int&)
+
+                auto parameterName = [&]() {
+                    if (p.name().isEmpty())
+                        return QString();
+                    return u" called %1"_s.arg(p.name());
+                };
+                switch (type->accessSemantics()) {
+                case QQmlJSScope::AccessSemantics::Reference:
+                    if (!p.isPointer())
+                        m_logger->log(QStringLiteral("Type %1 of parameter%2 in signal%3 should be "
+                                                     "passed by pointer to be able to compile %4. ")
+                                              .arg(p.typeName(), parameterName(), signalName(),
+                                                   pair.first),
+                                      qmlSignalParameters, location);
+                    break;
+                case QQmlJSScope::AccessSemantics::Value:
+                case QQmlJSScope::AccessSemantics::Sequence:
+                    if (p.isPointer())
+                        m_logger->log(
+                                QStringLiteral(
+                                        "Type %1 of parameter%2 in signal%3 should be passed by "
+                                        "value or const reference to be able to compile %4. ")
+                                        .arg(p.typeName(), parameterName(), signalName(),
+                                             pair.first),
+                                qmlSignalParameters, location);
+                    break;
+                case QQmlJSScope::AccessSemantics::None:
+                    m_logger->log(
+                            QStringLiteral("Type %1 of parameter%2 in signal%3 required by the "
+                                           "compilation of %4 cannot be used. ")
+                                    .arg(p.typeName(), parameterName(), signalName(), pair.first),
+                            qmlSignalParameters, location);
+                    break;
+                }
+            }
 
             if (pair.second.size() > signalParameters.size()) {
                 m_logger->log(QStringLiteral("Signal handler for \"%2\" has more formal"
diff --git a/src/qmlcompiler/qqmljsmetatypes_p.h b/src/qmlcompiler/qqmljsmetatypes_p.h
index 593a16b0c4..823a1849e5 100644
--- a/src/qmlcompiler/qqmljsmetatypes_p.h
+++ b/src/qmlcompiler/qqmljsmetatypes_p.h
@@ -135,6 +135,8 @@ public:
     void setType(QWeakPointer<const QQmlJSScope> type) { m_type = type; }
     Constness typeQualifier() const { return m_typeQualifier; }
     void setTypeQualifier(Constness typeQualifier) { m_typeQualifier = typeQualifier; }
+    bool isPointer() const { return m_isPointer; }
+    void setIsPointer(bool isPointer) { m_isPointer = isPointer; }
 
     friend bool operator==(const QQmlJSMetaParameter &a, const QQmlJSMetaParameter &b)
     {
@@ -159,6 +161,7 @@ private:
     QString m_typeName;
     QWeakPointer<const QQmlJSScope> m_type;
     Constness m_typeQualifier = NonConst;
+    bool m_isPointer = false;
 };
 
 class QQmlJSMetaMethod
diff --git a/src/qmlcompiler/qqmljsscope.cpp b/src/qmlcompiler/qqmljsscope.cpp
index ec0a90643d..e3d8bafeb3 100644
--- a/src/qmlcompiler/qqmljsscope.cpp
+++ b/src/qmlcompiler/qqmljsscope.cpp
@@ -508,6 +508,8 @@ QTypeRevision QQmlJSScope::resolveType(
             if (const QString typeName = parameter.typeName();
                 !parameter.type() && !typeName.isEmpty()) {
                 const auto type = findType(typeName, context, usedTypes);
+                if (type.scope && type.scope->isReferenceType())
+                    parameter.setIsPointer(true);
                 parameter.setType({ type.scope });
             }
         }
diff --git a/src/qmlcompiler/qqmljsscope_p.h b/src/qmlcompiler/qqmljsscope_p.h
index 9586d1c333..127d58abcc 100644
--- a/src/qmlcompiler/qqmljsscope_p.h
+++ b/src/qmlcompiler/qqmljsscope_p.h
@@ -561,6 +561,7 @@ public:
     void setAccessSemantics(AccessSemantics semantics) { m_semantics = semantics; }
     AccessSemantics accessSemantics() const { return m_semantics; }
     bool isReferenceType() const { return m_semantics == QQmlJSScope::AccessSemantics::Reference; }
+    bool isValueType() const { return m_semantics == QQmlJSScope::AccessSemantics::Value; }
 
     bool isIdInCurrentQmlScopes(const QString &id) const;
     bool isIdInCurrentJSScopes(const QString &id) const;
diff --git a/src/qmlcompiler/qqmljstypedescriptionreader.cpp b/src/qmlcompiler/qqmljstypedescriptionreader.cpp
index 6f40b1d42f..8528d39f0f 100644
--- a/src/qmlcompiler/qqmljstypedescriptionreader.cpp
+++ b/src/qmlcompiler/qqmljstypedescriptionreader.cpp
@@ -428,6 +428,7 @@ void QQmlJSTypeDescriptionReader::readParameter(UiObjectDefinition *ast, QQmlJSM
     QString name;
     QString type;
     bool isConstant = false;
+    bool isPointer = false;
 
     for (UiObjectMemberList *it = ast->initializer->members; it; it = it->next) {
         UiObjectMember *member = it->member;
@@ -443,9 +444,9 @@ void QQmlJSTypeDescriptionReader::readParameter(UiObjectDefinition *ast, QQmlJSM
         } else if (id == QLatin1String("type")) {
             type = readStringBinding(script);
         } else if (id == QLatin1String("isPointer")) {
-            // ### unhandled
+            isPointer = readBoolBinding(script);
         } else if (id == QLatin1String("isConstant")) {
-            isConstant = true;
+            isConstant = readBoolBinding(script);
         } else if (id == QLatin1String("isReadonly")) {
             // ### unhandled
         } else if (id == QLatin1String("isList")) {
@@ -458,6 +459,7 @@ void QQmlJSTypeDescriptionReader::readParameter(UiObjectDefinition *ast, QQmlJSM
 
     QQmlJSMetaParameter p(name, type);
     p.setTypeQualifier(isConstant ? QQmlJSMetaParameter::Const : QQmlJSMetaParameter::NonConst);
+    p.setIsPointer(isPointer);
     metaMethod->addParameter(std::move(p));
 }