Cleanup handling of direct evals

There's no need to force lookups by name in an outer function
just because an inner function uses eval(). The lookup by name
is only required on the call context level, where eval() could
add new variables.

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

4 files changed, 24 insertions, 13 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 860a1307f2..8d0d0695f5 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -2405,8 +2405,6 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
         _context->blockIndex = _module->blocks.count() - 1;
     }
 
-    _context->hasDirectEval |= (_context->contextType == ContextType::Eval || _context->contextType == ContextType::Global || _module->debugMode); // Conditional breakpoints are like eval in the function
-
     // When a user writes the following QML signal binding:
     //    onSignal: function() { doSomethingUsefull }
     // we will generate a binding function that just returns the closure. However, that's not useful
diff --git a/src/qml/compiler/qv4compilercontext.cpp b/src/qml/compiler/qv4compilercontext.cpp
index c4f7e51f6c..7acc8256f0 100644
--- a/src/qml/compiler/qv4compilercontext.cpp
+++ b/src/qml/compiler/qv4compilercontext.cpp
@@ -143,8 +143,10 @@ Context::ResolvedName Context::resolveName(const QString &name)
                 return result;
             }
         }
-        if (!c->isStrict && c->hasDirectEval && c->contextType != ContextType::Block)
+        if (c->hasDirectEval) {
+            Q_ASSERT(!c->isStrict && c->contextType != ContextType::Block);
             return result;
+        }
 
         if (c->requiresExecutionContext)
             ++scope;
diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h
index 95fca2c2c8..cce833f2ac 100644
--- a/src/qml/compiler/qv4compilercontext_p.h
+++ b/src/qml/compiler/qv4compilercontext_p.h
@@ -144,6 +144,7 @@ struct Context {
     int nRegisters = 0;
     int registerOffset = -1;
     bool hasDirectEval = false;
+    bool allVarsEscape = false;
     bool hasNestedFunctions = false;
     bool isStrict = false;
     bool isArrowFunction = false;
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index ef69706f5d..88fccc548d 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -552,19 +552,29 @@ void ScanFunctions::calcEscapingVariables()
                 c = c->parent;
             }
         }
-        Context *c = inner->parent;
-        while (c) {
-            c->hasDirectEval |= inner->hasDirectEval;
-            c = c->parent;
+        if (inner->hasDirectEval) {
+            inner->hasDirectEval = false;
+            if (!inner->isStrict) {
+                Context *c = inner;
+                while (c->contextType == ContextType::Block) {
+                    c = c->parent;
+                }
+                Q_ASSERT(c);
+                c->hasDirectEval = true;
+            }
+            Context *c = inner;
+            while (c) {
+                c->allVarsEscape = true;
+                c = c->parent;
+            }
         }
     }
     for (Context *c : qAsConst(m->contextMap)) {
-        bool allVarsEscape = c->hasDirectEval;
-        if (allVarsEscape && c->contextType == ContextType::Block && c->members.isEmpty())
-            allVarsEscape = false;
+        if (c->allVarsEscape && c->contextType == ContextType::Block && c->members.isEmpty())
+            c->allVarsEscape = false;
         if (!c->parent || m->debugMode)
-            allVarsEscape = true;
-        if (allVarsEscape) {
+            c->allVarsEscape = true;
+        if (c->allVarsEscape) {
             if (c->parent) {
                 c->requiresExecutionContext = true;
                 c->argumentsCanEscape = true;
@@ -588,7 +598,7 @@ void ScanFunctions::calcEscapingVariables()
             // we don't really need this for bindings, but we do for signal handlers, and in this case,
             // we don't know if the code is a signal handler or not.
             c->requiresExecutionContext = true;
-        if (allVarsEscape) {
+        if (c->allVarsEscape) {
             for (auto &m : c->members)
                 m.canEscape = true;
         }