Correctly instantiate variables in the local scope

This fixes cases where eval() would create variables
in the wrong scope.

Change-Id: Ie93ec2d1fb125e588c1b6ffa2ca8ca4b6e3112c9
Reviewed-by: Erik Verbruggen <erik.verbruggen@digia.com>

9 files changed, 67 insertions, 46 deletions

diff --git a/qmljs_environment.cpp b/qmljs_environment.cpp
index 8093c120f2..0a8fadaf84 100644
--- a/qmljs_environment.cpp
+++ b/qmljs_environment.cpp
@@ -100,20 +100,22 @@ bool DeclarativeEnvironment::hasBinding(String *name) const
         if (__qmljs_string_equal(formals[i], name))
             return true;
     }
-    if (!deletableLocals)
-        return false;
-    return deletableLocals->contains(name->toQString());
+    return false;
 }
 
-void DeclarativeEnvironment::createMutableBinding(String *name, bool deletable)
+void DeclarativeEnvironment::createMutableBinding(ExecutionContext *ctx, String *name, bool deletable)
 {
-    // all non deletable vars should get created at compile time
-    assert(deletable);
-    assert(!hasBinding(name));
-
-    if (!deletableLocals)
-        deletableLocals = new QHash<QString, Value>();
-    deletableLocals->insert(name->toQString(), Value::undefinedValue());
+    if (activation) {
+        if (activation->__hasProperty__(ctx, name))
+            return;
+        PropertyDescriptor desc;
+        desc.value = Value::undefinedValue();
+        desc.type = PropertyDescriptor::Data;
+        desc.configurable = deletable ? PropertyDescriptor::Set : PropertyDescriptor::Unset;
+        desc.writable = PropertyDescriptor::Set;
+        desc.enumberable = PropertyDescriptor::Set;
+        activation->__defineOwnProperty__(ctx, name, &desc, true);
+    }
 }
 
 void DeclarativeEnvironment::setMutableBinding(String *name, Value value, bool strict)
@@ -133,12 +135,6 @@ void DeclarativeEnvironment::setMutableBinding(String *name, Value value, bool s
             return;
         }
     }
-    assert(deletableLocals);
-    QHash<QString, Value>::iterator it = deletableLocals->find(name->toQString());
-    if (it != deletableLocals->end()) {
-        *it = value;
-        return;
-    }
     assert(false);
 }
 
@@ -154,24 +150,16 @@ Value DeclarativeEnvironment::getBindingValue(String *name, bool strict) const
         if (__qmljs_string_equal(formals[i], name))
             return arguments[i];
     }
-    assert(deletableLocals);
-    QHash<QString, Value>::const_iterator it = deletableLocals->find(name->toQString());
-    if (it != deletableLocals->end())
-        return *it;
-
     assert(false);
 }
 
-bool DeclarativeEnvironment::deleteBinding(String *name)
+bool DeclarativeEnvironment::deleteBinding(ExecutionContext *ctx, String *name)
 {
-    if (deletableLocals) {
-        QHash<QString, Value>::iterator it = deletableLocals->find(name->toQString());
-        if (it != deletableLocals->end()) {
-            deletableLocals->erase(it);
-            return true;
-        }
-    }
-    return !hasBinding(name);
+    if (activation)
+        activation->__delete__(ctx, name, false);
+
+    // ### throw in strict mode?
+    return false;
 }
 
 void DeclarativeEnvironment::pushWithObject(Object *with)
diff --git a/qmljs_environment.h b/qmljs_environment.h
index 47376649d0..e631ec79a6 100644
--- a/qmljs_environment.h
+++ b/qmljs_environment.h
@@ -73,19 +73,15 @@ struct DeclarativeEnvironment
         With *next;
     } *withObject;
 
-    // these get used for createMutableBinding(..., true).
-    // the only place this is being used is eval(...)
-    QHash<QString, Value> *deletableLocals;
 
     DeclarativeEnvironment(ExecutionEngine *e);
     DeclarativeEnvironment(FunctionObject *f, Value *args, uint argc);
-    ~DeclarativeEnvironment() { delete deletableLocals; }
 
     bool hasBinding(String *name) const;
-    void createMutableBinding(String *name, bool deletable);
+    void createMutableBinding(ExecutionContext *ctx, String *name, bool deletable);
     void setMutableBinding(String *name, Value value, bool strict);
     Value getBindingValue(String *name, bool strict) const;
-    bool deleteBinding(String *name);
+    bool deleteBinding(ExecutionContext *ctx, String *name);
 
     // ### needs a bit of work in exception handlers
     void pushWithObject(Object *with);
diff --git a/qmljs_objects.cpp b/qmljs_objects.cpp
index 88a7c953be..a7e2f8068b 100644
--- a/qmljs_objects.cpp
+++ b/qmljs_objects.cpp
@@ -253,8 +253,9 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, String *name, Property
         if (!extensible)
             goto reject;
         // clause 4
-        *current = *desc;
-        current->fullyPopulated();
+        PropertyDescriptor *pd = members->insert(name);
+        *pd = *desc;
+        pd->fullyPopulated();
         return true;
     }
 
@@ -270,7 +271,7 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, String *name, Property
     if (!current->isConfigurable()) {
         if (desc->isConfigurable())
             goto reject;
-        if (desc->enumberable != PropertyDescriptor::Unset && desc->enumberable != current->enumberable)
+        if (desc->enumberable != PropertyDescriptor::Undefined && desc->enumberable != current->enumberable)
             goto reject;
     }
 
