Use a PatternElement for VariableDeclarations

Required to get proper destructuring working.

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

5 files changed, 27 insertions, 32 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 15faefe66c..0323012686 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -374,17 +374,17 @@ void Codegen::statementList(StatementList *ast)
     requiresReturnValue = _requiresReturnValue;
 }
 
-void Codegen::variableDeclaration(VariableDeclaration *ast)
+void Codegen::variableDeclaration(PatternElement *ast)
 {
     RegisterScope scope(this);
 
-    if (!ast->expression)
+    if (!ast->initializer)
         return;
-    Reference rhs = expression(ast->expression);
+    Reference rhs = expression(ast->initializer);
     if (hasError)
         return;
 
-    Reference lhs = referenceForName(ast->name.toString(), true);
+    Reference lhs = referenceForName(ast->bindingIdentifier, true);
     //### if lhs is a temp, this won't generate a temp-to-temp move. Same for when rhs is a const
     rhs.loadInAccumulator();
     lhs.storeConsumeAccumulator();
@@ -627,12 +627,6 @@ bool Codegen::visit(UiQualifiedPragmaId *)
     return false;
 }
 
-bool Codegen::visit(VariableDeclaration *)
-{
-    Q_UNREACHABLE();
-    return false;
-}
-
 bool Codegen::visit(VariableDeclarationList *)
 {
     Q_UNREACHABLE();
@@ -2350,7 +2344,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     if (!_context->parent || _context->usesArgumentsObject == Context::ArgumentsObjectUnknown)
         _context->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
     if (_context->usesArgumentsObject == Context::ArgumentsObjectUsed)
-        _context->addLocalVar(QStringLiteral("arguments"), Context::VariableDeclaration, AST::VariableDeclaration::FunctionScope);
+        _context->addLocalVar(QStringLiteral("arguments"), Context::VariableDeclaration, AST::VariableScope::Var);
 
     bool allVarsEscape = _context->hasWith || _context->hasTry || _context->hasDirectEval;
     if (_context->compilationMode == QmlBinding // we don't really need this for bindings, but we do for signal handlers, and we don't know if the code is a signal handler or not.
@@ -2787,7 +2781,7 @@ bool Codegen::visit(LocalForEachStatement *ast)
 
     BytecodeGenerator::Label body = bytecodeGenerator->label();
 
-    Reference it = referenceForName(ast->declaration->name.toString(), true).asLValue();
+    Reference it = referenceForName(ast->declaration->bindingIdentifier, true).asLValue();
 
     nextIterObj.loadInAccumulator();
     it.storeConsumeAccumulator();
diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h
index df7096f455..ed43620e3a 100644
--- a/src/qml/compiler/qv4codegen_p.h
+++ b/src/qml/compiler/qv4codegen_p.h
@@ -508,7 +508,7 @@ protected:
 
     void program(AST::Program *ast);
     void statementList(AST::StatementList *ast);
-    void variableDeclaration(AST::VariableDeclaration *ast);
+    void variableDeclaration(AST::PatternElement *ast);
     void variableDeclarationList(AST::VariableDeclarationList *ast);
 
     void initializeAndDestructureBindingElement(AST::PatternElement *e, const Reference &baseRef);
@@ -545,7 +545,6 @@ protected:
     bool visit(AST::UiProgram *ast) override;
     bool visit(AST::UiQualifiedId *ast) override;
     bool visit(AST::UiQualifiedPragmaId *ast) override;
-    bool visit(AST::VariableDeclaration *ast) override;
     bool visit(AST::VariableDeclarationList *ast) override;
 
     bool visit(AST::PatternElement *ast) override;
diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h
index 9ce0d5fa1e..e53fb26e4a 100644
--- a/src/qml/compiler/qv4compilercontext_p.h
+++ b/src/qml/compiler/qv4compilercontext_p.h
@@ -118,11 +118,11 @@ struct Context {
     struct Member {
         MemberType type = UndefinedMember;
         int index = -1;
-        QQmlJS::AST::VariableDeclaration::VariableScope scope = QQmlJS::AST::VariableDeclaration::FunctionScope;
+        QQmlJS::AST::VariableScope scope = QQmlJS::AST::VariableScope::Var;
         mutable bool canEscape = false;
         QQmlJS::AST::FunctionExpression *function = nullptr;
 
-        bool isLexicallyScoped() const { return this->scope != QQmlJS::AST::VariableDeclaration::FunctionScope; }
+        bool isLexicallyScoped() const { return this->scope != QQmlJS::AST::VariableScope::Var; }
     };
     typedef QMap<QString, Member> MemberMap;
 
@@ -259,18 +259,18 @@ struct Context {
         usedVariables.insert(name);
     }
 
-    bool addLocalVar(const QString &name, MemberType type, QQmlJS::AST::VariableDeclaration::VariableScope scope, QQmlJS::AST::FunctionExpression *function = nullptr)
+    bool addLocalVar(const QString &name, MemberType type, QQmlJS::AST::VariableScope scope, QQmlJS::AST::FunctionExpression *function = nullptr)
     {
         if (name.isEmpty())
             return true;
 
         if (type != FunctionDefinition) {
             if (formals->containsName(name))
-                return (scope == QQmlJS::AST::VariableDeclaration::FunctionScope);
+                return (scope == QQmlJS::AST::VariableScope::Var);
         }
         MemberMap::iterator it = members.find(name);
         if (it != members.end()) {
-            if (scope != QQmlJS::AST::VariableDeclaration::FunctionScope || (*it).scope != QQmlJS::AST::VariableDeclaration::FunctionScope)
+            if (scope != QQmlJS::AST::VariableScope::Var || (*it).scope != QQmlJS::AST::VariableScope::Var)
                 return false;
             if ((*it).type <= type) {
                 (*it).type = type;
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index 217c077dc0..3f98d7fd67 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -191,20 +191,22 @@ bool ScanFunctions::visit(ArrayPattern *ast)
     return true;
 }
 
-bool ScanFunctions::visit(VariableDeclaration *ast)
+bool ScanFunctions::visit(PatternElement *ast)
 {
-    if (_context->isStrict && (ast->name == QLatin1String("eval") || ast->name == QLatin1String("arguments")))
+    if (!ast->isVariableDeclaration())
+        return true;
+
+    if (_context->isStrict && (ast->bindingIdentifier == QLatin1String("eval") || ast->bindingIdentifier == QLatin1String("arguments")))
         _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Variable name may not be eval or arguments in strict mode"));
-    checkName(ast->name, ast->identifierToken);
-    if (ast->name == QLatin1String("arguments"))
+    checkName(QStringRef(&ast->bindingIdentifier), ast->identifierToken);
+    if (ast->bindingIdentifier == QLatin1String("arguments"))
         _context->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
-    if (ast->scope == AST::VariableDeclaration::VariableScope::ReadOnlyBlockScope && !ast->expression) {
+    if (ast->scope == VariableScope::Const && !ast->initializer) {
         _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Missing initializer in const declaration"));
         return false;
     }
-    QString name = ast->name.toString();
-    if (!_context->addLocalVar(ast->name.toString(), ast->expression ? Context::VariableDefinition : Context::VariableDeclaration, ast->scope)) {
-        _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Identifier %1 has already been declared").arg(name));
+    if (!_context->addLocalVar(ast->bindingIdentifier, ast->initializer ? Context::VariableDefinition : Context::VariableDeclaration, ast->scope)) {
+        _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Identifier %1 has already been declared").arg(ast->bindingIdentifier));
         return false;
     }
     return true;
@@ -396,11 +398,11 @@ bool ScanFunctions::enterFunction(Node *ast, const QString &name, FormalParamete
         outerContext->hasNestedFunctions = true;
         // The identifier of a function expression cannot be referenced from the enclosing environment.
         if (enterName) {
-            if (!outerContext->addLocalVar(name, Context::FunctionDefinition, VariableDeclaration::FunctionScope, expr)) {
+            if (!outerContext->addLocalVar(name, Context::FunctionDefinition, VariableScope::Var, expr)) {
                 _cg->throwSyntaxError(ast->firstSourceLocation(), QStringLiteral("Identifier %1 has already been declared").arg(name));
                 return false;
             }
-            outerContext->addLocalVar(name, Context::FunctionDefinition, VariableDeclaration::FunctionScope, expr);
+            outerContext->addLocalVar(name, Context::FunctionDefinition, VariableScope::Var, expr);
         }
         if (name == QLatin1String("arguments"))
             outerContext->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
@@ -417,7 +419,7 @@ bool ScanFunctions::enterFunction(Node *ast, const QString &name, FormalParamete
 
 
     if (!name.isEmpty() && !formals->containsName(name))
-        _context->addLocalVar(name, Context::ThisFunctionName, QQmlJS::AST::VariableDeclaration::FunctionScope);
+        _context->addLocalVar(name, Context::ThisFunctionName, VariableScope::Var);
     _context->formals = formals;
 
     if (body && !_context->isStrict)
@@ -444,7 +446,7 @@ bool ScanFunctions::enterFunction(Node *ast, const QString &name, FormalParamete
             }
         }
         if (!_context->arguments.contains(arg))
-            _context->addLocalVar(arg, Context::VariableDefinition, QQmlJS::AST::VariableDeclaration::FunctionScope);
+            _context->addLocalVar(arg, Context::VariableDefinition, VariableScope::Var);
     }
     return true;
 }
diff --git a/src/qml/compiler/qv4compilerscanfunctions_p.h b/src/qml/compiler/qv4compilerscanfunctions_p.h
index b479b65e6c..5ec9310b8d 100644
--- a/src/qml/compiler/qv4compilerscanfunctions_p.h
+++ b/src/qml/compiler/qv4compilerscanfunctions_p.h
@@ -105,7 +105,7 @@ protected:
     bool visit(AST::CallExpression *ast) override;
     bool visit(AST::NewMemberExpression *ast) override;
     bool visit(AST::ArrayPattern *ast) override;
-    bool visit(AST::VariableDeclaration *ast) override;
+    bool visit(AST::PatternElement *ast) override;
     bool visit(AST::IdentifierExpression *ast) override;
     bool visit(AST::ExpressionStatement *ast) override;
     bool visit(AST::FunctionExpression *ast) override;