Cleanup handling of with() statements

Add a CompilerContext for with, whose only purpose it
is to trigger variable lookup by name. This avoids looking
up variables declared inside the with() {} block by name and
we do not lookup variables outside the with block by name
neither anymore.

Change-Id: I52e9fb2daa9601f9e5102714c002dc506ad5ed23
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

10 files changed, 61 insertions, 62 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 794428b14d..61fe5587a5 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -2977,17 +2977,18 @@ bool Codegen::visit(WithStatement *ast)
 
     RegisterScope scope(this);
 
-    Q_ASSERT(_context->hasWith);
-
     Reference src = expression(ast->expression);
     if (hasError)
         return false;
     src = src.storeOnStack(); // trigger load before we setup the exception handler, so exceptions here go to the right place
     src.loadInAccumulator();
 
-    ControlFlowWith flow(this);
-
-    statement(ast->statement);
+    enterContext(ast);
+    {
+        ControlFlowWith flow(this);
+        statement(ast->statement);
+    }
+    leaveContext();
 
     return false;
 }
diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp
index 74f1ab81aa..0a4ba72cf0 100644
--- a/src/qml/compiler/qv4compileddata.cpp
+++ b/src/qml/compiler/qv4compileddata.cpp
@@ -540,9 +540,6 @@ Unit *CompilationUnit::createUnitData(QmlIR::Document *irDocument)
             function->nFormals = formals.size();
             function->length = function->nFormals;
 
-            // Hack to ensure an activation is created.
-            function->flags |= QV4::CompiledData::Function::HasWith | QV4::CompiledData::Function::HasDirectEval;
-
             signalParameterNameTableOffset += function->nFormals * sizeof(quint32);
         }
     }
diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h
index 668532fc41..a0b7eb0ed8 100644
--- a/src/qml/compiler/qv4compileddata_p.h
+++ b/src/qml/compiler/qv4compileddata_p.h
@@ -227,11 +227,8 @@ struct Function
 {
     enum Flags : unsigned int {
         IsStrict            = 0x1,
-        HasDirectEval       = 0x2,
-        UsesArgumentsObject = 0x4,
-        IsArrowFunction     = 0x8,
-        IsGenerator         = 0x20,
-        HasWith      = 0x10
+        IsArrowFunction     = 0x2,
+        IsGenerator         = 0x4
     };
 
     // Absolute offset into file where the code for this function is located.
diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp
index cca4c01be6..bd9dd96d93 100644
--- a/src/qml/compiler/qv4compiler.cpp
+++ b/src/qml/compiler/qv4compiler.cpp
@@ -311,18 +311,12 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::Compiler::Conte
 
     function->nameIndex = getStringId(irFunction->name);
     function->flags = 0;
-    if (irFunction->hasDirectEval)
-        function->flags |= CompiledData::Function::HasDirectEval;
-    if (irFunction->usesArgumentsObject)
-        function->flags |= CompiledData::Function::UsesArgumentsObject;
     if (irFunction->isStrict)
         function->flags |= CompiledData::Function::IsStrict;
     if (irFunction->isArrowFunction)
         function->flags |= CompiledData::Function::IsArrowFunction;
     if (irFunction->isGenerator)
         function->flags |= CompiledData::Function::IsGenerator;
-    if (irFunction->hasWith)
-        function->flags |= CompiledData::Function::HasWith;
     function->nestedFunctionIndex =
             irFunction->returnsClosure ? quint32(module->functions.indexOf(irFunction->nestedContexts.first()))
                                        : std::numeric_limits<uint32_t>::max();
diff --git a/src/qml/compiler/qv4compilercontext.cpp b/src/qml/compiler/qv4compilercontext.cpp
index f340d62e52..a0467aa427 100644
--- a/src/qml/compiler/qv4compilercontext.cpp
+++ b/src/qml/compiler/qv4compilercontext.cpp
@@ -112,7 +112,7 @@ Context::ResolvedName Context::resolveName(const QString &name)
     ResolvedName result;
 
     while (c->parent) {
-        if (c->forceLookupByName)
+        if (c->isWithBlock)
             return result;
 
         Context::Member m = c->findMember(name);
@@ -148,7 +148,7 @@ Context::ResolvedName Context::resolveName(const QString &name)
     }
 
     // ### can we relax the restrictions here?
-    if (c->forceLookupByName || contextType == ContextType::Eval || c->contextType == ContextType::Binding)
+    if (contextType == ContextType::Eval || c->contextType == ContextType::Binding)
         return result;
 
     result.type = ResolvedName::Global;
diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h
index 8c7da397a9..20bee6583a 100644
--- a/src/qml/compiler/qv4compilercontext_p.h
+++ b/src/qml/compiler/qv4compilercontext_p.h
@@ -150,11 +150,10 @@ struct Context {
     bool isGenerator = false;
     bool usesThis = false;
     bool hasTry = false;
-    bool hasWith = false;
     bool returnsClosure = false;
     mutable bool argumentsCanEscape = false;
     bool requiresExecutionContext = false;
-    bool forceLookupByName = false;
+    bool isWithBlock = false;
     bool isCatchBlock = false;
     QString catchedVariable;
 
diff --git a/src/qml/compiler/qv4compilercontrolflow_p.h b/src/qml/compiler/qv4compilercontrolflow_p.h
index 6f524bb366..b89226c411 100644
--- a/src/qml/compiler/qv4compilercontrolflow_p.h
+++ b/src/qml/compiler/qv4compilercontrolflow_p.h
@@ -92,18 +92,15 @@ struct ControlFlow {
     Codegen *cg;
     ControlFlow *parent;
     Type type;
-    bool contextUsedLookupByName;
 
     ControlFlow(Codegen *cg, Type type)
         : cg(cg), parent(cg->controlFlow), type(type)
     {
-        contextUsedLookupByName = cg->_context->forceLookupByName;
         cg->controlFlow = this;
     }
 
     virtual ~ControlFlow() {
         cg->controlFlow = parent;
-        cg->_context->forceLookupByName = contextUsedLookupByName;
     }
 
     void emitReturnStatement() const {
@@ -308,8 +305,6 @@ struct ControlFlowWith : public ControlFlowUnwind
     ControlFlowWith(Codegen *cg)
         : ControlFlowUnwind(cg, With)
     {
-        cg->currentContext()->forceLookupByName = true;
-
         setupExceptionHandler();
         savedContextRegister = Moth::StackSlot::createRegister(generator()->newRegister());
 
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index 120c606f91..f5bfd14c93 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -150,7 +150,7 @@ void ScanFunctions::endVisit(Program *)
 
 bool ScanFunctions::visit(CallExpression *ast)
 {
-    if (! _context->hasDirectEval) {
+    if (!_context->hasDirectEval) {
         if (IdentifierExpression *id = cast<IdentifierExpression *>(ast->base)) {
             if (id->name == QLatin1String("eval")) {
                 if (_context->usesArgumentsObject == Context::ArgumentsObjectUnknown)
@@ -317,17 +317,6 @@ void ScanFunctions::endVisit(FunctionDeclaration *)
     leaveEnvironment();
 }
 
-bool ScanFunctions::visit(WithStatement *ast)
-{
-    if (_context->isStrict) {
-        _cg->throwSyntaxError(ast->withToken, QStringLiteral("'with' statement is not allowed in strict mode"));
-        return false;
-    }
-
-    _context->hasWith = true;
-    return true;
-}
-
 bool ScanFunctions::visit(DoWhileStatement *ast) {
     {
         TemporaryBoolAssignment allowFuncDecls(_allowFuncDecls, !_context->isStrict);
@@ -416,6 +405,29 @@ void ScanFunctions::endVisit(Catch *)
     leaveEnvironment();
 }
 
+bool ScanFunctions::visit(WithStatement *ast)
+{
+    Node::accept(ast->expression, this);
+
+    TemporaryBoolAssignment allowFuncDecls(_allowFuncDecls, _context->isStrict ? false : _allowFuncDecls);
+    enterEnvironment(ast, ContextType::Block);
+    _context->name = QLatin1String("WithBlock");
+    _context->isWithBlock = true;
+
+    if (_context->isStrict) {
+        _cg->throwSyntaxError(ast->withToken, QStringLiteral("'with' statement is not allowed in strict mode"));
+        return false;
+    }
+    Node::accept(ast->statement, this);
+
+    return false;
+}
+
+void ScanFunctions::endVisit(WithStatement *)
+{
+    leaveEnvironment();
+}
+
 bool ScanFunctions::enterFunction(Node *ast, const QString &name, FormalParameterList *formals, StatementList *body, bool enterName)
 {
     Context *outerContext = _context;
@@ -487,6 +499,28 @@ void ScanFunctions::calcEscapingVariables()
     Module *m = _cg->_module;
 
     for (Context *inner : qAsConst(m->contextMap)) {
+        if (inner->contextType == ContextType::Block && inner->usesArgumentsObject == Context::ArgumentsObjectUsed) {
+            Context *c = inner->parent;
+            while (c->contextType == ContextType::Block)
+                c = c->parent;
+            c->usesArgumentsObject = Context::ArgumentsObjectUsed;
+            inner->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
+        }
+    }
+    for (Context *inner : qAsConst(m->contextMap)) {
+        if (!inner->parent || inner->usesArgumentsObject == Context::ArgumentsObjectUnknown)
+            inner->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
+        if (inner->usesArgumentsObject == Context::ArgumentsObjectUsed) {
+            QString arguments = QStringLiteral("arguments");
+            inner->addLocalVar(arguments, Context::VariableDeclaration, AST::VariableScope::Var);
+            if (!inner->isStrict) {
+                inner->argumentsCanEscape = true;
+                inner->requiresExecutionContext = true;
+            }
+        }
+    }
+
+    for (Context *inner : qAsConst(m->contextMap)) {
         for (const QString &var : qAsConst(inner->usedVariables)) {
             Context *c = inner;
             while (c) {
@@ -511,19 +545,11 @@ void ScanFunctions::calcEscapingVariables()
         Context *c = inner->parent;
         while (c) {
             c->hasDirectEval |= inner->hasDirectEval;
-            c->hasWith |= inner->hasWith;
             c = c->parent;
         }
-        if (inner->contextType == ContextType::Block && inner->usesArgumentsObject == Context::ArgumentsObjectUsed) {
-            Context *f = inner->parent;
-            while (f->contextType == ContextType::Block)
-                f = f->parent;
-            f->usesArgumentsObject = Context::ArgumentsObjectUsed;
-            inner->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
-        }
     }
     for (Context *c : qAsConst(m->contextMap)) {
-        bool allVarsEscape = c->hasWith || c->hasDirectEval;
+        bool allVarsEscape = c->hasDirectEval;
         if (allVarsEscape && c->contextType == ContextType::Block && c->members.isEmpty())
             allVarsEscape = false;
         if (m->debugMode)
@@ -543,16 +569,6 @@ void ScanFunctions::calcEscapingVariables()
             auto m = c->members.find(c->catchedVariable);
             m->canEscape = true;
         }
-        if (!c->parent || c->usesArgumentsObject == Context::ArgumentsObjectUnknown)
-            c->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
-        if (c->usesArgumentsObject == Context::ArgumentsObjectUsed) {
-            QString arguments = QStringLiteral("arguments");
-            c->addLocalVar(arguments, Context::VariableDeclaration, AST::VariableScope::Var);
-            if (!c->isStrict) {
-                c->argumentsCanEscape = true;
-                c->requiresExecutionContext = true;
-            }
-        }
         if (allVarsEscape) {
             for (auto &m : c->members)
                 m.canEscape = true;
diff --git a/src/qml/compiler/qv4compilerscanfunctions_p.h b/src/qml/compiler/qv4compilerscanfunctions_p.h
index 013cb0d4e6..31e260b7ad 100644
--- a/src/qml/compiler/qv4compilerscanfunctions_p.h
+++ b/src/qml/compiler/qv4compilerscanfunctions_p.h
@@ -123,8 +123,6 @@ protected:
     bool visit(AST::FunctionDeclaration *ast) override;
     void endVisit(AST::FunctionDeclaration *) override;
 
-    bool visit(AST::WithStatement *ast) override;
-
     bool visit(AST::DoWhileStatement *ast) override;
     bool visit(AST::ForStatement *ast) override;
     bool visit(AST::LocalForStatement *ast) override;
@@ -138,6 +136,9 @@ protected:
     bool visit(AST::Catch *ast) override;
     void endVisit(AST::Catch *ast) override;
 
+    bool visit(AST::WithStatement *ast) override;
+    void endVisit(AST::WithStatement *ast) override;
+
 protected:
     bool enterFunction(AST::Node *ast, const QString &name, AST::FormalParameterList *formals, AST::StatementList *body, bool enterName);
 
diff --git a/src/qml/jsruntime/qv4function_p.h b/src/qml/jsruntime/qv4function_p.h
index d3206cc230..dc18245819 100644
--- a/src/qml/jsruntime/qv4function_p.h
+++ b/src/qml/jsruntime/qv4function_p.h
@@ -98,7 +98,6 @@ struct Q_QML_EXPORT Function {
     inline QString sourceFile() const { return compilationUnit->fileName(); }
     inline QUrl finalUrl() const { return compilationUnit->finalUrl(); }
 
-    inline bool usesArgumentsObject() const { return compiledFunction->flags & CompiledData::Function::UsesArgumentsObject; }
     inline bool isStrict() const { return compiledFunction->flags & CompiledData::Function::IsStrict; }
     inline bool isArrowFunction() const { return compiledFunction->flags & CompiledData::Function::IsArrowFunction; }