Properly handle redeclarations of variables

This is only allowed for var type variables. Also fixes an
assertion we'd run into with code such as

let x; var x;

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

2 files changed, 38 insertions, 33 deletions

diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h
index 8fabf41c40..455a76c729 100644
--- a/src/qml/compiler/qv4compilercontext_p.h
+++ b/src/qml/compiler/qv4compilercontext_p.h
@@ -257,29 +257,32 @@ struct Context {
         usedVariables.insert(name);
     }
 
-    void 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::VariableDeclaration::VariableScope scope, QQmlJS::AST::FunctionExpression *function = nullptr)
     {
-        if (! name.isEmpty()) {
-            if (type != FunctionDefinition) {
-                for (QQmlJS::AST::FormalParameterList *it = formals; it; it = it->next)
-                    if (it->name == name)
-                        return;
-            }
-            MemberMap::iterator it = members.find(name);
-            if (it == members.end()) {
-                Member m;
-                m.type = type;
-                m.function = function;
-                m.scope = scope;
-                members.insert(name, m);
-            } else {
-                Q_ASSERT(scope == (*it).scope);
-                if ((*it).type <= type) {
-                    (*it).type = type;
-                    (*it).function = function;
-                }
+        if (name.isEmpty())
+            return true;
+
+        if (type != FunctionDefinition) {
+            for (QQmlJS::AST::FormalParameterList *it = formals; it; it = it->next)
+                if (it->name == name)
+                    return (scope == QQmlJS::AST::VariableDeclaration::FunctionScope);
+        }
+        MemberMap::iterator it = members.find(name);
+        if (it != members.end()) {
+            if (scope != QQmlJS::AST::VariableDeclaration::FunctionScope || (*it).scope != QQmlJS::AST::VariableDeclaration::FunctionScope)
+                return false;
+            if ((*it).type <= type) {
+                (*it).type = type;
+                (*it).function = function;
             }
+            return true;
         }
+        Member m;
+        m.type = type;
+        m.function = function;
+        m.scope = scope;
+        members.insert(name, m);
+        return true;
     }
 };
 
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index 92df98f201..84ee452332 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -215,14 +215,10 @@ bool ScanFunctions::visit(VariableDeclaration *ast)
         return false;
     }
     QString name = ast->name.toString();
-    const Context::Member *m = nullptr;
-    if (_context->memberInfo(name, &m)) {
-        if (m->isLexicallyScoped() || ast->isLexicallyScoped()) {
-            _cg->throwSyntaxError(ast->identifierToken, QStringLiteral("Identifier %1 has already been declared").arg(name));
-            return false;
-        }
+    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));
+        return false;
     }
-    _context->addLocalVar(ast->name.toString(), ast->expression ? Context::VariableDefinition : Context::VariableDeclaration, ast->scope);
     return true;
 }
 
@@ -397,16 +393,22 @@ bool ScanFunctions::visit(Block *ast) {
 
 void ScanFunctions::enterFunction(Node *ast, const QString &name, FormalParameterList *formals, FunctionBody *body, FunctionExpression *expr)
 {
-    if (_context) {
-        _context->hasNestedFunctions = true;
+    Context *outerContext = _context;
+    enterEnvironment(ast, FunctionCode);
+
+    if (outerContext) {
+        outerContext->hasNestedFunctions = true;
         // The identifier of a function expression cannot be referenced from the enclosing environment.
-        if (expr)
-            _context->addLocalVar(name, Context::FunctionDefinition, AST::VariableDeclaration::FunctionScope, expr);
+        if (expr) {
+            if (!outerContext->addLocalVar(name, Context::FunctionDefinition, AST::VariableDeclaration::FunctionScope, expr)) {
+                _cg->throwSyntaxError(ast->firstSourceLocation(), QStringLiteral("Identifier %1 has already been declared").arg(name));
+                return;
+            }
+        }
         if (name == QLatin1String("arguments"))
-            _context->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
+            outerContext->usesArgumentsObject = Context::ArgumentsObjectNotUsed;
     }
 
-    enterEnvironment(ast, FunctionCode);
     if (formalsContainName(formals, QStringLiteral("arguments")))
         _context->usesArgumentsObject = Context::ArgumentsObjectNotUsed;