Clean up import resolution handling

Instead of storing raw pointers into the compiled data and creating
synthetic CompiledData::Import objects for implicit imports, this patch
introduces a dedicated PendingImport data structure that removes the
need of using raw pointers, reduces the amount of string index to string
resolution and incorporates the priority field thus eliminating the need
for a hash table.

Change-Id: Ia59aad17687bc0935aafd36236bda85d5f4f82a2
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

5 files changed, 81 insertions, 58 deletions

diff --git a/src/qml/qml/qqmldirdata.cpp b/src/qml/qml/qqmldirdata.cpp
index ec398fa896..7652cec322 100644
--- a/src/qml/qml/qqmldirdata.cpp
+++ b/src/qml/qml/qqmldirdata.cpp
@@ -51,16 +51,15 @@ const QString &QQmlQmldirData::content() const
     return m_content;
 }
 
-const QV4::CompiledData::Import *QQmlQmldirData::import(QQmlTypeLoader::Blob *blob) const
+QQmlTypeLoader::Blob::PendingImportPtr QQmlQmldirData::import(QQmlTypeLoader::Blob *blob) const
 {
-    QHash<QQmlTypeLoader::Blob *, const QV4::CompiledData::Import *>::const_iterator it =
-            m_imports.find(blob);
+    auto it = m_imports.find(blob);
     if (it == m_imports.end())
         return nullptr;
     return *it;
 }
 
-void QQmlQmldirData::setImport(QQmlTypeLoader::Blob *blob, const QV4::CompiledData::Import *import)
+void QQmlQmldirData::setImport(QQmlTypeLoader::Blob *blob, QQmlTypeLoader::Blob::PendingImportPtr import)
 {
     m_imports[blob] = import;
 }
diff --git a/src/qml/qml/qqmldirdata_p.h b/src/qml/qml/qqmldirdata_p.h
index 6af393c47f..34f1ff1678 100644
--- a/src/qml/qml/qqmldirdata_p.h
+++ b/src/qml/qml/qqmldirdata_p.h
@@ -65,8 +65,8 @@ private:
 public:
     const QString &content() const;
 
-    const QV4::CompiledData::Import *import(QQmlTypeLoader::Blob *) const;
-    void setImport(QQmlTypeLoader::Blob *, const QV4::CompiledData::Import *);
+    PendingImportPtr import(QQmlTypeLoader::Blob *) const;
+    void setImport(QQmlTypeLoader::Blob *, PendingImportPtr);
 
     int priority(QQmlTypeLoader::Blob *) const;
     void setPriority(QQmlTypeLoader::Blob *, int);
@@ -77,7 +77,7 @@ protected:
 
 private:
     QString m_content;
-    QHash<QQmlTypeLoader::Blob *, const QV4::CompiledData::Import *> m_imports;
+    QHash<QQmlTypeLoader::Blob *, QQmlTypeLoader::Blob::PendingImportPtr> m_imports;
     QHash<QQmlTypeLoader::Blob *, int> m_priorities;
 };
 
