Expose let/const variables from imported JS scripts

This patch allows QML to access let/const variables defined in JS files.

Detailed changes:

  - The recently added ContextType::ScriptImportedByQML is changed to avoid
    creating Push/PopScriptContext instructions, similar to
    ContextType::ESModule.

  - QV4::Module is changed to also work with CompilationUnits which are not
    ESModules. In this case QV4::Module will behave as if all lexically scoped
    variables were exported.

  - CompilationUnit is changed to support instantiating and evaluating
    QV4::Modules for non-ESModules as well.

  - QQmlTypeLoader is changed to always create QV4::Modules for evaluating
    scripts. For the non-ESModule case, the QV4::Module is evaluated inside a
    QV4::QmlContext, as before.

  - A pointer to the QV4::Module is added to QV4::QQmlContextWrapper, and used
    in virtualGet to access the let/const variables in the CallContext. Access
    is read-only.

Fixes: QTBUG-69408
Change-Id: I6f299363fdf5e1c5a4a0f1d9e655b4dc5112dd00
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

10 files changed, 140 insertions, 104 deletions

diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp
index 244e762faf..39f48f67f8 100644
--- a/src/qml/compiler/qv4compileddata.cpp
+++ b/src/qml/compiler/qv4compileddata.cpp
@@ -381,13 +381,20 @@ QStringList CompilationUnit::moduleRequests() const
 
 Heap::Module *CompilationUnit::instantiate(ExecutionEngine *engine)
 {
-    if (m_module)
+    if (isESModule() && m_module)
         return m_module;
 
+    if (data->indexOfRootFunction < 0)
+        return nullptr;
+
     if (!this->engine)
         linkToEngine(engine);
 
-    m_module = engine->memoryManager->allocate<Module>(engine, this);
+    Scope scope(engine);
+    Scoped<Module> module(scope, engine->memoryManager->allocate<Module>(engine, this));
+
+    if (isESModule())
+        m_module = module->d();
 
     for (const QString &request: moduleRequests()) {
         auto dependentModuleUnit = engine->loadModule(QUrl(request), this);
@@ -396,7 +403,6 @@ Heap::Module *CompilationUnit::instantiate(ExecutionEngine *engine)
         dependentModuleUnit->instantiate(engine);
     }
 
-    Scope scope(engine);
     ScopedString importName(scope);
 
     const uint importCount = data->importEntryTableSize;
@@ -431,7 +437,7 @@ Heap::Module *CompilationUnit::instantiate(ExecutionEngine *engine)
         }
     }
 
-    return m_module;
+    return module->d();
 }
 
 const Value *CompilationUnit::resolveExport(QV4::String *exportName)
