Limit the amount of escaping variables

Calculate more exactly which variables can
be referenced from an inner context, and convert
all the non escaping ones to temporaries on the
stack.

Change-Id: I0e33e85b0f6f426ef2812b8ecccee1870492b7b5
Reviewed-by: Erik Verbruggen <erik.verbruggen@qt.io>

5 files changed, 60 insertions, 27 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 5d5daebf6c..7c8a6c0432 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -154,11 +154,13 @@ void Codegen::enterContext(Node *node)
     Q_ASSERT(_context);
 }
 
-void Codegen::leaveContext()
+int Codegen::leaveContext()
 {
     Q_ASSERT(_context);
     Q_ASSERT(!_context->controlFlow);
+    int functionIndex = _context->functionIndex;
     _context = _context->parent;
+    return functionIndex;
 }
 
 Codegen::Reference Codegen::unop(UnaryOperation op, const Reference &expr)
@@ -1077,6 +1079,10 @@ bool Codegen::visit(DeleteExpression *ast)
         return false;
 
     switch (expr.type) {
+    case Reference::Temp:
+        if (!expr.tempIsLocal)
+            break;
+        // fall through
     case Reference::Local:
     case Reference::Argument:
         // Trying to delete a function argument might throw.
@@ -1117,10 +1123,11 @@ bool Codegen::visit(DeleteExpression *ast)
         return false;
     }
     default:
-        // [[11.4.1]] Return true if it's not a reference
-        _expr.result = Reference::fromConst(this, QV4::Encode(true));
-        return false;
+        break;
     }
+    // [[11.4.1]] Return true if it's not a reference
+    _expr.result = Reference::fromConst(this, QV4::Encode(true));
+    return false;
 }
 
 bool Codegen::visit(FalseLiteral *)
@@ -1165,10 +1172,9 @@ Codegen::Reference Codegen::referenceForName(const QString &name, bool isLhs)
         if (c->forceLookupByName() || (c->isNamedFunctionExpression && c->name == name))
             goto loadByName;
 
