Strict mode delete of non-configurable property.

Strict mode flag is passed to runtime DELETE function
and then to JSObject::Delete(Property/Element) as STRICT_DELETION enum.
When deleting non-configurable property/eleemnt, TypeError is thrown.
Adding mozilla test to .gitignore.
Incorporate CR feedback.

Review URL: http://codereview.chromium.org/6515005/

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6782 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/.gitignore b/.gitignore
index 2eab942..db57d1b 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -21,6 +21,7 @@ shell
 shell_g
 /obj/
 /test/es5conform/data/
+/test/mozilla/data/
 /test/sputnik/sputniktests/
 /tools/oom_dump/oom_dump
 /tools/oom_dump/oom_dump.o

diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc
index c827110..a3921d8 100644 (file)
--- a/src/arm/codegen-arm.cc
+++ b/src/arm/codegen-arm.cc
@@ -5844,15 +5844,20 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) {
     if (property != NULL) {
       Load(property->obj());
       Load(property->key());
-      frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 2);
+      frame_->EmitPush(Operand(Smi::FromInt(strict_mode_flag())));
+      frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 3);
       frame_->EmitPush(r0);
 
     } else if (variable != NULL) {
+      // Delete of an unqualified identifier is disallowed in strict mode
+      // so this code can only be reached in non-strict mode.
+      ASSERT(strict_mode_flag() == kNonStrictMode);
       Slot* slot = variable->AsSlot();
       if (variable->is_global()) {
         LoadGlobal();
         frame_->EmitPush(Operand(variable->name()));
-        frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 2);
+        frame_->EmitPush(Operand(Smi::FromInt(kNonStrictMode)));
+        frame_->InvokeBuiltin(Builtins::DELETE, CALL_JS, 3);
         frame_->EmitPush(r0);
 
       } else if (slot != NULL && slot->type() == Slot::LOOKUP) {

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index 801296b..29a1971 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -3062,14 +3062,20 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
         } else {
           VisitForStackValue(prop->obj());
           VisitForStackValue(prop->key());
+          __ mov(r1, Operand(Smi::FromInt(strict_mode_flag())));
+          __ push(r1);
           __ InvokeBuiltin(Builtins::DELETE, CALL_JS);
           context()->Plug(r0);
         }
       } else if (var != NULL) {
+        // Delete of an unqualified identifier is disallowed in strict mode
+        // so this code can only be reached in non-strict mode.
+        ASSERT(strict_mode_flag() == kNonStrictMode);
         if (var->is_global()) {
-          __ ldr(r1, GlobalObjectOperand());
-          __ mov(r0, Operand(var->name()));
-          __ Push(r1, r0);
+          __ ldr(r2, GlobalObjectOperand());
+          __ mov(r1, Operand(var->name()));
+          __ mov(r0, Operand(Smi::FromInt(kNonStrictMode)));
+          __ Push(r2, r1, r0);
           __ InvokeBuiltin(Builtins::DELETE, CALL_JS);
           context()->Plug(r0);
         } else if (var->AsSlot() != NULL &&

diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 057ac24..4a30d7c 100644 (file)
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -3888,7 +3888,9 @@ void LCodeGen::DoDeoptimize(LDeoptimize* instr) {
 void LCodeGen::DoDeleteProperty(LDeleteProperty* instr) {
   Register object = ToRegister(instr->object());
   Register key = ToRegister(instr->key());
-  __ Push(object, key);
+  Register strict = scratch0();
+  __ mov(strict, Operand(Smi::FromInt(strict_mode_flag())));
+  __ Push(object, key, strict);
   ASSERT(instr->HasPointerMap() && instr->HasDeoptimizationEnvironment());
   LPointerMap* pointers = instr->pointer_map();
   LEnvironment* env = instr->deoptimization_environment();

diff --git a/src/arm/lithium-codegen-arm.h b/src/arm/lithium-codegen-arm.h
index 7bc6689..732db44 100644 (file)
--- a/src/arm/lithium-codegen-arm.h
+++ b/src/arm/lithium-codegen-arm.h
@@ -129,6 +129,10 @@ class LCodeGen BASE_EMBEDDED {
   bool is_done() const { return status_ == DONE; }
   bool is_aborted() const { return status_ == ABORTED; }
 
+  int strict_mode_flag() const {
+    return info_->is_strict() ? kStrictMode : kNonStrictMode;
+  }
+
   LChunk* chunk() const { return chunk_; }
   Scope* scope() const { return scope_; }
   HGraph* graph() const { return chunk_->graph(); }

diff --git a/src/builtins.h b/src/builtins.h
index ada23a7..2733410 100644 (file)
--- a/src/builtins.h
+++ b/src/builtins.h
@@ -214,7 +214,7 @@ enum BuiltinExtraArguments {
   V(SHL, 1)                              \
   V(SAR, 1)                              \
   V(SHR, 1)                              \
-  V(DELETE, 1)                           \
+  V(DELETE, 2)                           \
   V(IN, 1)                               \
   V(INSTANCE_OF, 1)                      \
   V(GET_KEYS, 0)                         \

diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index b977db8..02e2919 100644 (file)
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -8225,19 +8225,24 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) {
     if (property != NULL) {
       Load(property->obj());
       Load(property->key());
-      Result answer = frame_->InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, 2);
+      frame_->Push(Smi::FromInt(strict_mode_flag()));
+      Result answer = frame_->InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, 3);
       frame_->Push(&answer);
       return;
     }
 
     Variable* variable = node->expression()->AsVariableProxy()->AsVariable();
     if (variable != NULL) {
+      // Delete of an unqualified identifier is disallowed in strict mode
+      // so this code can only be reached in non-strict mode.
+      ASSERT(strict_mode_flag() == kNonStrictMode);
       Slot* slot = variable->AsSlot();
       if (variable->is_global()) {
         LoadGlobal();
         frame_->Push(variable->name());
+        frame_->Push(Smi::FromInt(kNonStrictMode));
         Result answer = frame_->InvokeBuiltin(Builtins::DELETE,
-                                              CALL_FUNCTION, 2);
+                                              CALL_FUNCTION, 3);
         frame_->Push(&answer);
         return;
 

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index ada7b3d..8427983 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -3730,13 +3730,18 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
         } else {
           VisitForStackValue(prop->obj());
           VisitForStackValue(prop->key());
+          __ push(Immediate(Smi::FromInt(strict_mode_flag())));
           __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
           context()->Plug(eax);
         }
       } else if (var != NULL) {
+        // Delete of an unqualified identifier is disallowed in strict mode
+        // so this code can only be reached in non-strict mode.
+        ASSERT(strict_mode_flag() == kNonStrictMode);
         if (var->is_global()) {
           __ push(GlobalObjectOperand());
           __ push(Immediate(var->name()));
+          __ push(Immediate(Smi::FromInt(kNonStrictMode)));
           __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
           context()->Plug(eax);
         } else if (var->AsSlot() != NULL &&

diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index 7d5de29..a25d678 100644 (file)
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -3735,6 +3735,7 @@ void LCodeGen::DoDeleteProperty(LDeleteProperty* instr) {
                                          pointers,
                                          env->deoptimization_index());
   __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
+  __ push(Immediate(Smi::FromInt(strict_mode_flag())));
   __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, &safepoint_generator);
 }
 

diff --git a/src/ia32/lithium-codegen-ia32.h b/src/ia32/lithium-codegen-ia32.h
index 3ac3a41..977fbcd 100644 (file)
--- a/src/ia32/lithium-codegen-ia32.h
+++ b/src/ia32/lithium-codegen-ia32.h
@@ -120,6 +120,10 @@ class LCodeGen BASE_EMBEDDED {
   bool is_done() const { return status_ == DONE; }
   bool is_aborted() const { return status_ == ABORTED; }
 
+  int strict_mode_flag() const {
+    return info_->is_strict() ? kStrictMode : kNonStrictMode;
+  }
+
   LChunk* chunk() const { return chunk_; }
   Scope* scope() const { return scope_; }
   HGraph* graph() const { return chunk_->graph(); }

diff --git a/src/messages.js b/src/messages.js
index ac7962c..b7e57aa 100644 (file)
--- a/src/messages.js
+++ b/src/messages.js
@@ -225,6 +225,7 @@ function FormatMessage(message) {
       strict_lhs_prefix:            ["Prefix increment/decrement may not have eval or arguments operand in strict mode"],
       strict_reserved_word:         ["Use of future reserved word in strict mode"],
       strict_delete:                ["Delete of an unqualified identifier in strict mode."],
+      strict_delete_property:       ["Cannot delete property '", "%0", "' of ", "%1"],
     };
   }
   var message_type = %MessageGetType(message);

diff --git a/src/objects.cc b/src/objects.cc
index 5003b4f..31566ff 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1399,7 +1399,7 @@ MaybeObject* JSObject::AddProperty(String* name,
   if (!map()->is_extensible()) {
     Handle<Object> args[1] = {Handle<String>(name)};
     return Top::Throw(*Factory::NewTypeError("object_not_extensible",
-                                               HandleVector(args, 1)));
+                                             HandleVector(args, 1)));
   }
   if (HasFastProperties()) {
     // Ensure the descriptor array does not get too big.
@@ -2620,7 +2620,17 @@ MaybeObject* JSObject::DeleteElement(uint32_t index, DeleteMode mode) {
       NumberDictionary* dictionary = element_dictionary();
       int entry = dictionary->FindEntry(index);
       if (entry != NumberDictionary::kNotFound) {
-        return dictionary->DeleteProperty(entry, mode);
+        Object* result = dictionary->DeleteProperty(entry, mode);
+        if (mode == STRICT_DELETION && result == Heap::false_value()) {
+          // In strict mode, deleting a non-configurable property throws
+          // exception. dictionary->DeleteProperty will return false_value()
+          // if a non-configurable property is being deleted.
+          HandleScope scope;
+          Handle<Object> i = Factory::NewNumberFromUint(index);
+          Handle<Object> args[2] = { i, Handle<Object>(this) };
+          return Top::Throw(*Factory::NewTypeError("strict_delete_property",
+                                                   HandleVector(args, 2)));
+        }
       }
       break;
     }
@@ -2659,6 +2669,13 @@ MaybeObject* JSObject::DeleteProperty(String* name, DeleteMode mode) {
     if (!result.IsProperty()) return Heap::true_value();
     // Ignore attributes if forcing a deletion.
     if (result.IsDontDelete() && mode != FORCE_DELETION) {
+      if (mode == STRICT_DELETION) {
+        // Deleting a non-configurable property in strict mode.
+        HandleScope scope;
+        Handle<Object> args[2] = { Handle<Object>(name), Handle<Object>(this) };
+        return Top::Throw(*Factory::NewTypeError("strict_delete_property",
+                                                 HandleVector(args, 2)));
+      }
       return Heap::false_value();
     }
     // Check for interceptor.
@@ -9345,7 +9362,7 @@ Object* Dictionary<Shape, Key>::DeleteProperty(int entry,
                                                JSObject::DeleteMode mode) {
   PropertyDetails details = DetailsAt(entry);
   // Ignore attributes if forcing a deletion.
-  if (details.IsDontDelete() && mode == JSObject::NORMAL_DELETION) {
+  if (details.IsDontDelete() && mode != JSObject::FORCE_DELETION) {
     return Heap::false_value();
   }
   SetEntry(entry, Heap::null_value(), Heap::null_value(), Smi::FromInt(0));

diff --git a/src/objects.h b/src/objects.h
index 264cc0b..559ada6 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -1286,7 +1286,12 @@ class HeapNumber: public HeapObject {
 // caching.
 class JSObject: public HeapObject {
  public:
-  enum DeleteMode { NORMAL_DELETION, FORCE_DELETION };
+  enum DeleteMode {
+    NORMAL_DELETION,
+    STRICT_DELETION,
+    FORCE_DELETION
+  };
+
   enum ElementsKind {
     // The only "fast" kind.
     FAST_ELEMENTS,

diff --git a/src/runtime.cc b/src/runtime.cc
index 60e5e38..b83164e 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -3914,11 +3914,14 @@ static MaybeObject* Runtime_IgnoreAttributesAndSetProperty(Arguments args) {
 
 static MaybeObject* Runtime_DeleteProperty(Arguments args) {
   NoHandleAllocation ha;
-  ASSERT(args.length() == 2);
+  ASSERT(args.length() == 3);
 
   CONVERT_CHECKED(JSObject, object, args[0]);
   CONVERT_CHECKED(String, key, args[1]);
-  return object->DeleteProperty(key, JSObject::NORMAL_DELETION);
+  CONVERT_SMI_CHECKED(strict, args[2]);
+  return object->DeleteProperty(key, strict == kStrictMode
+                                      ? JSObject::STRICT_DELETION
+                                      : JSObject::NORMAL_DELETION);
 }
 
 

diff --git a/src/runtime.h b/src/runtime.h
index 8551196..06437ef 100644 (file)
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -45,7 +45,7 @@ namespace internal {
   /* Property access */ \
   F(GetProperty, 2, 1) \
   F(KeyedGetProperty, 2, 1) \
-  F(DeleteProperty, 2, 1) \
+  F(DeleteProperty, 3, 1) \
   F(HasLocalProperty, 2, 1) \
   F(HasProperty, 2, 1) \
   F(HasElement, 2, 1) \

diff --git a/src/runtime.js b/src/runtime.js
index 2cdbbde..66d839b 100644 (file)
--- a/src/runtime.js
+++ b/src/runtime.js
@@ -338,8 +338,8 @@ function SHR(y) {
 */
 
 // ECMA-262, section 11.4.1, page 46.
-function DELETE(key) {
-  return %DeleteProperty(%ToObject(this), %ToString(key));
+function DELETE(key, strict) {
+  return %DeleteProperty(%ToObject(this), %ToString(key), strict);
 }
 
 

diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc
index fe90567..150ed66 100644 (file)
--- a/src/x64/codegen-x64.cc
+++ b/src/x64/codegen-x64.cc
@@ -7230,19 +7230,24 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) {
     if (property != NULL) {
       Load(property->obj());
       Load(property->key());
-      Result answer = frame_->InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, 2);
+      frame_->Push(Smi::FromInt(strict_mode_flag()));
+      Result answer = frame_->InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, 3);
       frame_->Push(&answer);
       return;
     }
 
     Variable* variable = node->expression()->AsVariableProxy()->AsVariable();
     if (variable != NULL) {
+      // Delete of an unqualified identifier is disallowed in strict mode
+      // so this code can only be reached in non-strict mode.
+      ASSERT(strict_mode_flag() == kNonStrictMode);
       Slot* slot = variable->AsSlot();
       if (variable->is_global()) {
         LoadGlobal();
         frame_->Push(variable->name());
+        frame_->Push(Smi::FromInt(kNonStrictMode));
         Result answer = frame_->InvokeBuiltin(Builtins::DELETE,
-                                              CALL_FUNCTION, 2);
+                                              CALL_FUNCTION, 3);
         frame_->Push(&answer);
         return;
 

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index 24a8524..a28bcb7 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -3069,13 +3069,18 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
         } else {
           VisitForStackValue(prop->obj());
           VisitForStackValue(prop->key());
+          __ Push(Smi::FromInt(strict_mode_flag()));
           __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
           context()->Plug(rax);
         }
       } else if (var != NULL) {
+        // Delete of an unqualified identifier is disallowed in strict mode
+        // so this code can only be reached in non-strict mode.
+        ASSERT(strict_mode_flag() == kNonStrictMode);
         if (var->is_global()) {
           __ push(GlobalObjectOperand());
           __ Push(var->name());
+          __ Push(Smi::FromInt(kNonStrictMode));
           __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION);
           context()->Plug(rax);
         } else if (var->AsSlot() != NULL &&

diff --git a/test/es5conform/es5conform.status b/test/es5conform/es5conform.status
index 4cf0118..d7b10a8 100644 (file)
--- a/test/es5conform/es5conform.status
+++ b/test/es5conform/es5conform.status
@@ -347,15 +347,11 @@ chapter11/11.13/11.13.1/11.13.1-4-26-s: FAIL
 # in strict mode (Global.undefined)
 chapter11/11.13/11.13.1/11.13.1-4-27-s: FAIL
 
-# delete operator throws TypeError when deleting a non-configurable data
-# property in strict mode
-chapter11/11.4/11.4.1/11.4.1-4.a-3-s: FAIL
 # delete operator throws TypeError when when deleting a non-configurable
 # data property in strict mode (Global.NaN)
-chapter11/11.4/11.4.1/11.4.1-4.a-4-s: FAIL
-# delete operator throws TypeError when deleting a non-configurable data
-# property in strict mode (Math.LN2)
-chapter11/11.4/11.4.1/11.4.1-4.a-9-s: FAIL
+# Invalid test case - "this" is not a global object within the test case.
+# (http://es5conform.codeplex.com/workitem/29151)
+chapter11/11.4/11.4.1/11.4.1-4.a-4-s: FAIL_OK
 
 # delete operator throws ReferenceError when deleting a direct reference
 # to a var in strict mode

diff --git a/test/mjsunit/strict-mode.js b/test/mjsunit/strict-mode.js
index c6e6ca4..fead64a 100644 (file)
--- a/test/mjsunit/strict-mode.js
+++ b/test/mjsunit/strict-mode.js
@@ -280,7 +280,7 @@ CheckStrictMode("function strict() { print(--arguments); }", SyntaxError);
 CheckStrictMode("function strict() { var x = --eval; }", SyntaxError);
 CheckStrictMode("function strict() { var x = --arguments; }", SyntaxError);
 
-// Delete of an unqialified identifier
+// Delete of an unqualified identifier
 CheckStrictMode("delete unqualified;", SyntaxError);
 CheckStrictMode("function strict() { delete unqualified; }", SyntaxError);
 CheckStrictMode("function function_name() { delete function_name; }",
@@ -406,3 +406,33 @@ delete possibly_undefined_variable_for_strict_mode_test;
 repeat(10, function() { testAssignToUndefined(true); });
 possibly_undefined_variable_for_strict_mode_test = undefined;
 repeat(10, function() { testAssignToUndefined(false); });
+
+(function testDeleteNonConfigurable() {
+  function delete_property(o) {
+    "use strict";
+    delete o.property;
+  }
+  function delete_element(o, i) {
+    "use strict";
+    delete o[i];
+  }
+
+  var object = {};
+
+  Object.defineProperty(object, "property", { value: "property_value" });
+  Object.defineProperty(object, "1", { value: "one" });
+  Object.defineProperty(object, 7, { value: "seven" });
+  Object.defineProperty(object, 3.14, { value: "pi" });
+
+  assertThrows(function() { delete_property(object); }, TypeError);
+  assertEquals(object.property, "property_value");
+  assertThrows(function() { delete_element(object, "1"); }, TypeError);
+  assertThrows(function() { delete_element(object, 1); }, TypeError);
+  assertEquals(object[1], "one");
+  assertThrows(function() { delete_element(object, "7"); }, TypeError);
+  assertThrows(function() { delete_element(object, 7); }, TypeError);
+  assertEquals(object[7], "seven");
+  assertThrows(function() { delete_element(object, "3.14"); }, TypeError);
+  assertThrows(function() { delete_element(object, 3.14); }, TypeError);
+  assertEquals(object[3.14], "pi");
+})();