QQmlTypeNameCache: Unify querying for types

Template all of the code and make sure the different query variants do
the same work. There is no reason not to query the namespaced imports if
we are passed a different string type as parameter. If we want to skip
the namespaced imports, that is a separate parameter.

This needs to be picked to 6.2 as precondition for fixing
AOTCompiledContext::initLoadAttachedLookup. We need to pass a
QQmlImport::RecursionRestriction when querying by QHashedStringRef.

Pick-to: 6.2
Change-Id: I98aecc7775036728668cc93f550aa73fdefafe9a
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

6 files changed, 150 insertions, 144 deletions

diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp
index c3fbff7f55..1af801163d 100644
--- a/src/qml/jsruntime/qv4qmlcontext.cpp
+++ b/src/qml/jsruntime/qv4qmlcontext.cpp
@@ -218,7 +218,7 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r
 
     if (context->imports() && name->startsWithUpper()) {
         // Search for attached properties, enums and imported scripts
-        QQmlTypeNameCache::Result r = context->imports()->query(name, QQmlImport::AllowRecursion);
+        QQmlTypeNameCache::Result r = context->imports()->query<QQmlImport::AllowRecursion>(name);
 
         if (r.isValid()) {
             if (hasProperty)
diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp
index 825a2cfc8a..02a18164ab 100644
--- a/src/qml/qml/qqmlproperty.cpp
+++ b/src/qml/qml/qqmlproperty.cpp
@@ -295,17 +295,24 @@ void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name,
                         currentObject = qmlAttachedPropertiesObject(currentObject, func);
                         if (!currentObject) return; // Something is broken with the attachable type
                     } else if (r.importNamespace) {
-                        if ((ii + 1) == path.count()) return; // No type following the namespace
+                        if (++ii == path.count())
+                            return; // No type following the namespace
 
-                        ++ii; r = typeNameCache->query(path.at(ii), r.importNamespace);
-                        if (!r.type.isValid()) return; // Invalid type in namespace
+                        // TODO: Do we really _not_ want to query the namespaced types here?
+                        r = typeNameCache->query<QQmlTypeNameCache::QueryNamespaced::No>(
+                                    path.at(ii), r.importNamespace);
+
+                        if (!r.type.isValid())
+                            return; // Invalid type in namespace
 
                         QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine);
                         QQmlAttachedPropertiesFunc func = r.type.attachedPropertiesFunction(enginePrivate);
-                        if (!func) return; // Not an attachable type
+                        if (!func)
+                            return; // Not an attachable type
 
                         currentObject = qmlAttachedPropertiesObject(currentObject, func);
-                        if (!currentObject) return; // Something is broken with the attachable type
+                        if (!currentObject)
+                            return; // Something is broken with the attachable type
 
                     } else if (r.scriptIndex != -1) {
                         return; // Not a type
diff --git a/src/qml/qml/qqmltypemodule.cpp b/src/qml/qml/qqmltypemodule.cpp
index 5120728031..c373a4904e 100644
--- a/src/qml/qml/qqmltypemodule.cpp
+++ b/src/qml/qml/qqmltypemodule.cpp
@@ -87,23 +87,8 @@ void QQmlTypeModule::remove(const QQmlTypePrivate *type)
         QQmlMetaType::removeQQmlTypePrivate(elementIt.value(), type);
 }
 
