Unify DeleteMember and DeleteSubscript instructions

The delete operator is rarely used, so it's simpler to
unify these into one DeleteProperty instruction.

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

9 files changed, 34 insertions, 92 deletions

diff --git a/src/qml/compiler/qv4bytecodehandler.cpp b/src/qml/compiler/qv4bytecodehandler.cpp
index 5709d84f58..18fef62dab 100644
--- a/src/qml/compiler/qv4bytecodehandler.cpp
+++ b/src/qml/compiler/qv4bytecodehandler.cpp
@@ -325,11 +325,8 @@ std::vector<int> ByteCodeHandler::collectLabelsInBytecode(const char *code, uint
         COLLECTOR_BEGIN_INSTR(DestructureRestElement)
         COLLECTOR_END_INSTR(DestructureRestElement)
 
-        COLLECTOR_BEGIN_INSTR(DeleteMember)
-        COLLECTOR_END_INSTR(DeleteMember)
-
-        COLLECTOR_BEGIN_INSTR(DeleteSubscript)
-        COLLECTOR_END_INSTR(DeleteSubscript)
+        COLLECTOR_BEGIN_INSTR(DeleteProperty)
+        COLLECTOR_END_INSTR(DeleteProperty)
 
         COLLECTOR_BEGIN_INSTR(DeleteName)
         COLLECTOR_END_INSTR(DeleteName)
diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index e39bb61688..72e6b2bdf5 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -1798,6 +1798,7 @@ bool Codegen::visit(DeleteExpression *ast)
     if (hasError)
         return false;
 
+    RegisterScope scope(this);
     Reference expr = expression(ast->expression);
     if (hasError)
         return false;
@@ -1829,9 +1830,14 @@ bool Codegen::visit(DeleteExpression *ast)
     case Reference::Member: {
         //### maybe add a variant where the base can be in the accumulator?
         expr = expr.asLValue();
-        Instruction::DeleteMember del;
+        Instruction::LoadRuntimeString instr;
+        instr.stringId = expr.propertyNameIndex;
+        bytecodeGenerator->addInstruction(instr);
+        Reference index = Reference::fromStackSlot(this);
+        index.storeConsumeAccumulator();
+        Instruction::DeleteProperty del;
         del.base = expr.propertyBase.stackSlot();
-        del.member = expr.propertyNameIndex;
+        del.index = index.stackSlot();
         bytecodeGenerator->addInstruction(del);
         _expr.setResult(Reference::fromAccumulator(this));
         return false;
@@ -1839,7 +1845,7 @@ bool Codegen::visit(DeleteExpression *ast)
     case Reference::Subscript: {
         //### maybe add a variant where the index can be in the accumulator?
         expr = expr.asLValue();
-        Instruction::DeleteSubscript del;
+        Instruction::DeleteProperty del;
         del.base = expr.elementBase;
         del.index = expr.elementSubscript.stackSlot();
         bytecodeGenerator->addInstruction(del);
diff --git a/src/qml/compiler/qv4instr_moth.cpp b/src/qml/compiler/qv4instr_moth.cpp
index 0fb789f479..b31b3f8584 100644
--- a/src/qml/compiler/qv4instr_moth.cpp
+++ b/src/qml/compiler/qv4instr_moth.cpp
@@ -449,13 +449,9 @@ void dumpBytecode(const char *code, int len, int nLocals, int nFormals, int /*st
         MOTH_BEGIN_INSTR(DestructureRestElement)
         MOTH_END_INSTR(DestructureRestElement)
 
-        MOTH_BEGIN_INSTR(DeleteMember)
-            d << dumpRegister(base, nFormals) << "[" << member << "]";
-        MOTH_END_INSTR(DeleteMember)
-
-        MOTH_BEGIN_INSTR(DeleteSubscript)
+        MOTH_BEGIN_INSTR(DeleteProperty)
             d << dumpRegister(base, nFormals) << "[" << dumpRegister(index, nFormals) << "]";
-        MOTH_END_INSTR(DeleteSubscript)
+        MOTH_END_INSTR(DeleteProperty)
 
         MOTH_BEGIN_INSTR(DeleteName)
             d << name;
diff --git a/src/qml/compiler/qv4instr_moth_p.h b/src/qml/compiler/qv4instr_moth_p.h
index a2bb85749b..4f6767f07f 100644
--- a/src/qml/compiler/qv4instr_moth_p.h
+++ b/src/qml/compiler/qv4instr_moth_p.h
@@ -129,8 +129,7 @@ QT_BEGIN_NAMESPACE
 #define INSTR_IteratorNext(op) INSTRUCTION(op, IteratorNext, 1, value)
 #define INSTR_IteratorClose(op) INSTRUCTION(op, IteratorClose, 1, done)
 #define INSTR_DestructureRestElement(op) INSTRUCTION(op, DestructureRestElement, 0)
-#define INSTR_DeleteMember(op) INSTRUCTION(op, DeleteMember, 2, member, base)
-#define INSTR_DeleteSubscript(op) INSTRUCTION(op, DeleteSubscript, 2, base, index)
+#define INSTR_DeleteProperty(op) INSTRUCTION(op, DeleteProperty, 2, base, index)
 #define INSTR_DeleteName(op) INSTRUCTION(op, DeleteName, 1, name)
 #define INSTR_TypeofName(op) INSTRUCTION(op, TypeofName, 1, name)
 #define INSTR_TypeofValue(op) INSTRUCTION(op, TypeofValue, 0)
@@ -258,8 +257,7 @@ QT_BEGIN_NAMESPACE
     F(IteratorNext) \
     F(IteratorClose) \
     F(DestructureRestElement) \
-    F(DeleteMember) \
-    F(DeleteSubscript) \
+    F(DeleteProperty) \
     F(DeleteName) \
     F(TypeofName) \
     F(TypeofValue) \
diff --git a/src/qml/jit/qv4baselinejit.cpp b/src/qml/jit/qv4baselinejit.cpp
index b994d38c2b..ac6fc976f4 100644
--- a/src/qml/jit/qv4baselinejit.cpp
+++ b/src/qml/jit/qv4baselinejit.cpp
@@ -710,35 +710,10 @@ void BaselineJIT::generate_DestructureRestElement()
     as->checkException();
 }
 
-
-
-static ReturnedValue deleteMemberHelper(QV4::Function *function, const QV4::Value &base, int member)
-{
-    auto engine = function->internalClass->engine;
-    if (!Runtime::method_deleteMember(engine, base, member)) {
-        if (function->isStrict())
-            engine->throwTypeError();
-        return Encode(false);
-    } else {
-        return Encode(true);
-    }
-}
-
-void BaselineJIT::generate_DeleteMember(int member, int base)
-{
-    STORE_IP();
-    as->prepareCallWithArgCount(3);
-    as->passInt32AsArg(member, 2);
-    as->passRegAsArg(base, 1);
-    as->passFunctionAsArg(0);
-    JIT_GENERATE_RUNTIME_CALL(deleteMemberHelper, Assembler::ResultInAccumulator);
-    as->checkException();
-}
-
-static ReturnedValue deleteSubscriptHelper(QV4::Function *function, const QV4::Value &base, const QV4::Value &index)
+static ReturnedValue deletePropertyHelper(QV4::Function *function, const QV4::Value &base, const QV4::Value &index)
 {
     auto engine = function->internalClass->engine;
-    if (!Runtime::method_deleteElement(engine, base, index)) {
+    if (!Runtime::method_deleteProperty(engine, base, index)) {
         if (function->isStrict())
             engine->throwTypeError();
         return Encode(false);
@@ -747,14 +722,14 @@ static ReturnedValue deleteSubscriptHelper(QV4::Function *function, const QV4::V
     }
 }
 
-void BaselineJIT::generate_DeleteSubscript(int base, int index)
+void BaselineJIT::generate_DeleteProperty(int base, int index)
 {
     STORE_IP();
     as->prepareCallWithArgCount(3);
     as->passRegAsArg(index, 2);
     as->passRegAsArg(base, 1);
     as->passFunctionAsArg(0);
-    JIT_GENERATE_RUNTIME_CALL(deleteSubscriptHelper, Assembler::ResultInAccumulator);
+    JIT_GENERATE_RUNTIME_CALL(deletePropertyHelper, Assembler::ResultInAccumulator);
     as->checkException();
 }
 
diff --git a/src/qml/jit/qv4baselinejit_p.h b/src/qml/jit/qv4baselinejit_p.h
index 112cb92c75..fda410db69 100644
--- a/src/qml/jit/qv4baselinejit_p.h
+++ b/src/qml/jit/qv4baselinejit_p.h
@@ -146,8 +146,7 @@ public:
     void generate_IteratorNext(int value) override;
     void generate_IteratorClose(int done) override;
     void generate_DestructureRestElement() override;
-    void generate_DeleteMember(int member, int base) override;
-    void generate_DeleteSubscript(int base, int index) override;
+    void generate_DeleteProperty(int base, int index) override;
     void generate_DeleteName(int name) override;
     void generate_TypeofName(int name) override;
     void generate_TypeofValue() override;
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index b879e3fc14..4c2477ded2 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -321,36 +321,22 @@ ReturnedValue Runtime::method_closure(ExecutionEngine *engine, int functionId)
     return FunctionObject::createScriptFunction(current, clos)->asReturnedValue();
 }
 
-bool Runtime::method_deleteElement(ExecutionEngine *engine, const Value &base, const Value &index)
+bool Runtime::method_deleteProperty(ExecutionEngine *engine, const Value &base, const Value &index)
 {
     Scope scope(engine);
-    ScopedObject o(scope, base);
-    if (o) {
-        uint n = index.asArrayIndex();
-        if (n < UINT_MAX)
-            return o->deleteIndexedProperty(n);
-    }
+    ScopedObject o(scope, base.toObject(engine));
+    if (scope.engine->hasException)
+        return Encode::undefined();
+    Q_ASSERT(o);
+
+    uint n = index.asArrayIndex();
+    if (n < UINT_MAX)
+        return o->deleteIndexedProperty(n);
 
     ScopedStringOrSymbol name(scope, index.toPropertyKey(engine));
     if (engine->hasException)
         return false;
-    return method_deleteMemberString(engine, base, name);
-}
-
-bool Runtime::method_deleteMember(ExecutionEngine *engine, const Value &base, int nameIndex)
-{
-    Scope scope(engine);
-    ScopedString name(scope, engine->currentStackFrame->v4Function->compilationUnit->runtimeStrings[nameIndex]);
-    return method_deleteMemberString(engine, base, name);
-}
-
-bool Runtime::method_deleteMemberString(ExecutionEngine *engine, const Value &base, StringOrSymbol *name)
-{
-    Scope scope(engine);
-    ScopedObject obj(scope, base.toObject(engine));
-    if (scope.engine->hasException)
-        return Encode::undefined();
-    return obj->deleteProperty(name);
+    return o->deleteProperty(name);
 }
 
 bool Runtime::method_deleteName(ExecutionEngine *engine, int nameIndex)
diff --git a/src/qml/jsruntime/qv4runtimeapi_p.h b/src/qml/jsruntime/qv4runtimeapi_p.h
index 66d3bb2a81..63f4f426aa 100644
--- a/src/qml/jsruntime/qv4runtimeapi_p.h
+++ b/src/qml/jsruntime/qv4runtimeapi_p.h
@@ -117,9 +117,7 @@ struct ExceptionCheck<void (*)(QV4::NoThrowEngine *, A, B, C)> {
     F(ReturnedValue, typeofName, (ExecutionEngine *engine, int nameIndex)) \
     \
     /* delete */ \
-    F(bool, deleteElement, (ExecutionEngine *engine, const Value &base, const Value &index)) \
-    F(bool, deleteMember, (ExecutionEngine *engine, const Value &base, int nameIndex)) \
-    F(bool, deleteMemberString, (ExecutionEngine *engine, const Value &base, StringOrSymbol *name)) \
+    F(bool, deleteProperty, (ExecutionEngine *engine, const Value &base, const Value &index)) \
     F(bool, deleteName, (ExecutionEngine *engine, int nameIndex)) \
     \
     /* exceptions & scopes */ \
diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp
index a5d114dd78..bf77a96bec 100644
--- a/src/qml/jsruntime/qv4vme_moth.cpp
+++ b/src/qml/jsruntime/qv4vme_moth.cpp
@@ -995,8 +995,8 @@ QV4::ReturnedValue VME::interpret(CppStackFrame &frame, const char *code)
         CHECK_EXCEPTION;
     MOTH_END_INSTR(DestructureRestElement)
 
-    MOTH_BEGIN_INSTR(DeleteMember)
-        if (!Runtime::method_deleteMember(engine, STACK_VALUE(base), member)) {
+    MOTH_BEGIN_INSTR(DeleteProperty)
+        if (!Runtime::method_deleteProperty(engine, STACK_VALUE(base), STACK_VALUE(index))) {
             if (function->isStrict()) {
                 STORE_IP();
                 engine->throwTypeError();
@@ -1006,20 +1006,7 @@ QV4::ReturnedValue VME::interpret(CppStackFrame &frame, const char *code)
         } else {
             acc = Encode(true);
         }
-    MOTH_END_INSTR(DeleteMember)
-
-    MOTH_BEGIN_INSTR(DeleteSubscript)
-        if (!Runtime::method_deleteElement(engine, STACK_VALUE(base), STACK_VALUE(index))) {
-            if (function->isStrict()) {
-                STORE_IP();
-                engine->throwTypeError();
-                goto handleUnwind;
-            }
-            acc = Encode(false);
-        } else {
-            acc = Encode(true);
-        }
-    MOTH_END_INSTR(DeleteSubscript)
+    MOTH_END_INSTR(DeleteProperty)
 
     MOTH_BEGIN_INSTR(DeleteName)
         if (!Runtime::method_deleteName(engine, name)) {