From 9ea0380a4dd5a681f1675cb5203e7a8c71764704 Mon Sep 17 00:00:00 2001
From: Christian Kamm <christian.d.kamm@nokia.com>
Date: Fri, 1 Jul 2011 13:41:40 +0200
Subject: [PATCH] QmlJS: Enforce Context always being linked.

The default way of creating a Context is through Link.

Change-Id: Ia81f7ce90ba2b33d02eed432a61be836d404bedd
Reviewed-on: http://codereview.qt.nokia.com/1041
Reviewed-by: Fawzi Mohamed <fawzi.mohamed@nokia.com>
---
 src/libs/qmljs/qmljsinterpreter.cpp           | 12 ++---
 src/libs/qmljs/qmljsinterpreter.h             |  8 ++--
 src/libs/qmljs/qmljslink.cpp                  | 45 +++++++++----------
 src/libs/qmljs/qmljslink.h                    | 11 ++---
 src/libs/qmljs/qmljslookupcontext.cpp         | 20 ++++-----
 .../designercore/model/texttomodelmerger.cpp  | 11 +++--
 .../qmljseditor/qmljsfindreferences.cpp       |  5 +--
 .../qmljseditor/qmljssemantichighlighter.cpp  |  5 +--
 8 files changed, 50 insertions(+), 67 deletions(-)

diff --git a/src/libs/qmljs/qmljsinterpreter.cpp b/src/libs/qmljs/qmljsinterpreter.cpp
index dc64264e65a..8877dcfebec 100644
--- a/src/libs/qmljs/qmljsinterpreter.cpp
+++ b/src/libs/qmljs/qmljsinterpreter.cpp
@@ -779,9 +779,10 @@ QList<const ObjectValue *> ScopeChain::all() const
 }
 
 
-Context::Context(const QmlJS::Snapshot &snapshot)
+Context::Context(const QmlJS::Snapshot &snapshot, ValueOwner *valueOwner, const ImportsPerDocument &imports)
     : _snapshot(snapshot),
-      _valueOwner(new ValueOwner),
+      _valueOwner(valueOwner),
+      _imports(imports),
       _qmlScopeObjectIndex(-1),
       _qmlScopeObjectSet(false)
 {
@@ -819,13 +820,6 @@ const Imports *Context::imports(const QmlJS::Document *doc) const
     return _imports.value(doc).data();
 }
 