@@ -556,10 +562,13 @@ void CompilationUnit::getExportedNamesRecursively(QStringList *names, QVector<co
 
 void CompilationUnit::evaluate()
 {
-    if (m_moduleEvaluated)
-        return;
-    m_moduleEvaluated = true;
+    QV4::Scope scope(engine);
+    QV4::Scoped<Module> module(scope, m_module);
+    module->evaluate();
+}
 
+void CompilationUnit::evaluateModuleRequests()
+{
     for (const QString &request: moduleRequests()) {
         auto dependentModuleUnit = engine->loadModule(QUrl(request), this);
         if (engine->hasException)
@@ -568,19 +577,6 @@ void CompilationUnit::evaluate()
         if (engine->hasException)
             return;
     }
-
-    QV4::Function *moduleFunction = runtimeFunctions[data->indexOfRootFunction];
-    CppStackFrame frame;
-    frame.init(engine, moduleFunction, nullptr, 0);
-    frame.setupJSFrame(engine->jsStackTop, Value::undefinedValue(), m_module->scope,
-                       Value::undefinedValue(), Value::undefinedValue());
-
-    frame.push();
-    engine->jsStackTop += frame.requiredJSStackFrameSize();
-    auto frameCleanup = qScopeGuard([&frame]() {
-       frame.pop();
-    });
-    Moth::VME::exec(&frame, engine);
 }
 
 bool CompilationUnit::loadFromDisk(const QUrl &url, const QDateTime &sourceTimeStamp, QString *errorString)
diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h
index b36b1a91ea..5c03303029 100644
--- a/src/qml/compiler/qv4compileddata_p.h
+++ b/src/qml/compiler/qv4compileddata_p.h
@@ -1183,6 +1183,7 @@ public:
     const Value *resolveExport(QV4::String *exportName);
     QStringList exportedNames() const;
     void evaluate();
+    void evaluateModuleRequests();
 
     QV4::Function *linkToEngine(QV4::ExecutionEngine *engine);
     void unlink();
@@ -1225,7 +1226,6 @@ private:
     Q_NEVER_INLINE IdentifierHash createNamedObjectsPerComponent(int componentObjectIndex);
 
     Heap::Module *m_module = nullptr;
-    bool m_moduleEvaluated = false;
 
 public:
 #if defined(V4_BOOTSTRAP)
diff --git a/src/qml/compiler/qv4compilercontext.cpp b/src/qml/compiler/qv4compilercontext.cpp
index 572f24f148..ca4cbfc4fc 100644
--- a/src/qml/compiler/qv4compilercontext.cpp
+++ b/src/qml/compiler/qv4compilercontext.cpp
@@ -207,7 +207,7 @@ void Context::emitBlockHeader(Codegen *codegen)
             blockIndex = codegen->module()->blocks.count() - 1;
         }
 
-        if (contextType == ContextType::Global || contextType == ContextType::ScriptImportedByQML) {
+        if (contextType == ContextType::Global) {
             Instruction::PushScriptContext scriptContext;
             scriptContext.index = blockIndex;
             bytecodeGenerator->addInstruction(scriptContext);
@@ -222,7 +222,7 @@ void Context::emitBlockHeader(Codegen *codegen)
                 blockContext.index = blockIndex;
                 bytecodeGenerator->addInstruction(blockContext);
             }
-        } else if (contextType != ContextType::ESModule) {
+        } else if (contextType != ContextType::ESModule && contextType != ContextType::ScriptImportedByQML) {
             Instruction::CreateCallContext createContext;
             bytecodeGenerator->addInstruction(createContext);
         }
@@ -316,9 +316,9 @@ void Context::emitBlockFooter(Codegen *codegen)
 
 QT_WARNING_PUSH
 QT_WARNING_DISABLE_GCC("-Wmaybe-uninitialized") // the loads below are empty structs.
-    if (contextType == ContextType::Global || contextType == ContextType::ScriptImportedByQML)
+    if (contextType == ContextType::Global)
         bytecodeGenerator->addInstruction(Instruction::PopScriptContext());
-    else if (contextType != ContextType::ESModule)
+    else if (contextType != ContextType::ESModule && contextType != ContextType::ScriptImportedByQML)
         bytecodeGenerator->addInstruction(Instruction::PopContext());
 QT_WARNING_POP
 }
diff --git a/src/qml/jsruntime/qv4module.cpp b/src/qml/jsruntime/qv4module.cpp
index 19a036374f..237ada8321 100644
--- a/src/qml/jsruntime/qv4module.cpp
+++ b/src/qml/jsruntime/qv4module.cpp
@@ -46,6 +46,8 @@
 #include <private/qv4symbol_p.h>
 #include <private/qv4identifiertable_p.h>
 
+#include <QScopeGuard>
+
 using namespace QV4;
 
 DEFINE_OBJECT_VTABLE(Module);
@@ -98,20 +100,60 @@ void Heap::Module::init(ExecutionEngine *engine, CompiledData::CompilationUnit *
     This->setPrototypeUnchecked(nullptr);
 }
 