diff --git a/src/qml/qml/qqmltypedata.cpp b/src/qml/qml/qqmltypedata.cpp
index 12d4d8cbad..8d75b57fc1 100644
--- a/src/qml/qml/qqmltypedata.cpp
+++ b/src/qml/qml/qqmltypedata.cpp
@@ -155,7 +155,8 @@ bool QQmlTypeData::tryLoadFromDiskCache()
                     && import->majorVersion == -1
                     && import->minorVersion == -1) {
                     QList<QQmlError> errors;
-                    if (!fetchQmldir(qmldirUrl, import, 1, &errors)) {
+                    auto pendingImport = std::make_shared<PendingImport>(this, import);
+                    if (!fetchQmldir(qmldirUrl, pendingImport, 1, &errors)) {
                         setError(errors);
                         return false;
                     }
@@ -522,10 +523,8 @@ void QQmlTypeData::continueLoadFromIR()
             if (!loadImplicitImport())
                 return;
             // This qmldir is for the implicit import
-            QQmlJS::MemoryPool *pool = m_document->jsParserEngine.pool();
-            auto implicitImport = pool->New<QV4::CompiledData::Import>();
-            implicitImport->uriIndex = m_document->registerString(QLatin1String("."));
-            implicitImport->qualifierIndex = 0; // empty string
+            auto implicitImport = std::make_shared<PendingImport>();
+            implicitImport->uri = QLatin1String(".");
             implicitImport->majorVersion = -1;
             implicitImport->minorVersion = -1;
             QList<QQmlError> errors;
@@ -560,16 +559,16 @@ void QQmlTypeData::allDependenciesDone()
     if (!m_typesResolved) {
         // Check that all imports were resolved
         QList<QQmlError> errors;
-        QHash<const QV4::CompiledData::Import *, int>::const_iterator it = m_unresolvedImports.constBegin(), end = m_unresolvedImports.constEnd();
+        auto it = m_unresolvedImports.constBegin(), end = m_unresolvedImports.constEnd();
         for ( ; it != end; ++it) {
-            if (*it == 0) {
+            if ((*it)->priority == 0) {
                 // This import was not resolved
-                for (auto keyIt = m_unresolvedImports.keyBegin(),
-                          keyEnd = m_unresolvedImports.keyEnd();
+                for (auto keyIt = m_unresolvedImports.constBegin(),
+                          keyEnd = m_unresolvedImports.constEnd();
                      keyIt != keyEnd; ++keyIt) {
-                    const QV4::CompiledData::Import *import = *keyIt;
+                    PendingImportPtr import = *keyIt;
                     QQmlError error;
-                    error.setDescription(QQmlTypeLoader::tr("module \"%1\" is not installed").arg(stringAt(import->uriIndex)));
+                    error.setDescription(QQmlTypeLoader::tr("module \"%1\" is not installed").arg(import->uri));
                     error.setUrl(m_importCache.baseUrl());
                     error.setLine(import->location.line);
                     error.setColumn(import->location.column);
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 4b6c09769a..a1bf9e9201 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -468,6 +468,16 @@ void QQmlTypeLoader::shutdownThread()
         m_thread->shutdown();
 }
 
+QQmlTypeLoader::Blob::PendingImport::PendingImport(QQmlTypeLoader::Blob *blob, const QV4::CompiledData::Import *import)
+{
+    type = static_cast<QV4::CompiledData::Import::ImportType>(quint32(import->type));
+    uri = blob->stringAt(import->uriIndex);
+    qualifier = blob->stringAt(import->qualifierIndex);
+    majorVersion = import->majorVersion;
+    minorVersion = import->minorVersion;
+    location = import->location;
+}
+
 QQmlTypeLoader::Blob::Blob(const QUrl &url, QQmlDataBlob::Type type, QQmlTypeLoader *loader)
   : QQmlDataBlob(url, type, loader), m_importCache(loader)
 {
@@ -477,7 +487,7 @@ QQmlTypeLoader::Blob::~Blob()
 {
 }
 
-bool QQmlTypeLoader::Blob::fetchQmldir(const QUrl &url, const QV4::CompiledData::Import *import, int priority, QList<QQmlError> *errors)
+bool QQmlTypeLoader::Blob::fetchQmldir(const QUrl &url, PendingImportPtr import, int priority, QList<QQmlError> *errors)
 {
     QQmlRefPointer<QQmlQmldirData> data = typeLoader()->getQmldir(url);
 
@@ -497,26 +507,22 @@ bool QQmlTypeLoader::Blob::fetchQmldir(const QUrl &url, const QV4::CompiledData:
     return true;
 }
 
-bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer<QQmlQmldirData> &data, const QV4::CompiledData::Import *import, QList<QQmlError> *errors)
+bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer<QQmlQmldirData> &data, PendingImportPtr import, QList<QQmlError> *errors)
 {
     QString qmldirIdentifier = data->urlString();
     QString qmldirUrl = qmldirIdentifier.left(qmldirIdentifier.lastIndexOf(QLatin1Char('/')) + 1);
 
     typeLoader()->setQmldirContent(qmldirIdentifier, data->content());
 
-    if (!m_importCache.updateQmldirContent(typeLoader()->importDatabase(), stringAt(import->uriIndex), stringAt(import->qualifierIndex), qmldirIdentifier, qmldirUrl, errors))
+    if (!m_importCache.updateQmldirContent(typeLoader()->importDatabase(), import->uri, import->qualifier, qmldirIdentifier, qmldirUrl, errors))
         return false;
 
-    QHash<const QV4::CompiledData::Import *, int>::iterator it = m_unresolvedImports.find(import);
-    if (it != m_unresolvedImports.end()) {
-        *it = data->priority(this);
-    }
+    import->priority = data->priority(this);
 
     // Release this reference at destruction
     m_qmldirs << data;
 
-    const QString &importQualifier = stringAt(import->qualifierIndex);
-    if (!importQualifier.isEmpty()) {
+    if (!import->qualifier.isEmpty()) {
         // Does this library contain any qualified scripts?
         QUrl libraryUrl(qmldirUrl);
         const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirIdentifier);
@@ -526,7 +532,7 @@ bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer<QQmlQmldirData> &da
             QQmlRefPointer<QQmlScriptBlob> blob = typeLoader()->getScript(scriptUrl);
             addDependency(blob.data());
 
-            scriptImported(blob, import->location, script.nameSpace, importQualifier);
+            scriptImported(blob, import->location, script.nameSpace, import->qualifier);
         }
     }
 
@@ -535,36 +541,39 @@ bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer<QQmlQmldirData> &da
 
 bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QList<QQmlError> *errors)
 {
+    return addImport(std::make_shared<PendingImport>(this, import), errors);
+}
+
+bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr import, QList<QQmlError> *errors)
+{
     Q_ASSERT(errors);
 
     QQmlImportDatabase *importDatabase = typeLoader()->importDatabase();
 
-    const QString &importUri = stringAt(import->uriIndex);
-    const QString &importQualifier = stringAt(import->qualifierIndex);
     if (import->type == QV4::CompiledData::Import::ImportScript) {
-        QUrl scriptUrl = finalUrl().resolved(QUrl(importUri));
+        QUrl scriptUrl = finalUrl().resolved(QUrl(import->uri));
         QQmlRefPointer<QQmlScriptBlob> blob = typeLoader()->getScript(scriptUrl);
         addDependency(blob.data());
 
-        scriptImported(blob, import->location, importQualifier, QString());
+        scriptImported(blob, import->location, import->qualifier, QString());
     } else if (import->type == QV4::CompiledData::Import::ImportLibrary) {
         QString qmldirFilePath;
         QString qmldirUrl;
 
-        if (QQmlMetaType::isLockedModule(importUri, import->majorVersion)) {
+        if (QQmlMetaType::isLockedModule(import->uri, import->majorVersion)) {
             //Locked modules are checked first, to save on filesystem checks
-            if (!m_importCache.addLibraryImport(importDatabase, importUri, importQualifier, import->majorVersion,
+            if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion,
                                           import->minorVersion, QString(), QString(), false, errors))
                 return false;
 
-        } else if (m_importCache.locateQmldir(importDatabase, importUri, import->majorVersion, import->minorVersion,
+        } else if (m_importCache.locateQmldir(importDatabase, import->uri, import->majorVersion, import->minorVersion,
                                  &qmldirFilePath, &qmldirUrl)) {
             // This is a local library import
-            if (!m_importCache.addLibraryImport(importDatabase, importUri, importQualifier, import->majorVersion,
+            if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion,
                                           import->minorVersion, qmldirFilePath, qmldirUrl, false, errors))
                 return false;
 
-            if (!importQualifier.isEmpty()) {
+            if (!import->qualifier.isEmpty()) {
                 // Does this library contain any qualified scripts?
                 QUrl libraryUrl(qmldirUrl);
                 const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirFilePath);
@@ -574,18 +583,18 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL
                     QQmlRefPointer<QQmlScriptBlob> blob = typeLoader()->getScript(scriptUrl);
                     addDependency(blob.data());
 
-                    scriptImported(blob, import->location, script.nameSpace, importQualifier);
+                    scriptImported(blob, import->location, script.nameSpace, import->qualifier);
                 }
             }
         } else {
             // Is this a module?
-            if (QQmlMetaType::isAnyModule(importUri)) {
-                if (!m_importCache.addLibraryImport(importDatabase, importUri, importQualifier, import->majorVersion,
+            if (QQmlMetaType::isAnyModule(import->uri)) {
+                if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion,
                                               import->minorVersion, QString(), QString(), false, errors))
                     return false;
             } else {
                 // We haven't yet resolved this import
-                m_unresolvedImports.insert(import, 0);
+                m_unresolvedImports << import;
 
                 QQmlAbstractUrlInterceptor *interceptor = typeLoader()->engine()->urlInterceptor();
 
@@ -596,13 +605,13 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL
                                         : QQmlImportDatabase::Remote);
                 if (!remotePathList.isEmpty()) {
                     // Add this library and request the possible locations for it
-                    if (!m_importCache.addLibraryImport(importDatabase, importUri, importQualifier, import->majorVersion,
+                    if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion,
                                                   import->minorVersion, QString(), QString(), true, errors))
                         return false;
 
                     // Probe for all possible locations
                     int priority = 0;
-                    const QStringList qmlDirPaths = QQmlImports::completeQmldirPaths(importUri, remotePathList, import->majorVersion, import->minorVersion);
+                    const QStringList qmlDirPaths = QQmlImports::completeQmldirPaths(import->uri, remotePathList, import->majorVersion, import->minorVersion);
                     for (const QString &qmldirPath : qmlDirPaths) {
                         if (interceptor) {
                             QUrl url = interceptor->intercept(
@@ -625,7 +634,7 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL
 
         bool incomplete = false;
 
-        QUrl importUrl(importUri);
+        QUrl importUrl(import->uri);
         QString path = importUrl.path();
         path.append(QLatin1String(path.endsWith(QLatin1Char('/')) ? "qmldir" : "/qmldir"));
         importUrl.setPath(path);
@@ -635,7 +644,7 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL
             incomplete = true;
         }
 
-        if (!m_importCache.addFileImport(importDatabase, importUri, importQualifier, import->majorVersion,
+        if (!m_importCache.addFileImport(importDatabase, import->uri, import->qualifier, import->majorVersion,
                                    import->minorVersion, incomplete, errors))
             return false;
 
@@ -653,7 +662,7 @@ void QQmlTypeLoader::Blob::dependencyComplete(QQmlDataBlob *blob)
     if (blob->type() == QQmlDataBlob::QmldirFile) {
         QQmlQmldirData *data = static_cast<QQmlQmldirData *>(blob);
 
-        const QV4::CompiledData::Import *import = data->import(this);
+        PendingImportPtr import = data->import(this);
 
         QList<QQmlError> errors;
         if (!qmldirDataAvailable(data, &errors)) {
@@ -685,9 +694,7 @@ bool QQmlTypeLoader::Blob::diskCacheForced()
 
 bool QQmlTypeLoader::Blob::qmldirDataAvailable(const QQmlRefPointer<QQmlQmldirData> &data, QList<QQmlError> *errors)
 {
-    bool resolve = true;
-
-    const QV4::CompiledData::Import *import = data->import(this);
+    PendingImportPtr import = data->import(this);
     data->setImport(this, nullptr);
 
     int priority = data->priority(this);
@@ -695,10 +702,7 @@ bool QQmlTypeLoader::Blob::qmldirDataAvailable(const QQmlRefPointer<QQmlQmldirDa
 
     if (import) {
         // Do we need to resolve this import?
-        QHash<const QV4::CompiledData::Import *, int>::iterator it = m_unresolvedImports.find(import);
-        if (it != m_unresolvedImports.end()) {
-            resolve = (*it == 0) || (*it > priority);
-        }
+        const bool resolve = (import->priority == 0) || (import->priority > priority);
 
         if (resolve) {
             // This is the (current) best resolution for this import
@@ -706,8 +710,7 @@ bool QQmlTypeLoader::Blob::qmldirDataAvailable(const QQmlRefPointer<QQmlQmldirDa
                 return false;
             }
 
-            if (it != m_unresolvedImports.end())
-                *it = priority;
+            import->priority = priority;
             return true;
         }
     }
diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h
index 5710bdba56..f8cec700a5 100644
--- a/src/qml/qml/qqmltypeloader_p.h
+++ b/src/qml/qml/qqmltypeloader_p.h
@@ -61,6 +61,8 @@
 #include <QtCore/qcache.h>
 #include <QtCore/qmutex.h>
 
+#include <memory>
+
 QT_BEGIN_NAMESPACE
 
 class QQmlScriptBlob;
@@ -87,11 +89,31 @@ public:
 
         void setCachedUnitStatus(QQmlMetaType::CachedUnitLookupError status) { m_cachedUnitStatus = status; }
 
+        struct PendingImport
+        {
+            QV4::CompiledData::Import::ImportType type = QV4::CompiledData::Import::ImportType::ImportLibrary;
+
+            QString uri;
+            QString qualifier;
+
+            int majorVersion = -1;
+            int minorVersion = -1;
+
+            QV4::CompiledData::Location location;
+
+            int priority = 0;
+
+            PendingImport() = default;
+            PendingImport(Blob *blob, const QV4::CompiledData::Import *import);
+        };
+        using PendingImportPtr = std::shared_ptr<PendingImport>;
+
     protected:
         bool addImport(const QV4::CompiledData::Import *import, QList<QQmlError> *errors);
+        bool addImport(PendingImportPtr import, QList<QQmlError> *errors);
 
-        bool fetchQmldir(const QUrl &url, const QV4::CompiledData::Import *import, int priority, QList<QQmlError> *errors);
-        bool updateQmldir(const QQmlRefPointer<QQmlQmldirData> &data, const QV4::CompiledData::Import *import, QList<QQmlError> *errors);
+        bool fetchQmldir(const QUrl &url, PendingImportPtr import, int priority, QList<QQmlError> *errors);
+        bool updateQmldir(const QQmlRefPointer<QQmlQmldirData> &data, PendingImportPtr import, QList<QQmlError> *errors);
 
     private:
         virtual bool qmldirDataAvailable(const QQmlRefPointer<QQmlQmldirData> &, QList<QQmlError> *);
@@ -109,7 +131,7 @@ public:
         static bool diskCacheForced();
 
         QQmlImports m_importCache;
-        QHash<const QV4::CompiledData::Import*, int> m_unresolvedImports;
+        QVector<PendingImportPtr> m_unresolvedImports;
         QVector<QQmlRefPointer<QQmlQmldirData>> m_qmldirs;
         QQmlMetaType::CachedUnitLookupError m_cachedUnitStatus = QQmlMetaType::CachedUnitLookupError::NoError;
     };