-QQmlType QQmlTypeModule::type(const QHashedStringRef &name, QTypeRevision version) const
+QQmlType QQmlTypeModule::findType(const QList<QQmlTypePrivate *> *types, QTypeRevision version)
 {
-    QMutexLocker lock(&m_mutex);
-    QList<QQmlTypePrivate *> *types = m_typeHash.value(name);
-    if (types) {
-        for (int ii = 0; ii < types->count(); ++ii)
-            if (types->at(ii)->version.minorVersion() <= version.minorVersion())
-                return QQmlType(types->at(ii));
-    }
-
-    return QQmlType();
-}
-
-QQmlType QQmlTypeModule::type(const QV4::String *name, QTypeRevision version) const
-{
-    QMutexLocker lock(&m_mutex);
-    QList<QQmlTypePrivate *> *types = m_typeHash.value(name);
     if (types) {
         for (int ii = 0; ii < types->count(); ++ii)
             if (types->at(ii)->version.minorVersion() <= version.minorVersion())
diff --git a/src/qml/qml/qqmltypemodule_p.h b/src/qml/qml/qqmltypemodule_p.h
index c6dae7ef55..37b9a32b90 100644
--- a/src/qml/qml/qqmltypemodule_p.h
+++ b/src/qml/qml/qqmltypemodule_p.h
@@ -53,6 +53,7 @@
 
 #include <QtQml/qtqmlglobal.h>
 #include <QtQml/private/qstringhash_p.h>
+#include <QtQml/private/qqmltype_p.h>
 #include <QtCore/qmutex.h>
 #include <QtCore/qstring.h>
 #include <QtCore/qversionnumber.h>
@@ -114,12 +115,24 @@ public:
     quint8 minimumMinorVersion() const { return m_minMinorVersion.loadRelaxed(); }
     quint8 maximumMinorVersion() const { return m_maxMinorVersion.loadRelaxed(); }
 
-    QQmlType type(const QHashedStringRef &, QTypeRevision version) const;
-    QQmlType type(const QV4::String *, QTypeRevision version) const;
+    QQmlType type(const QHashedStringRef &name, QTypeRevision version) const
+    {
+        QMutexLocker lock(&m_mutex);
+        return findType(m_typeHash.value(name), version);
+    }
+
+    QQmlType type(const QV4::String *name, QTypeRevision version) const
+    {
+        QMutexLocker lock(&m_mutex);
+        return findType(m_typeHash.value(name), version);
+    }
 
     void walkCompositeSingletons(const std::function<void(const QQmlType &)> &callback) const;
 
 private:
+    static Q_QML_PRIVATE_EXPORT QQmlType findType(
+            const QList<QQmlTypePrivate *> *types, QTypeRevision version);
+
     const QString m_module;
     const quint8 m_majorVersion = 0;
 
diff --git a/src/qml/qml/qqmltypenamecache.cpp b/src/qml/qml/qqmltypenamecache.cpp
index 45333668e3..0f23b3190e 100644
--- a/src/qml/qml/qqmltypenamecache.cpp
+++ b/src/qml/qml/qqmltypenamecache.cpp
@@ -86,119 +86,5 @@ void QQmlTypeNameCache::add(const QHashedString &name, int importedScriptIndex,
     m_namedImports.insert(name, import);
 }
 
-QQmlTypeNameCache::Result QQmlTypeNameCache::query(const QHashedStringRef &name) const
-{
-    Result result = query(m_namedImports, name);
-
-    if (!result.isValid())
-        result = typeSearch(m_anonymousImports, name);
-
-    if (!result.isValid())
-        result = query(m_anonymousCompositeSingletons, name);
-
-    if (!result.isValid()) {
-        // Look up anonymous types from the imports of this document
-        QQmlImportNamespace *typeNamespace = nullptr;
-        QList<QQmlError> errors;
-        QQmlType t;
-        bool typeFound = m_imports.resolveType(name, &t, nullptr, &typeNamespace, &errors);
-        if (typeFound) {
-            return Result(t);
-        }
-
-    }
-
-    return result;
-}
-
-QQmlTypeNameCache::Result QQmlTypeNameCache::query(const QHashedStringRef &name,
-                                                   const QQmlImportRef *importNamespace) const
-{
-    Q_ASSERT(importNamespace && importNamespace->scriptIndex == -1);
-
-    Result result = typeSearch(importNamespace->modules, name);
-
-    if (!result.isValid())
-        result = query(importNamespace->compositeSingletons, name);
-
-    if (!result.isValid()) {
-        // Look up types from the imports of this document
-        // ### it would be nice if QQmlImports allowed us to resolve a namespace
-        // first, and then types on it.
-        QString qualifiedTypeName = importNamespace->m_qualifier + QLatin1Char('.') + name.toString();
-        QQmlImportNamespace *typeNamespace = nullptr;
-        QList<QQmlError> errors;
-        QQmlType t;
-        bool typeFound = m_imports.resolveType(qualifiedTypeName, &t, nullptr, &typeNamespace, &errors);
-        if (typeFound) {
-            return Result(t);
-        }
-    }
-
-    return result;
-}
-
-QQmlTypeNameCache::Result QQmlTypeNameCache::query(const QV4::String *name, QQmlImport::RecursionRestriction recursionRestriction) const
-{
-    Result result = query(m_namedImports, name);
-
-    if (!result.isValid())
-        result = typeSearch(m_anonymousImports, name);
-
-    if (!result.isValid())
-        result = query(m_anonymousCompositeSingletons, name);
-
-    if (!result.isValid()) {
-        // Look up anonymous types from the imports of this document
-        QString typeName = name->toQStringNoThrow();
-        QQmlImportNamespace *typeNamespace = nullptr;
-        QList<QQmlError> errors;
-        QQmlType t;
-        bool typeRecursionDetected = false;
-        bool typeFound = m_imports.resolveType(typeName, &t, nullptr, &typeNamespace, &errors,
-                                               QQmlType::AnyRegistrationType,
-                                               recursionRestriction == QQmlImport::AllowRecursion ? &typeRecursionDetected : nullptr);
-        if (typeFound) {
-            return Result(t);
-        }
-
-    }
-
-    return result;
-}
-
-QQmlTypeNameCache::Result QQmlTypeNameCache::query(const QV4::String *name, const QQmlImportRef *importNamespace) const
-{
-    Q_ASSERT(importNamespace && importNamespace->scriptIndex == -1);
-
-    QMap<const QQmlImportRef *, QStringHash<QQmlImportRef> >::const_iterator it = m_namespacedImports.constFind(importNamespace);
-    if (it != m_namespacedImports.constEnd()) {
-        Result r = query(*it, name);
-        if (r.isValid())
-            return r;
-    }
-
-    Result r = typeSearch(importNamespace->modules, name);
-
-    if (!r.isValid())
-        r = query(importNamespace->compositeSingletons, name);
-
-    if (!r.isValid()) {
-        // Look up types from the imports of this document
-        // ### it would be nice if QQmlImports allowed us to resolve a namespace
-        // first, and then types on it.
-        QString qualifiedTypeName = importNamespace->m_qualifier + QLatin1Char('.') + name->toQStringNoThrow();
-        QQmlImportNamespace *typeNamespace = nullptr;
-        QList<QQmlError> errors;
-        QQmlType t;
-        bool typeFound = m_imports.resolveType(qualifiedTypeName, &t, nullptr, &typeNamespace, &errors);
-        if (typeFound) {
-            return Result(t);
-        }
-    }
-
-    return r;
-}
-
 QT_END_NAMESPACE
 
diff --git a/src/qml/qml/qqmltypenamecache_p.h b/src/qml/qml/qqmltypenamecache_p.h
index 974f99ecae..5e1a88b2a5 100644
--- a/src/qml/qml/qqmltypenamecache_p.h
+++ b/src/qml/qml/qqmltypenamecache_p.h
@@ -104,16 +104,131 @@ public:
         const QQmlImportRef *importNamespace;
         int scriptIndex;
     };
-    Result query(const QHashedStringRef &) const;
-    Result query(const QHashedStringRef &, const QQmlImportRef *importNamespace) const;
-    Result query(const QV4::String *, QQmlImport::RecursionRestriction recursionRestriction = QQmlImport::PreventRecursion) const;
-    Result query(const QV4::String *, const QQmlImportRef *importNamespace) const;
+
+    enum class QueryNamespaced { No, Yes };
+
+    // Restrict the types allowed for key. We don't want QV4::ScopedString, for  example.
+
+    template<QQmlImport::RecursionRestriction recursionRestriction = QQmlImport::PreventRecursion>
+    Result query(const QHashedStringRef &key) const
+    {
+        return doQuery<const QHashedStringRef &, recursionRestriction>(key);
+    }
+
+    template<QueryNamespaced queryNamespaced = QueryNamespaced::Yes>
+    Result query(const QHashedStringRef &key, const QQmlImportRef *importNamespace) const
+    {
+        return doQuery<const QHashedStringRef &, queryNamespaced>(key, importNamespace);
+    }
+
+    template<QQmlImport::RecursionRestriction recursionRestriction = QQmlImport::PreventRecursion>
+    Result query(const QV4::String *key) const
+    {
+        return doQuery<const QV4::String *, recursionRestriction>(key);
+    }
+
+    template<QueryNamespaced queryNamespaced = QueryNamespaced::Yes>
+    Result query(const QV4::String *key, const QQmlImportRef *importNamespace) const
+    {
+        return doQuery<const QV4::String *, queryNamespaced>(key, importNamespace);
+    }
 
 private:
     friend class QQmlImports;
 
+    static QHashedStringRef toHashedStringRef(const QHashedStringRef &key) { return key; }
+    static QHashedStringRef toHashedStringRef(const QV4::String *key)
+    {
+        const QV4::Heap::String *heapString = key->d();
+
+        // toQString() would also do simplifyString(). Therefore, we can be sure that this
+        // is safe. Any other operation on the string data cannot keep references on the
+        // non-simplified pieces.
+        if (heapString->subtype >= QV4::Heap::String::StringType_Complex)
+            heapString->simplifyString();
+
+        // This is safe because the string data is backed by the QV4::String we got as
+        // parameter. The contract about passing V4 values as parameters is that you have to
+        // scope them first, so that they don't get gc'd while the callee is working on them.
+        const QStringPrivate &text = heapString->text();
+        return QHashedStringRef(QStringView(text.ptr, text.size));
+    }
+
+    static QString toQString(const QHashedStringRef &key) { return key.toString(); }
+    static QString toQString(const QV4::String *key) { return key->toQStringNoThrow(); }
+
+    template<typename Key, QQmlImport::RecursionRestriction recursionRestriction>
+    Result doQuery(Key name) const
+    {
+        Result result = doQuery(m_namedImports, name);
+
+        if (!result.isValid())
+            result = typeSearch(m_anonymousImports, name);
+
+        if (!result.isValid())
+            result = doQuery(m_anonymousCompositeSingletons, name);
+
+        if (!result.isValid()) {
+            // Look up anonymous types from the imports of this document
+            // ### it would be nice if QQmlImports allowed us to resolve a namespace
+            // first, and then types on it.
+            QQmlImportNamespace *typeNamespace = nullptr;
+            QList<QQmlError> errors;
+            QQmlType t;
+            bool typeRecursionDetected = false;
+            const bool typeFound = m_imports.resolveType(
+                        toHashedStringRef(name), &t, nullptr, &typeNamespace, &errors,
+                        QQmlType::AnyRegistrationType,
+                        recursionRestriction == QQmlImport::AllowRecursion
+                            ? &typeRecursionDetected
+                            : nullptr);
+            if (typeFound)
+                return Result(t);
+
+        }
+
+        return result;
+    }
+
+    template<typename Key, QueryNamespaced queryNamespaced>
+    Result doQuery(Key name, const QQmlImportRef *importNamespace) const
+    {
+        Q_ASSERT(importNamespace && importNamespace->scriptIndex == -1);
+
+        if constexpr (queryNamespaced == QueryNamespaced::Yes) {
+            QMap<const QQmlImportRef *, QStringHash<QQmlImportRef> >::const_iterator it
+                    = m_namespacedImports.constFind(importNamespace);
+            if (it != m_namespacedImports.constEnd()) {
+                Result r = doQuery(*it, name);
+                if (r.isValid())
+                    return r;
+            }
+        }
+
+        Result result = typeSearch(importNamespace->modules, name);
+
+        if (!result.isValid())
+            result = doQuery(importNamespace->compositeSingletons, name);
+
+        if (!result.isValid()) {
+            // Look up types from the imports of this document
+            // ### it would be nice if QQmlImports allowed us to resolve a namespace
+            // first, and then types on it.
+            const QString qualifiedTypeName = importNamespace->m_qualifier + u'.' + toQString(name);
+            QQmlImportNamespace *typeNamespace = nullptr;
+            QList<QQmlError> errors;
+            QQmlType t;
+            bool typeFound = m_imports.resolveType(
+                        qualifiedTypeName, &t, nullptr, &typeNamespace, &errors);
+            if (typeFound)
+                return Result(t);
+        }
+
+        return result;
+    }
+
     template<typename Key>
-    Result query(const QStringHash<QQmlImportRef> &imports, Key key) const
+    Result doQuery(const QStringHash<QQmlImportRef> &imports, Key key) const
     {
         QQmlImportRef *i = imports.value(key);
         if (i) {
@@ -129,7 +244,7 @@ private:
     }
 
     template<typename Key>
-    Result query(const QStringHash<QUrl> &urls, Key key) const
+    Result doQuery(const QStringHash<QUrl> &urls, Key key) const
     {
         QUrl *url = urls.value(key);
         if (url) {