Fix handling of const initialization. We did not handle the fact that

a const variable can be deleted between its declaration and its
initialization.

This fixes issue 189:

  http://code.google.com/p/v8/issues/detail?id=189

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

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

src/runtime.cc

@@ -758,56 +758,94 @@ static Object* Runtime_InitializeConstContextSlot(Arguments args) {
 
   int index;
   PropertyAttributes attributes;
-  ContextLookupFlags flags = DONT_FOLLOW_CHAINS;
+  ContextLookupFlags flags = FOLLOW_CHAINS;
   Handle<Object> holder =
       context->Lookup(name, flags, &index, &attributes);
 
-  // The property should always be present. It is always declared
-  // before being initialized through DeclareContextSlot.
-  ASSERT(attributes != ABSENT && (attributes & READ_ONLY) != 0);
-
-  // If the slot is in the context, we set it but only if it hasn't
-  // been set before.
+  // In most situations, the property introduced by the const
+  // declaration should be present in the context extension object.
+  // However, because declaration and initialization are separate, the
+  // property might have been deleted (if it was introduced by eval)
+  // before we reach the initialization point.
+  //
+  // Example:
+  //
+  //    function f() { eval("delete x; const x;"); }
+  //
+  // In that case, the initialization behaves like a normal assignment
+  // to property 'x'.
   if (index >= 0) {
-    // The constant context slot should always be in the function
-    // context; not in any outer context nor in the arguments object.
-    ASSERT(holder.is_identical_to(context));
-    if (context->get(index)->IsTheHole()) {
-      context->set(index, *value);
+    // Property was found in a context.
+    if (holder->IsContext()) {
+      // The holder cannot be the function context.  If it is, there
+      // should have been a const redeclaration error when declaring
+      // the const property.
+      ASSERT(!holder.is_identical_to(context));
+      if ((attributes & READ_ONLY) == 0) {
+        Handle<Context>::cast(holder)->set(index, *value);
+      }
+    } else {
+      // The holder is an arguments object.
+      ASSERT((attributes & READ_ONLY) == 0);
+      Handle<JSObject>::cast(holder)->SetElement(index, *value);
     }
     return *value;
   }
 
-  // Otherwise, the slot must be in a JS object extension.
-  Handle<JSObject> context_ext(JSObject::cast(*holder));
+  // The property could not be found, we introduce it in the global
+  // context.
+  if (attributes == ABSENT) {
+    Handle<JSObject> global = Handle<JSObject>(Top::context()->global());
+    SetProperty(global, name, value, NONE);
+    return *value;
+  }
 
-  // We must initialize the value only if it wasn't initialized
-  // before, e.g. for const declarations in a loop. The property has
-  // the hole value if it wasn't initialized yet. NOTE: We cannot use
-  // GetProperty() to get the current value as it 'unholes' the value.
-  LookupResult lookup;
-  context_ext->LocalLookupRealNamedProperty(*name, &lookup);
-  ASSERT(lookup.IsProperty());  // the property was declared
-  ASSERT(lookup.IsReadOnly());  // and it was declared as read-only
+  // The property was present in a context extension object.
+  Handle<JSObject> context_ext = Handle<JSObject>::cast(holder);
 
-  PropertyType type = lookup.type();
-  if (type == FIELD) {
-    FixedArray* properties = context_ext->properties();
-    int index = lookup.GetFieldIndex();
-    if (properties->get(index)->IsTheHole()) {
-      properties->set(index, *value);
-    }
-  } else if (type == NORMAL) {
-    Dictionary* dictionary = context_ext->property_dictionary();
-    int entry = lookup.GetDictionaryEntry();
-    if (dictionary->ValueAt(entry)->IsTheHole()) {
-      dictionary->ValueAtPut(entry, *value);
+  if (*context_ext == context->extension()) {
+    // This is the property that was introduced by the const
+    // declaration.  Set it if it hasn't been set before.  NOTE: We
+    // cannot use GetProperty() to get the current value as it
+    // 'unholes' the value.
+    LookupResult lookup;
+    context_ext->LocalLookupRealNamedProperty(*name, &lookup);
+    ASSERT(lookup.IsProperty());  // the property was declared
+    ASSERT(lookup.IsReadOnly());  // and it was declared as read-only
+
+    PropertyType type = lookup.type();
+    if (type == FIELD) {
+      FixedArray* properties = context_ext->properties();
+      int index = lookup.GetFieldIndex();
+      if (properties->get(index)->IsTheHole()) {
+        properties->set(index, *value);
+      }
+    } else if (type == NORMAL) {
+      Dictionary* dictionary = context_ext->property_dictionary();
+      int entry = lookup.GetDictionaryEntry();
+      if (dictionary->ValueAt(entry)->IsTheHole()) {
+        dictionary->ValueAtPut(entry, *value);
+      }
+    } else {
+      // We should not reach here. Any real, named property should be
+      // either a field or a dictionary slot.
+      UNREACHABLE();
     }
   } else {
-    // We should not reach here. Any real, named property should be
-    // either a field or a dictionary slot.
-    UNREACHABLE();
+    // The property was found in a different context extension object.
+    // Set it if it is not a read-only property.
+    if ((attributes & READ_ONLY) == 0) {
+      Handle<Object> set = SetProperty(context_ext, name, value, attributes);
+      // Setting a property might throw an exception.  Exceptions
+      // are converted to empty handles in handle operations.  We
+      // need to convert back to exceptions here.
+      if (set.is_null()) {
+        ASSERT(Top::has_pending_exception());
+        return Failure::Exception();
+      }
+    }
   }
+
   return *value;
 }
 

test/mjsunit/const-eval-init.js

+// Copyright 2009 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Test the handling of initialization of deleted const variables.
+// This only makes sense in local scopes since the declaration and
+// initialization of consts in the global scope happen at the same
+// time.
+
+function testIntroduceGlobal() {
+  var source =
+      // Deleting 'x' removes the local const property.
+      "delete x;" +
+      // Initialization turns into assignment to global 'x'.
+      "const x = 3; assertEquals(3, x);" +
+      // No constness of the global 'x'.
+      "x = 4; assertEquals(4, x);";
+  eval(source);
+}
+
+testIntroduceGlobal();
+assertEquals(4, x);
+
+function testAssignExistingGlobal() {
+  var source =
+      // Delete 'x' to remove the local const property.
+      "delete x;" +
+      // Initialization turns into assignment to global 'x'.
+      "const x = 5; assertEquals(5, x);" +
+      // No constness of the global 'x'.
+      "x = 6; assertEquals(6, x);";
+  eval(source);
+}
+
+testAssignExistingGlobal();
+assertEquals(6, x);
+
+function testAssignmentArgument(x) {
+  function local() {
+    var source = "delete x; const x = 7; assertEquals(7, x)";
+    eval(source);
+  }
+  local();
+  assertEquals(7, x);
+}
+
+testAssignmentArgument();
+assertEquals(6, x);
+
+__defineSetter__('x', function() { throw 42; });
+function testAssignGlobalThrows() {
+  // Initialization turns into assignment to global 'x' which
+  // throws an exception.
+  var source = "delete x; const x = 8";
+  eval(source);
+}
+
+assertThrows("testAssignGlobalThrows()");
+
+function testInitFastCaseExtension() {
+  var source = "const x = 9; assertEquals(9, x); x = 10; assertEquals(9, x)";
+  eval(source);
+}
+
+testInitFastCaseExtension();
+
+function testInitSlowCaseExtension() {
+  var source = "";
+  // Introduce 100 properties on the context extension object to force
+  // it in slow case.
+  for (var i = 0; i < 100; i++) source += ("var a" + i + " = i;");
+  source += "const x = 10; assertEquals(10, x); x = 11; assertEquals(10, x)";
+  eval(source);
+}
+
+testInitSlowCaseExtension();
+
+function testAssignSurroundingContextSlot() {
+  var x = 12;
+  function local() {
+    var source = "delete x; const x = 13; assertEquals(13, x)";
+    eval(source);
+  }
+  local();
+  assertEquals(13, x);
+}
+
+testAssignSurroundingContextSlot();

test/mjsunit/const.js

@@ -52,17 +52,19 @@ var valueOfCount = 0;
 function g() {
   const o = { valueOf: function() { valueOfCount++; return 42; } }
   assertEquals(42, o);
-  assertEquals(0, valueOfCount);
+  assertEquals(1, valueOfCount);
   o++;
   assertEquals(42, o);
-  assertEquals(1, valueOfCount);
+  assertEquals(3, valueOfCount);
   ++o;
   assertEquals(42, o);
-  assertEquals(2, valueOfCount);
+  assertEquals(5, valueOfCount);
   o--;
   assertEquals(42, o);
-  assertEquals(3, valueOfCount);
+  assertEquals(7, valueOfCount);
   --o;
   assertEquals(42, o);
-  assertEquals(4, valueOfCount);
+  assertEquals(9, valueOfCount);
 }
+
+g();

test/mjsunit/regress/regress-189.js

+// Copyright 2009 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Test that we can handle initialization of a deleted const variable.
+
+// See http://code.google.com/p/v8/issues/detail?id=189.
+
+function f() {
+  eval("delete x; const x = 32");
+}
+
+f();