@@ -313,6 +314,7 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, String *name, Property
     *current += *desc;
     return true;
   reject:
+    qDebug() << "___put__ rejected" << name->toQString();
     if (throwException)
         __qmljs_throw_type_error(ctx);
     return false;
diff --git a/qmljs_runtime.cpp b/qmljs_runtime.cpp
index 86d1bbea09..1491b01446 100644
--- a/qmljs_runtime.cpp
+++ b/qmljs_runtime.cpp
@@ -865,6 +865,10 @@ void __qmljs_builtin_pop_with(ExecutionContext *ctx)
     ctx->lexicalEnvironment->popWithObject();
 }
 
+void __qmljs_builtin_declare_var(ExecutionContext *ctx, bool deletable, String *name)
+{
+    ctx->lexicalEnvironment->createMutableBinding(ctx, name, deletable);
+}
 
 } // extern "C"
 
diff --git a/qmljs_runtime.h b/qmljs_runtime.h
index 1831f68d60..a02713d941 100644
--- a/qmljs_runtime.h
+++ b/qmljs_runtime.h
@@ -103,6 +103,7 @@ Value __qmljs_builtin_typeof(Value val, ExecutionContext *context);
 void __qmljs_builtin_throw(Value val, ExecutionContext *context);
 void __qmljs_builtin_push_with(Value o, ExecutionContext *ctx);
 void __qmljs_builtin_pop_with(ExecutionContext *ctx);
+void __qmljs_builtin_declare_var(ExecutionContext *ctx, bool deletable, String *name);
 
 // constructors
 Value __qmljs_init_closure(IR::Function *clos, ExecutionContext *ctx);
diff --git a/qv4codegen.cpp b/qv4codegen.cpp
index cd4cbebb0a..e605e2493a 100644
--- a/qv4codegen.cpp
+++ b/qv4codegen.cpp
@@ -1517,15 +1517,30 @@ IR::Function *Codegen::defineFunction(const QString &name, AST::Node *ast,
     function->maxNumberOfArguments = _env->maxNumberOfArguments;
 
     // variables in global code are properties of the global context object, not locals as with other functions.
-    for (int i = 0; i < _env->vars.size(); ++i) {
-        const QString &local = _env->vars.at(i);
-        if (!_env->parent) {
-            entryBlock->MOVE(entryBlock->NAME(local, 0, 0), entryBlock->CONST(IR::UndefinedType, 0));
-        } else {
+    if (_mode == FunctionCode) {
+        for (int i = 0; i < _env->vars.size(); ++i) {
+            const QString &local = _env->vars.at(i);
             function->LOCAL(local);
             unsigned t = entryBlock->newTemp();
             assert(t == unsigned(i));
         }
+    } else {
+        IR::ExprList *args = 0;
+        for (int i = 0; i < _env->vars.size(); ++i) {
+            const QString &local = _env->vars.at(i);
+            IR::ExprList *next = function->New<IR::ExprList>();
+            next->expr = entryBlock->NAME(local, 0, 0);
+            next->next = args;
+            args = next;
+        }
+        if (args) {
+            IR::ExprList *next = function->New<IR::ExprList>();
+            next->expr = entryBlock->CONST(IR::BoolType, mode == EvalCode);
+            next->next = args;
+            args = next;
+
+            entryBlock->EXP(entryBlock->CALL(entryBlock->NAME(IR::Name::builtin_declare_vars, 0, 0), args));
+        }
     }
 
     unsigned returnAddress = entryBlock->newTemp();
diff --git a/qv4ir.cpp b/qv4ir.cpp
index 57f6de958f..7646b371d0 100644
--- a/qv4ir.cpp
+++ b/qv4ir.cpp
@@ -238,6 +238,8 @@ static const char *builtin_to_string(Name::Builtin b)
         return "builtin_push_with";
     case IR::Name::builtin_pop_with:
         return "builtin_pop_with";
+    case IR::Name::builtin_declare_vars:
+        return "builtin_declare_vars";
     }
     return "builtin_(###FIXME)";
 };
diff --git a/qv4ir_p.h b/qv4ir_p.h
index b0be0f9bb2..740ca9b3f9 100644
--- a/qv4ir_p.h
+++ b/qv4ir_p.h
@@ -278,7 +278,8 @@ struct Name: Expr {
         builtin_foreach_iterator_object,
         builtin_foreach_next_property_name,
         builtin_push_with,
-        builtin_pop_with
+        builtin_pop_with,
+        builtin_declare_vars
     };
 
     const QString *id;
diff --git a/qv4isel_masm.cpp b/qv4isel_masm.cpp
index f39c3b75b1..a0262c8079 100644
--- a/qv4isel_masm.cpp
+++ b/qv4isel_masm.cpp
@@ -279,6 +279,18 @@ void InstructionSelection::callActivationProperty(IR::Call *call, IR::Temp *resu
     case IR::Name::builtin_pop_with:
         generateFunctionCall(Void, __qmljs_builtin_pop_with, ContextRegister);
         break;
+    case IR::Name::builtin_declare_vars: {
+        if (!call->args)
+            return;
+        IR::Const *deletable = call->args->expr->asConst();
+        assert(deletable->type == IR::BoolType);
+        for (IR::ExprList *it = call->args->next; it; it = it->next) {
+            IR::Name *arg = it->expr->asName();
+            assert(arg != 0);
+            generateFunctionCall(Void, __qmljs_builtin_declare_var, ContextRegister,
+                                 TrustedImm32(deletable->value != 0), identifier(*arg->id));
+        }
+    }
     }
 }