+void Module::evaluate()
+{
+    if (d()->evaluated)
+        return;
+    d()->evaluated = true;
+
+    CompiledData::CompilationUnit *unit = d()->unit;
+
+    unit->evaluateModuleRequests();
+
+    ExecutionEngine *v4 = engine();
+    Function *moduleFunction = unit->runtimeFunctions[unit->data->indexOfRootFunction];
+    CppStackFrame frame;
+    frame.init(v4, moduleFunction, nullptr, 0);
+    frame.setupJSFrame(v4->jsStackTop, Value::undefinedValue(), d()->scope,
+                       Value::undefinedValue(), Value::undefinedValue());
+
+    frame.push();
+    v4->jsStackTop += frame.requiredJSStackFrameSize();
+    auto frameCleanup = qScopeGuard([&frame]() {
+        frame.pop();
+    });
+    Moth::VME::exec(&frame, v4);
+}
+
+const Value *Module::resolveExport(PropertyKey id) const
+{
+    if (d()->unit->isESModule()) {
+        if (!id.isString())
+            return nullptr;
+        Scope scope(engine());
+        ScopedString name(scope, id.asStringOrSymbol());
+        return d()->unit->resolveExport(name);
+    } else {
+        InternalClassEntry entry = d()->scope->internalClass->find(id);
+        if (entry.isValid())
+            return &d()->scope->locals[entry.index];
+        return nullptr;
+    }
+}
+
 ReturnedValue Module::virtualGet(const Managed *m, PropertyKey id, const Value *receiver, bool *hasProperty)
 {
     if (id.isSymbol())
         return Object::virtualGet(m, id, receiver, hasProperty);
 
     const Module *module = static_cast<const Module *>(m);
-    Scope scope(m->engine());
-    ScopedString expectedName(scope, id.toStringOrSymbol(scope.engine));
-    const Value *v = module->d()->unit->resolveExport(expectedName);
+    const Value *v = module->resolveExport(id);
     if (hasProperty)
         *hasProperty = v != nullptr;
     if (!v)
         return Encode::undefined();
     if (v->isEmpty()) {
+        Scope scope(m->engine());
         ScopedValue propName(scope, id.toStringOrSymbol(scope.engine));
         return scope.engine->throwReferenceError(propName);
     }
@@ -124,9 +166,7 @@ PropertyAttributes Module::virtualGetOwnProperty(const Managed *m, PropertyKey i
         return Object::virtualGetOwnProperty(m, id, p);
 
     const Module *module = static_cast<const Module *>(m);
-    Scope scope(m->engine());
-    ScopedString expectedName(scope, id.toStringOrSymbol(scope.engine));
-    const Value *v = module->d()->unit->resolveExport(expectedName);
+    const Value *v = module->resolveExport(id);
     if (!v) {
         if (p)
             p->value = Encode::undefined();
@@ -135,6 +175,7 @@ PropertyAttributes Module::virtualGetOwnProperty(const Managed *m, PropertyKey i
     if (p)
         p->value = v->isEmpty() ? Encode::undefined() : v->asReturnedValue();
     if (v->isEmpty()) {
+        Scope scope(m->engine());
         ScopedValue propName(scope, id.toStringOrSymbol(scope.engine));
         scope.engine->throwReferenceError(propName);
     }
@@ -147,9 +188,7 @@ bool Module::virtualHasProperty(const Managed *m, PropertyKey id)
         return Object::virtualHasProperty(m, id);
 
     const Module *module = static_cast<const Module *>(m);
-    Scope scope(m->engine());
-    ScopedString expectedName(scope, id.toStringOrSymbol(scope.engine));
-    const Value *v = module->d()->unit->resolveExport(expectedName);
+    const Value *v = module->resolveExport(id);
     return v != nullptr;
 }
 
@@ -173,11 +212,7 @@ bool Module::virtualDeleteProperty(Managed *m, PropertyKey id)
     if (id.isSymbol())
         return Object::virtualDeleteProperty(m, id);
     const Module *module = static_cast<const Module *>(m);
-    Scope scope(m->engine());
-    ScopedString expectedName(scope, id.toStringOrSymbol(scope.engine));
-    if (!expectedName)
-        return true;
-    const Value *v = module->d()->unit->resolveExport(expectedName);
+    const Value *v = module->resolveExport(id);
     if (v)
         return false;
     return true;
@@ -202,7 +237,7 @@ PropertyKey ModuleNamespaceIterator::next(const Object *o, Property *pd, Propert
         Scope scope(module->engine());
         ScopedString exportName(scope, scope.engine->newString(exportedNames.at(exportIndex)));
         exportIndex++;
-        const Value *v = module->d()->unit->resolveExport(exportName);
+        const Value *v = module->resolveExport(exportName->toPropertyKey());
         if (pd) {
             if (v->isEmpty())
                 scope.engine->throwReferenceError(exportName);
@@ -218,7 +253,17 @@ OwnPropertyKeyIterator *Module::virtualOwnPropertyKeys(const Object *o, Value *t
 {
     const Module *module = static_cast<const Module *>(o);
     *target = *o;
-    return new ModuleNamespaceIterator(module->d()->unit->exportedNames());
+
+    QStringList names;
+    if (module->d()->unit->isESModule()) {
+        names = module->d()->unit->exportedNames();
+    } else {
+        Heap::InternalClass *scopeClass = module->d()->scope->internalClass;
+        for (uint i = 0; i < scopeClass->size; ++i)
+            names << scopeClass->keyAt(i);
+    }
+
+    return new ModuleNamespaceIterator(names);
 }
 
 Heap::Object *Module::virtualGetPrototypeOf(const Managed *)
diff --git a/src/qml/jsruntime/qv4module_p.h b/src/qml/jsruntime/qv4module_p.h
index 0cab161b82..dca0678fe9 100644
--- a/src/qml/jsruntime/qv4module_p.h
+++ b/src/qml/jsruntime/qv4module_p.h
@@ -62,7 +62,8 @@ namespace Heap {
 #define ModuleMembers(class, Member) \
     Member(class, NoMark, CompiledData::CompilationUnit *, unit) \
     Member(class, Pointer, CallContext *, scope) \
-    Member(class, HeapValue, HeapValue, self)
+    Member(class, HeapValue, HeapValue, self) \
+    Member(class, NoMark, bool, evaluated)
 
 DECLARE_EXPORTED_HEAP_OBJECT(Module, Object) {
     DECLARE_MARKOBJECTS(Module)
@@ -75,6 +76,9 @@ DECLARE_EXPORTED_HEAP_OBJECT(Module, Object) {
 struct Q_QML_EXPORT Module : public Object {
     V4_OBJECT2(Module, Object)
 
+    void evaluate();
+    const Value *resolveExport(PropertyKey key) const;
+
     static ReturnedValue virtualGet(const Managed *m, PropertyKey id, const Value *receiver, bool *hasProperty);
     static PropertyAttributes virtualGetOwnProperty(const Managed *m, PropertyKey id, Property *p);
     static bool virtualHasProperty(const Managed *m, PropertyKey id);
diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp
index cc0b0feeee..88b0822f42 100644
--- a/src/qml/jsruntime/qv4qmlcontext.cpp
+++ b/src/qml/jsruntime/qv4qmlcontext.cpp
@@ -54,6 +54,7 @@
 #include <private/qqmljavascriptexpression_p.h>
 #include <private/qjsvalue_p.h>
 #include <private/qv4qobjectwrapper_p.h>
+#include <private/qv4module_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -87,8 +88,20 @@ ReturnedValue QQmlContextWrapper::virtualGet(const Managed *m, PropertyKey id, c
     QV4::ExecutionEngine *v4 = resource->engine();
     QV4::Scope scope(v4);
 
-    if (v4->callingQmlContext() != *resource->d()->context)
+    if (v4->callingQmlContext() != *resource->d()->context) {
+        if (resource->d()->module) {
+            Scoped<Module> module(scope, resource->d()->module);
+            bool hasProp = false;
+            ScopedValue value(scope, module->get(id, receiver, &hasProp));
+            if (hasProp) {
+                if (hasProperty)
+                    *hasProperty = hasProp;
+                return value->asReturnedValue();
+            }
+        }
+
         return Object::virtualGet(m, id, receiver, hasProperty);
+    }
 
     bool hasProp = false;
     ScopedValue result(scope, Object::virtualGet(m, id, receiver, &hasProp));
diff --git a/src/qml/jsruntime/qv4qmlcontext_p.h b/src/qml/jsruntime/qv4qmlcontext_p.h
index 4fe34a0a06..dd6de3323d 100644
--- a/src/qml/jsruntime/qv4qmlcontext_p.h
+++ b/src/qml/jsruntime/qv4qmlcontext_p.h
@@ -66,7 +66,12 @@ struct QQmlContextWrapper;
 
 namespace Heap {
 
-struct QQmlContextWrapper : Object {
+#define QQmlContextWrapperMembers(class, Member) \
+    Member(class, Pointer, Module *, module)
+
+DECLARE_HEAP_OBJECT(QQmlContextWrapper, Object) {
+    DECLARE_MARKOBJECTS(QQmlContextWrapper);
+
     void init(QQmlContextData *context, QObject *scopeObject);
     void destroy();
 
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 89b023c164..7480475ca7 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -2881,30 +2881,9 @@ void QQmlTypeData::scriptImported(const QQmlRefPointer<QQmlScriptBlob> &blob, co
 QQmlScriptData::QQmlScriptData()
     : typeNameCache(nullptr)
     , m_loaded(false)
-    , m_program(nullptr)
 {
 }
 
-QQmlScriptData::~QQmlScriptData()
-{
-    delete m_program;
-}
-
-void QQmlScriptData::initialize(QQmlEngine *engine)
-{
-    Q_ASSERT(!m_program);
-    Q_ASSERT(engine);
-    Q_ASSERT(!hasEngine());
-
-    QV4::ExecutionEngine *v4 = engine->handle();
-
-    m_program = new QV4::Script(v4, nullptr, m_precompiledScript);
-
-    addToEngine(engine);
-
-    addref();
-}
-
 QQmlContextData *QQmlScriptData::qmlContextDataForContext(QQmlContextData *parentQmlContextData)
 {
     Q_ASSERT(parentQmlContextData && parentQmlContextData->engine);
@@ -2954,57 +2933,54 @@ QQmlContextData *QQmlScriptData::qmlContextDataForContext(QQmlContextData *paren
     return qmlContextData;
 }
 
-QV4::ReturnedValue QQmlScriptData::scriptValueForContext(QQmlContextData *parentCtxt)
+QV4::ReturnedValue QQmlScriptData::scriptValueForContext(QQmlContextData *parentQmlContextData)
 {
     if (m_loaded)
         return m_value.value();
 
-    Q_ASSERT(parentCtxt && parentCtxt->engine);
-    QV4::ExecutionEngine *v4 = parentCtxt->engine->handle();
-
-    if (m_precompiledScript->isESModule()) {
-        m_loaded = true;
-
-        m_value.set(v4, m_precompiledScript->instantiate(v4));
-        if (!m_value.isNullOrUndefined())
-            m_precompiledScript->evaluate();
+    Q_ASSERT(parentQmlContextData && parentQmlContextData->engine);
+    QV4::ExecutionEngine *v4 = parentQmlContextData->engine->handle();
+    QV4::Scope scope(v4);
 
-        return m_value.value();
+    if (!hasEngine()) {
+        addToEngine(parentQmlContextData->engine);
+        addref();
     }
 
-    QQmlEnginePrivate *ep = QQmlEnginePrivate::get(parentCtxt->engine);
-    QV4::Scope scope(v4);
-
-    // Create the script context if required
-    QQmlContextDataRef ctxt(qmlContextDataForContext(parentCtxt));
+    QQmlContextDataRef qmlContextData = qmlContextDataForContext(parentQmlContextData);
+    QV4::Scoped<QV4::QmlContext> qmlExecutionContext(scope);
+    if (qmlContextData)
+        qmlExecutionContext =
+            QV4::QmlContext::create(v4->rootContext(), qmlContextData, /* scopeObject: */ nullptr);
 
-    if (!hasEngine())
-        initialize(parentCtxt->engine);
+    QV4::Scoped<QV4::Module> module(scope, m_precompiledScript->instantiate(v4));
+    if (module) {
+        if (qmlContextData) {
+            module->d()->scope->outer.set(v4, qmlExecutionContext->d());
+            qmlExecutionContext->d()->qml()->module.set(v4, module->d());
+        }
 
-    if (!m_program) {
-        if (m_precompiledScript->isSharedLibrary())
-            m_loaded = true;
-        return QV4::Encode::undefined();
+        module->evaluate();
     }
 
-    QV4::Scoped<QV4::QmlContext> qmlContext(scope, QV4::QmlContext::create(v4->rootContext(), ctxt, nullptr));
-
-    m_program->qmlContext.set(scope.engine, qmlContext);
-    m_program->run();
-    m_program->qmlContext.clear();
-    if (scope.engine->hasException) {
-        QQmlError error = scope.engine->catchExceptionAsQmlError();
+    if (v4->hasException) {
+        QQmlError error = v4->catchExceptionAsQmlError();
         if (error.isValid())
-            ep->warning(error);
+            QQmlEnginePrivate::get(v4)->warning(error);
     }
 
-    QV4::ScopedValue retval(scope, qmlContext->d()->qml());
-    if (m_precompiledScript->isSharedLibrary()) {
-        m_value.set(scope.engine, retval);
+    QV4::ScopedValue value(scope);
+    if (qmlContextData)
+        value = qmlExecutionContext->d()->qml();
+    else if (module)
+        value = module->d();
+
+    if (m_precompiledScript->isSharedLibrary() || m_precompiledScript->isESModule()) {
         m_loaded = true;
+        m_value.set(v4, value);
     }
 
-    return retval->asReturnedValue();
+    return value->asReturnedValue();
 }
 
 void QQmlScriptData::clear()
diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h
index 29c91346de..5bb4e9d490 100644
--- a/src/qml/qml/qqmltypeloader_p.h
+++ b/src/qml/qml/qqmltypeloader_p.h
@@ -534,8 +534,6 @@ private:
     QQmlScriptData();
 
 public:
-    ~QQmlScriptData() override;
-
     QUrl url;
     QString urlString;
     QQmlTypeNameCache *typeNameCache;
@@ -556,7 +554,6 @@ private:
 
     bool m_loaded;
     QQmlRefPointer<QV4::CompiledData::CompilationUnit> m_precompiledScript;
-    QV4::Script *m_program;
     QV4::PersistentValue m_value;
 };
 
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 2f110ed5a5..71c4e03812 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -8822,10 +8822,10 @@ void tst_qqmlecmascript::importLexicalVariables_data()
 
     QTest::newRow("script")
         << testFileUrl("importLexicalVariables_script.qml")
-        << QStringLiteral("0?? 1?? 2??");
+        << QStringLiteral("000 100 210");
     QTest::newRow("pragmaLibrary")
         << testFileUrl("importLexicalVariables_pragmaLibrary.qml")
-        << QStringLiteral("0?? 1?? 2??");
+        << QStringLiteral("000 100 210");
     QTest::newRow("module")
         << testFileUrl("importLexicalVariables_module.qml")
         << QStringLiteral("000 000 110");