Optimize access to lexically scoped variables

If we access a lexically scoped variable after the initializer, then we
know it's either initialized or at least undefined, so we don't need to
do the TDZ check anymore.

The ES tests ensure that we don't optimize too much and the newly
revived tst_v4misc test ensures that we do not generate the TDZ check
instruction for certain scenarios.

Change-Id: I6706d1feb22217f323124ee698ebadb70324693b
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

5 files changed, 41 insertions, 13 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 5d16494c1b..f39f941cc6 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -523,7 +523,7 @@ void Codegen::variableDeclarationList(VariableDeclarationList *ast)
 Codegen::Reference Codegen::targetForPatternElement(AST::PatternElement *p)
 {
     if (!p->bindingIdentifier.isNull())
-        return referenceForName(p->bindingIdentifier.toString(), true);
+        return referenceForName(p->bindingIdentifier.toString(), true, p->firstSourceLocation());
     if (!p->bindingTarget || p->destructuringPattern())
         return Codegen::Reference::fromStackSlot(this);
     Reference lhs = expression(p->bindingTarget);
@@ -2242,9 +2242,9 @@ bool Codegen::visit(FunctionExpression *ast)
     return false;
 }
 
-Codegen::Reference Codegen::referenceForName(const QString &name, bool isLhs)
+Codegen::Reference Codegen::referenceForName(const QString &name, bool isLhs, const SourceLocation &accessLocation)
 {
-    Context::ResolvedName resolved = _context->resolveName(name);
+    Context::ResolvedName resolved = _context->resolveName(name, accessLocation);
 
     if (resolved.type == Context::ResolvedName::Local || resolved.type == Context::ResolvedName::Stack
         || resolved.type == Context::ResolvedName::Import) {
@@ -2302,7 +2302,7 @@ bool Codegen::visit(IdentifierExpression *ast)
     if (hasError)
         return false;
 
-    _expr.setResult(referenceForName(ast->name.toString(), false));
+    _expr.setResult(referenceForName(ast->name.toString(), false, ast->firstSourceLocation()));
     return false;
 }
 
@@ -3111,7 +3111,7 @@ bool Codegen::visit(ForEachStatement *ast)
     Reference lhsValue = Reference::fromStackSlot(this);
 
     // There should be a temporal block, so that variables declared in lhs shadow outside vars.
-    // This block should define a temporal dead zone for those variables, which is not yet implemented.
+    // This block should define a temporal dead zone for those variables.
     {
         RegisterScope innerScope(this);
         ControlFlowBlock controlFlow(this, ast);
diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h
index b7992bd8f2..d780df394b 100644
--- a/src/qml/compiler/qv4codegen_p.h
+++ b/src/qml/compiler/qv4codegen_p.h
@@ -685,7 +685,7 @@ public:
     void handleTryFinally(AST::TryStatement *ast);
 
 
-    Reference referenceForName(const QString &name, bool lhs);
+    Reference referenceForName(const QString &name, bool lhs, const QQmlJS::AST::SourceLocation &accessLocation = QQmlJS::AST::SourceLocation());
 
     QQmlRefPointer<QV4::CompiledData::CompilationUnit> generateCompilationUnit(bool generateUnitData = true);
     static QQmlRefPointer<QV4::CompiledData::CompilationUnit> createUnitForLoading();
diff --git a/src/qml/compiler/qv4compilercontext.cpp b/src/qml/compiler/qv4compilercontext.cpp
index ac0c2e0ecc..ba9b270601 100644
--- a/src/qml/compiler/qv4compilercontext.cpp
+++ b/src/qml/compiler/qv4compilercontext.cpp
@@ -71,7 +71,22 @@ Context *Module::newContext(Node *node, Context *parent, ContextType contextType
     return c;
 }
 
-bool Context::addLocalVar(const QString &name, Context::MemberType type, VariableScope scope, FunctionExpression *function)
+bool Context::Member::requiresTDZCheck(const SourceLocation &accessLocation, bool accessAcrossContextBoundaries) const
+{
+    if (!isLexicallyScoped())
+        return false;
+
+    if (accessAcrossContextBoundaries)
+        return true;
+
+    if (!accessLocation.isValid() || !endOfInitializerLocation.isValid())
+        return true;
+
+    return accessLocation.begin() < endOfInitializerLocation.end();
+}
+
+bool Context::addLocalVar(const QString &name, Context::MemberType type, VariableScope scope, FunctionExpression *function,
+                          const QQmlJS::AST::SourceLocation &endOfInitializer)
 {
     // ### can this happen?
     if (name.isEmpty())
@@ -96,17 +111,18 @@ bool Context::addLocalVar(const QString &name, Context::MemberType type, Variabl
 
     // hoist var declarations to the function level
     if (contextType == ContextType::Block && (scope == VariableScope::Var && type != MemberType::FunctionDefinition))
-        return parent->addLocalVar(name, type, scope, function);
+        return parent->addLocalVar(name, type, scope, function, endOfInitializer);
 
     Member m;
     m.type = type;
     m.function = function;
     m.scope = scope;
+    m.endOfInitializerLocation = endOfInitializer;
     members.insert(name, m);
     return true;
 }
 
-Context::ResolvedName Context::resolveName(const QString &name)
+Context::ResolvedName Context::resolveName(const QString &name, const QQmlJS::AST::SourceLocation &accessLocation)
 {
     int scope = 0;
     Context *c = this;
@@ -126,7 +142,7 @@ Context::ResolvedName Context::resolveName(const QString &name)
             result.scope = scope;
             result.index = m.index;
             result.isConst = (m.scope == VariableScope::Const);
-            result.requiresTDZCheck = m.isLexicallyScoped();
+            result.requiresTDZCheck = m.requiresTDZCheck(accessLocation, c != this);
             if (c->isStrict && (name == QLatin1String("arguments") || name == QLatin1String("eval")))
                 result.isArgOrEval = true;
             return result;
diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h
index e7847e7072..3df0aa6b3a 100644
--- a/src/qml/compiler/qv4compilercontext_p.h
+++ b/src/qml/compiler/qv4compilercontext_p.h
@@ -167,8 +167,10 @@ struct Context {
         QQmlJS::AST::VariableScope scope = QQmlJS::AST::VariableScope::Var;
         mutable bool canEscape = false;
         QQmlJS::AST::FunctionExpression *function = nullptr;
+        QQmlJS::AST::SourceLocation endOfInitializerLocation;
 
         bool isLexicallyScoped() const { return this->scope != QQmlJS::AST::VariableScope::Var; }
+        bool requiresTDZCheck(const QQmlJS::AST::SourceLocation &accessLocation, bool accessAcrossContextBoundaries) const;
     };
     typedef QMap<QString, Member> MemberMap;
 
@@ -206,6 +208,7 @@ struct Context {
     bool isWithBlock = false;
     bool isCatchBlock = false;
     QString caughtVariable;
+    QQmlJS::AST::SourceLocation lastBlockInitializerLocation;
 
     enum UsesArgumentsObject {
         ArgumentsObjectUnknown,
@@ -313,7 +316,8 @@ struct Context {
         usedVariables.insert(name);
     }
 
-    bool addLocalVar(const QString &name, MemberType contextType, QQmlJS::AST::VariableScope scope, QQmlJS::AST::FunctionExpression *function = nullptr);
+    bool addLocalVar(const QString &name, MemberType contextType, QQmlJS::AST::VariableScope scope, QQmlJS::AST::FunctionExpression *function = nullptr,
+                     const QQmlJS::AST::SourceLocation &endOfInitializer = QQmlJS::AST::SourceLocation());
 
     struct ResolvedName {
         enum Type {
@@ -329,9 +333,10 @@ struct Context {
         bool requiresTDZCheck = false;
         int scope = -1;
         int index = -1;
+        QQmlJS::AST::SourceLocation endOfDeclarationLocation;
         bool isValid() const { return type != Unresolved; }
     };
-    ResolvedName resolveName(const QString &name);
+    ResolvedName resolveName(const QString &name, const QQmlJS::AST::SourceLocation &accessLocation);
     void emitBlockHeader(Compiler::Codegen *codegen);
     void emitBlockFooter(Compiler::Codegen *codegen);
 
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index 06335b3aa0..5d3a7a6d8c 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -312,6 +312,10 @@ bool ScanFunctions::visit(PatternElement *ast)
     QStringList names;
     ast->boundNames(&names);
 
+    QQmlJS::AST::SourceLocation lastInitializerLocation = ast->lastSourceLocation();
+    if (_context->lastBlockInitializerLocation.isValid())
+        lastInitializerLocation = _context->lastBlockInitializerLocation;
+
     for (const QString &name : qAsConst(names)) {
         if (_context->isStrict && (name == QLatin1String("eval") || name == QLatin1String("arguments")))
             _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Variable name may not be eval or arguments in strict mode"));
@@ -322,7 +326,8 @@ bool ScanFunctions::visit(PatternElement *ast)
             _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Missing initializer in const declaration"));
             return false;
         }
-        if (!_context->addLocalVar(name, ast->initializer ? Context::VariableDefinition : Context::VariableDeclaration, ast->scope)) {
+        if (!_context->addLocalVar(name, ast->initializer ? Context::VariableDefinition : Context::VariableDeclaration, ast->scope,
+                                   /*function*/nullptr, lastInitializerLocation)) {
             _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Identifier %1 has already been declared").arg(name));
             return false;
         }
@@ -485,6 +490,8 @@ void ScanFunctions::endVisit(ForStatement *)
 
 bool ScanFunctions::visit(ForEachStatement *ast) {
     enterEnvironment(ast, ContextType::Block, QStringLiteral("%Foreach"));
+    if (ast->expression)
+        _context->lastBlockInitializerLocation = ast->expression->lastSourceLocation();
     Node::accept(ast->lhs, this);
     Node::accept(ast->expression, this);