-        int index = c->findMember(name);
-        Q_ASSERT (index < c->members.size());
-        if (index != -1) {
-            Reference r = Reference::fromLocal(this, index, scope);
+        Context::Member m = c->findMember(name);
+        if (m.type != Context::UndefinedMember) {
+            Reference r = m.canEscape ? Reference::fromLocal(this, m.index, scope) : Reference::fromTemp(this, m.index, true /*isLocal*/);
             if (name == QLatin1String("arguments") || name == QLatin1String("eval")) {
                 r.isArgOrEval = true;
                 if (isLhs && c->isStrict)
@@ -1655,9 +1661,13 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
 
     enterContext(ast);
 
+    if (_context->functionIndex >= 0)
+        // already defined
+        return leaveContext();
+
     _context->name = name;
     _module->functions.append(_context);
-    int functionIndex = _module->functions.count() - 1;
+    _context->functionIndex = _module->functions.count() - 1;
 
     _context->hasDirectEval |= _context->compilationMode == EvalCode || _module->debugMode; // Conditional breakpoints are like eval in the function
     // ### still needed?
@@ -1675,14 +1685,20 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     if (_context->usesArgumentsObject == Context::ArgumentsObjectUsed)
         _context->addLocalVar(QStringLiteral("arguments"), Context::VariableDeclaration, AST::VariableDeclaration::FunctionScope);
 
+    bool allVarsEscape = _context->hasWith || _context->hasTry || _context->hasDirectEval;
+
     // variables in global code are properties of the global context object, not locals as with other functions.
     if (_context->compilationMode == FunctionCode || _context->compilationMode == QmlBinding) {
-        unsigned t = 0;
         for (Context::MemberMap::iterator it = _context->members.begin(), end = _context->members.end(); it != end; ++it) {
             const QString &local = it.key();
-            _context->locals.append(local);
-            (*it).index = t;
-            ++t;
+            if (allVarsEscape)
+                it->canEscape = true;
+            if (it->canEscape) {
+                it->index = _context->locals.size();
+                _context->locals.append(local);
+            } else {
+                it->index = bytecodeGenerator->newTemp();
+            }
         }
     } else {
         for (Context::MemberMap::const_iterator it = _context->members.constBegin(), cend = _context->members.constEnd(); it != cend; ++it) {
@@ -1710,7 +1726,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
                 name.store(func);
             } else {
                 Q_ASSERT(member.index >= 0);
-                Reference local = Reference::fromLocal(this, member.index, 0);
+                Reference local = member.canEscape ? Reference::fromLocal(this, member.index, 0) : Reference::fromTemp(this, member.index, true);
                 local.store(func);
             }
         }
@@ -1742,7 +1758,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     _context->code = bytecodeGenerator->finalize();
     static const bool showCode = qEnvironmentVariableIsSet("QV4_SHOW_BYTECODE");
     if (showCode) {
-        qDebug() << "=== Bytecode for" << _context->name;
+        qDebug() << "=== Bytecode for" << _context->name << "strict mode" << _context->isStrict;
         QV4::Moth::dumpBytecode(_context->code);
         qDebug();
     }
@@ -1750,11 +1766,9 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     qSwap(_exitBlock, exitBlock);
     qSwap(_returnAddress, returnAddress);
 
-    leaveContext();
-
     bytecodeGenerator = savedBytecodeGenerator;
 
-    return functionIndex;
+    return leaveContext();
 }
 
 bool Codegen::visit(FunctionSourceElement *ast)
@@ -2261,9 +2275,9 @@ bool Codegen::visit(TryStatement *ast)
     if (hasError)
         return true;
 
-    TempScope scope(this);
+    Q_ASSERT(_context->hasTry);
 
-    _context->hasTry = true;
+    TempScope scope(this);
 
     if (ast->finallyExpression && ast->finallyExpression->statement) {
         handleTryFinally(ast);
@@ -2375,7 +2389,7 @@ bool Codegen::throwSyntaxErrorOnEvalOrArgumentsInStrictMode(const Reference &r,
         if (str == QLatin1String("eval") || str == QLatin1String("arguments")) {
             isArgOrEval = true;
         }
-    } else if (r.type == Reference::Local) {
+    } else if (r.type == Reference::Local || r.type == Reference::Temp) {
         isArgOrEval = r.isArgOrEval;
     }
     if (isArgOrEval)
@@ -2510,6 +2524,7 @@ Codegen::Reference &Codegen::Reference::operator =(const Reference &other)
     isArgOrEval = other.isArgOrEval;
     codegen = other.codegen;
     isReadonly = other.isReadonly;
+    tempIsLocal = other.tempIsLocal;
     global = other.global;
     return *this;
 }
diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h
index 4170001954..4cf543088e 100644
--- a/src/qml/compiler/qv4codegen_p.h
+++ b/src/qml/compiler/qv4codegen_p.h
@@ -138,11 +138,12 @@ public:
         bool isTempLocalArg() const { return isValid() && type < Argument; }
         bool isConst() const { return type == Const; }
 
-        static Reference fromTemp(Codegen *cg, int tempIndex = -1) {
+        static Reference fromTemp(Codegen *cg, int tempIndex = -1, bool isLocal = false) {
             Reference r(cg, Temp);
             if (tempIndex == -1)
                 tempIndex = cg->bytecodeGenerator->newTemp();
             r.base = QV4::Moth::Param::createTemp(tempIndex);
+            r.tempIsLocal = isLocal;
             return r;
         }
         static Reference fromLocal(Codegen *cg, uint index, uint scope) {
@@ -239,6 +240,7 @@ public:
         mutable bool needsWriteBack = false;
         mutable bool isArgOrEval = false;
         bool isReadonly = false;
+        bool tempIsLocal = false;
         bool global = false;
         Codegen *codegen;
 
@@ -319,7 +321,7 @@ protected:
 
 
     void enterContext(AST::Node *node);
-    void leaveContext();
+    int leaveContext();
 
     void leaveLoop();
 
diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h
index 87202089c5..f3481f0251 100644
--- a/src/qml/compiler/qv4compilercontext_p.h
+++ b/src/qml/compiler/qv4compilercontext_p.h
@@ -104,6 +104,8 @@ struct Context {
     int line = 0;
     int column = 0;
 
+    int functionIndex = -1;
+
     enum MemberType {
         UndefinedMember,
         VariableDefinition,
@@ -227,13 +229,13 @@ struct Context {
         return -1;
     }
 
-    int findMember(const QString &name) const
+    Member findMember(const QString &name) const
     {
         MemberMap::const_iterator it = members.find(name);
         if (it == members.end())
-            return -1;
-        Q_ASSERT((*it).index != -1 || !parent);
-        return (*it).index;
+            return Member();
+        Q_ASSERT(it->index != -1 || !parent);
+        return (*it);
     }
 
     bool memberInfo(const QString &name, const Member **m) const
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index 2a217fbb99..b59beb0315 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -305,6 +305,13 @@ void ScanFunctions::endVisit(FunctionDeclaration *)
     leaveEnvironment();
 }
 
+bool ScanFunctions::visit(TryStatement *)
+{
+    // ### should limit to catch(), as try{} finally{} should be ok without
+    _context->hasTry = true;
+    return true;
+}
+
 bool ScanFunctions::visit(WithStatement *ast)
 {
     if (_context->isStrict) {
@@ -312,6 +319,7 @@ bool ScanFunctions::visit(WithStatement *ast)
         return false;
     }
 
+    _context->hasWith = true;
     return true;
 }
 
@@ -436,6 +444,11 @@ void ScanFunctions::calcEscapingVariables()
                 c = c->parent;
             }
         }
+        Context *c = inner->parent;
+        while (c) {
+            c->hasDirectEval |= inner->hasDirectEval;
+            c = c->parent;
+        }
     }
 
     static const bool showEscapingVars = qEnvironmentVariableIsSet("QV4_SHOW_ESCAPING_VARS");
diff --git a/src/qml/compiler/qv4compilerscanfunctions_p.h b/src/qml/compiler/qv4compilerscanfunctions_p.h
index 5d6a0bc9ee..d3588ecfed 100644
--- a/src/qml/compiler/qv4compilerscanfunctions_p.h
+++ b/src/qml/compiler/qv4compilerscanfunctions_p.h
@@ -125,6 +125,7 @@ protected:
     bool visit(AST::FunctionDeclaration *ast) override;
     void endVisit(AST::FunctionDeclaration *) override;
 
+    bool visit(AST::TryStatement *ast) override;
     bool visit(AST::WithStatement *ast) override;
 
     bool visit(AST::DoWhileStatement *ast) override;