-void Context::setImports(const QmlJS::Document *doc, const Imports *imports)
-{
-    if (!doc)
-        return;
-    _imports[doc] = QSharedPointer<const Imports>(imports);
-}
-
 const Value *Context::lookup(const QString &name, const ObjectValue **foundInScope) const
 {
     QList<const ObjectValue *> scopes = _scopeChain.all();
diff --git a/src/libs/qmljs/qmljsinterpreter.h b/src/libs/qmljs/qmljsinterpreter.h
index eb5ce51003f..53f682556ae 100644
--- a/src/libs/qmljs/qmljsinterpreter.h
+++ b/src/libs/qmljs/qmljsinterpreter.h
@@ -319,7 +319,10 @@ private:
 class QMLJS_EXPORT Context
 {
 public:
-    Context(const Snapshot &snapshot);
+    typedef QHash<const Document *, QSharedPointer<const Imports> > ImportsPerDocument;
+
+    // Context takes ownership of valueOwner
+    Context(const Snapshot &snapshot, ValueOwner *valueOwner, const ImportsPerDocument &imports);
     ~Context();
 
     ValueOwner *valueOwner() const;
@@ -329,7 +332,6 @@ public:
     ScopeChain &scopeChain();
 
     const Imports *imports(const Document *doc) const;
-    void setImports(const Document *doc, const Imports *imports);
 
     const Value *lookup(const QString &name, const ObjectValue **foundInScope = 0) const;
     const ObjectValue *lookupType(const Document *doc, AST::UiQualifiedId *qmlTypeName,
@@ -344,7 +346,7 @@ private:
 
     Snapshot _snapshot;
     QSharedPointer<ValueOwner> _valueOwner;
-    QHash<const Document *, QSharedPointer<const Imports> > _imports;
+    ImportsPerDocument _imports;
     ScopeChain _scopeChain;
     int _qmlScopeObjectIndex;
     bool _qmlScopeObjectSet;
diff --git a/src/libs/qmljs/qmljslink.cpp b/src/libs/qmljs/qmljslink.cpp
index f1b3a622918..76de63cc465 100644
--- a/src/libs/qmljs/qmljslink.cpp
+++ b/src/libs/qmljs/qmljslink.cpp
@@ -84,7 +84,8 @@ class QmlJS::LinkPrivate
 {
 public:
     Snapshot snapshot;
-    Interpreter::Context *context;
+    Interpreter::ValueOwner *valueOwner;
+    Interpreter::Context::ImportsPerDocument imports;
     QStringList importPaths;
 
     QHash<ImportCacheKey, Import> importCache;
@@ -108,11 +109,11 @@ public:
     \l{Context} with \l{Link}.
 */
 
-Link::Link(Context *context, const Snapshot &snapshot, const QStringList &importPaths)
+Link::Link(const Snapshot &snapshot, const QStringList &importPaths)
     : d_ptr(new LinkPrivate)
 {
     Q_D(Link);
-    d->context = context;
+    d->valueOwner = new ValueOwner;
     d->snapshot = snapshot;
     d->importPaths = importPaths;
 
@@ -123,54 +124,50 @@ Link::Link(Context *context, const Snapshot &snapshot, const QStringList &import
     ModelManagerInterface *modelManager = ModelManagerInterface::instance();
     if (modelManager) {
         foreach (const QList<FakeMetaObject::ConstPtr> &cppTypes, modelManager->cppQmlTypes()) {
-            valueOwner()->cppQmlTypes().load(valueOwner(), cppTypes);
+            d->valueOwner->cppQmlTypes().load(d->valueOwner, cppTypes);
         }
     }
 }
 
-void Link::operator()(QHash<QString, QList<DiagnosticMessage> > *messages)
+Context Link::operator()(QHash<QString, QList<DiagnosticMessage> > *messages)
 {
     Q_D(Link);
     d->allDiagnosticMessages = messages;
     linkImports();
+    return Context(d->snapshot, d->valueOwner, d->imports);
 }
 
-void Link::operator()(const Document::Ptr &doc, QList<DiagnosticMessage> *messages)
+Context Link::operator()(const Document::Ptr &doc, QList<DiagnosticMessage> *messages)
 {
     Q_D(Link);
     d->doc = doc;
     d->diagnosticMessages = messages;
     linkImports();
+    return Context(d->snapshot, d->valueOwner, d->imports);
 }
 
 Link::~Link()
 {
 }
 
-Interpreter::ValueOwner *Link::valueOwner()
-{
-    Q_D(Link);
-    return d->context->valueOwner();
-}
-
 void Link::linkImports()
 {
     Q_D(Link);
 
     if (d->doc) {
         // do it on d->doc first, to make sure import errors are shown
-        Imports *imports = new Imports(valueOwner());
+        Imports *imports = new Imports(d->valueOwner);
         populateImportedTypes(imports, d->doc);
-        d->context->setImports(d->doc.data(), imports);
+        d->imports.insert(d->doc.data(), QSharedPointer<Imports>(imports));
     }
 
     foreach (Document::Ptr doc, d->snapshot) {
         if (doc == d->doc)
             continue;
 
-        Imports *imports = new Imports(valueOwner());
+        Imports *imports = new Imports(d->valueOwner);
         populateImportedTypes(imports, doc);
-        d->context->setImports(doc.data(), imports);
+        d->imports.insert(doc.data(), QSharedPointer<Imports>(imports));
     }
 }
 
@@ -238,7 +235,7 @@ Import Link::importFileOrDirectory(Document::Ptr doc, const ImportInfo &importIn
 
     if (importInfo.type() == ImportInfo::DirectoryImport
             || importInfo.type() == ImportInfo::ImplicitDirectoryImport) {
-        import.object = new ObjectValue(valueOwner());
+        import.object = new ObjectValue(d->valueOwner);
 
         importLibrary(doc, path, &import);
 
@@ -269,7 +266,7 @@ Import Link::importNonFile(Document::Ptr doc, const ImportInfo &importInfo)
 
     Import import;
     import.info = importInfo;
-    import.object = new ObjectValue(valueOwner());
+    import.object = new ObjectValue(d->valueOwner);
 
     const QString packageName = Bind::toString(importInfo.ast()->importUri, '.');
     const ComponentVersion version = importInfo.version();
@@ -308,10 +305,10 @@ Import Link::importNonFile(Document::Ptr doc, const ImportInfo &importInfo)
     }
 
     // if there are cpp-based types for this package, use them too
-    if (valueOwner()->cppQmlTypes().hasPackage(packageName)) {
+    if (d->valueOwner->cppQmlTypes().hasPackage(packageName)) {
         importFound = true;
         foreach (QmlObjectValue *object,
-                 valueOwner()->cppQmlTypes().typesForImport(packageName, version)) {
+                 d->valueOwner->cppQmlTypes().typesForImport(packageName, version)) {
             import.object->setMember(object->className(), object);
         }
     }
@@ -382,7 +379,7 @@ bool Link::importLibrary(Document::Ptr doc,
             }
         } else {
             QList<QmlObjectValue *> loadedObjects =
-                    valueOwner()->cppQmlTypes().load(valueOwner(), libraryInfo.metaObjects());
+                    d->valueOwner->cppQmlTypes().load(d->valueOwner, libraryInfo.metaObjects());
             foreach (QmlObjectValue *object, loadedObjects) {
                 if (object->packageName().isEmpty()) {
                     import->object->setMember(object->className(), object);
@@ -479,14 +476,14 @@ void Link::loadImplicitDefaultImports(Imports *imports)
     Q_D(Link);
 
     const QString defaultPackage = CppQmlTypes::defaultPackage;
-    if (valueOwner()->cppQmlTypes().hasPackage(defaultPackage)) {
+    if (d->valueOwner->cppQmlTypes().hasPackage(defaultPackage)) {
         ImportInfo info(ImportInfo::LibraryImport, defaultPackage);
         Import import = d->importCache.value(ImportCacheKey(info));
         if (!import.object) {
             import.info = info;
-            import.object = new ObjectValue(valueOwner());
+            import.object = new ObjectValue(d->valueOwner);
             foreach (QmlObjectValue *object,
-                     valueOwner()->cppQmlTypes().typesForImport(defaultPackage, ComponentVersion())) {
+                     d->valueOwner->cppQmlTypes().typesForImport(defaultPackage, ComponentVersion())) {
                 import.object->setMember(object->className(), object);
             }
             d->importCache.insert(ImportCacheKey(info), import);
diff --git a/src/libs/qmljs/qmljslink.h b/src/libs/qmljs/qmljslink.h
index 218f22d10fb..bbd8bd2e305 100644
--- a/src/libs/qmljs/qmljslink.h
+++ b/src/libs/qmljs/qmljslink.h
@@ -34,7 +34,7 @@
 #define QMLJSLINK_H
 
 #include <qmljs/qmljsdocument.h>
-#include <qmljs/qmljsinterpreter.h>
+#include <qmljs/qmljsvalueowner.h>
 #include <qmljs/parser/qmljsastfwd_p.h>
 #include <languageutils/componentversion.h>
 
@@ -54,21 +54,18 @@ class QMLJS_EXPORT Link
     Q_DECLARE_TR_FUNCTIONS(QmlJS::Link)
 
 public:
-    Link(Interpreter::Context *context, const Snapshot &snapshot,
-         const QStringList &importPaths);
+    Link(const Snapshot &snapshot, const QStringList &importPaths);
 
     // Link all documents in snapshot, collecting all diagnostic messages (if messages != 0)
-    void operator()(QHash<QString, QList<DiagnosticMessage> > *messages = 0);
+    Interpreter::Context operator()(QHash<QString, QList<DiagnosticMessage> > *messages = 0);
 
     // Link all documents in snapshot, appending the diagnostic messages
     // for 'doc' in 'messages'
-    void operator()(const Document::Ptr &doc, QList<DiagnosticMessage> *messages);
+    Interpreter::Context operator()(const Document::Ptr &doc, QList<DiagnosticMessage> *messages);
 
     ~Link();
 
 private:
-    Interpreter::ValueOwner *valueOwner();
-
     static AST::UiQualifiedId *qualifiedTypeNameId(AST::Node *node);
 
     void linkImports();
diff --git a/src/libs/qmljs/qmljslookupcontext.cpp b/src/libs/qmljs/qmljslookupcontext.cpp
index de371209299..97275b0eea6 100644
--- a/src/libs/qmljs/qmljslookupcontext.cpp
+++ b/src/libs/qmljs/qmljslookupcontext.cpp
@@ -43,31 +43,27 @@ class QmlJS::LookupContextData
 {
 public:
     LookupContextData(Document::Ptr doc, const Snapshot &snapshot, const QList<AST::Node *> &path)
-        : context(snapshot),
-          doc(doc)
+        : doc(doc)
+        , context(Link(snapshot, ModelManagerInterface::instance()->importPaths())())
     {
-        // since we keep the document and snapshot around, we don't need to keep the Link instance
-        Link link(&context, snapshot, ModelManagerInterface::instance()->importPaths());
-        link();
-
         ScopeBuilder scopeBuilder(&context, doc);
         scopeBuilder.initializeRootScope();
         scopeBuilder.push(path);
     }
 
     LookupContextData(Document::Ptr doc,
-                      const Interpreter::Context &linkedContextWithoutScope,
+                      const Interpreter::Context &contextWithoutScope,
                       const QList<AST::Node *> &path)
-        : context(linkedContextWithoutScope),
-          doc(doc)
+        : doc(doc)
+        , context(contextWithoutScope)
     {
         ScopeBuilder scopeBuilder(&context, doc);
         scopeBuilder.initializeRootScope();
         scopeBuilder.push(path);
     }
 
-    Interpreter::Context context;
     Document::Ptr doc;
+    Interpreter::Context context;
     // snapshot is in context
 };
 
@@ -77,9 +73,9 @@ LookupContext::LookupContext(Document::Ptr doc, const Snapshot &snapshot, const
 }
 
 LookupContext::LookupContext(const Document::Ptr doc,
-                             const Interpreter::Context &linkedContextWithoutScope,
+                             const Interpreter::Context &contextWithoutScope,
                              const QList<AST::Node *> &path)
-    : d(new LookupContextData(doc, linkedContextWithoutScope, path))
+    : d(new LookupContextData(doc, contextWithoutScope, path))
 {
 }
 
diff --git a/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp b/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp
index 8de2db0d95e..f8a8b99acdd 100644
--- a/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp
+++ b/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp
@@ -332,13 +332,12 @@ public:
                    const QStringList importPaths)
         : m_snapshot(snapshot)
         , m_doc(doc)
-        , m_context(new Interpreter::Context(snapshot))
-        , m_link(m_context, snapshot, importPaths)
+        , m_link(snapshot, importPaths)
+        , m_context(new Interpreter::Context(m_link(doc, &m_diagnosticLinkMessages)))
         , m_scopeBuilder(m_context, doc)
     {
-        m_link(doc, &m_diagnosticLinkMessages);
         m_lookupContext = LookupContext::create(doc, *m_context, QList<AST::Node*>());
-        // cheaper than calling m_scopeBuilder.initializeRootScope()
+        // cheaper than calling m_scopeBuilder.initializeRootScope() again
         *m_context = *m_lookupContext->context();
     }
 
@@ -629,9 +628,9 @@ public:
 private:
     Snapshot m_snapshot;
     Document::Ptr m_doc;
-    Interpreter::Context *m_context;
-    QList<DiagnosticMessage> m_diagnosticLinkMessages;
     Link m_link;
+    QList<DiagnosticMessage> m_diagnosticLinkMessages;
+    Interpreter::Context *m_context;
     LookupContext::Ptr m_lookupContext;
     ScopeBuilder m_scopeBuilder;
 };
diff --git a/src/plugins/qmljseditor/qmljsfindreferences.cpp b/src/plugins/qmljseditor/qmljsfindreferences.cpp
index 9f3f1c563b1..210982f7a4f 100644
--- a/src/plugins/qmljseditor/qmljsfindreferences.cpp
+++ b/src/plugins/qmljseditor/qmljsfindreferences.cpp
@@ -812,13 +812,12 @@ static void find_helper(QFutureInterface<FindReferences::Usage> &future,
     }
 
     // find the scope for the name we're searching
-    Context context(snapshot);
     Document::Ptr doc = snapshot.document(fileName);
     if (!doc)
         return;
 
-    Link link(&context, snapshot, ModelManagerInterface::instance()->importPaths());
-    link();
+    Link link(snapshot, ModelManagerInterface::instance()->importPaths());
+    Context context = link();
 
     ScopeBuilder builder(&context, doc);
     builder.initializeRootScope();
diff --git a/src/plugins/qmljseditor/qmljssemantichighlighter.cpp b/src/plugins/qmljseditor/qmljssemantichighlighter.cpp
index 04d5ac500d9..30dc4edd57d 100644
--- a/src/plugins/qmljseditor/qmljssemantichighlighter.cpp
+++ b/src/plugins/qmljseditor/qmljssemantichighlighter.cpp
@@ -132,9 +132,8 @@ SemanticInfo SemanticHighlighter::semanticInfo(const SemanticHighlighterSource &
     semanticInfo.snapshot = snapshot;
     semanticInfo.document = doc;
 
-    QmlJS::Interpreter::Context *ctx = new QmlJS::Interpreter::Context(snapshot);
-    QmlJS::Link link(ctx, snapshot, QmlJS::ModelManagerInterface::instance()->importPaths());
-    link(doc, &semanticInfo.semanticMessages);
+    QmlJS::Link link(snapshot, QmlJS::ModelManagerInterface::instance()->importPaths());
+    QmlJS::Interpreter::Context *ctx = new QmlJS::Interpreter::Context(link(doc, &semanticInfo.semanticMessages));
     semanticInfo.m_context = QSharedPointer<const QmlJS::Interpreter::Context>(ctx);
 
     QmlJS::Check checker(doc, ctx);
-- 
GitLab