Author: [email protected]
Date: Thu Jan 22 05:53:06 2009
New Revision: 1127
Added:
branches/bleeding_edge/test/mjsunit/const-eval-init.js
branches/bleeding_edge/test/mjsunit/regress/regress-189.js
Modified:
branches/bleeding_edge/src/runtime.cc
branches/bleeding_edge/test/mjsunit/const.js
Log:
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
Modified: branches/bleeding_edge/src/runtime.cc
==============================================================================
--- branches/bleeding_edge/src/runtime.cc (original)
+++ branches/bleeding_edge/src/runtime.cc Thu Jan 22 05:53:06 2009
@@ -758,56 +758,94 @@
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
-
- 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);
+ // The property was present in a context extension object.
+ Handle<JSObject> context_ext = Handle<JSObject>::cast(holder);
+
+ 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;
}
Added: branches/bleeding_edge/test/mjsunit/const-eval-init.js
==============================================================================
--- (empty file)
+++ branches/bleeding_edge/test/mjsunit/const-eval-init.js Thu Jan 22
05:53:06 2009
@@ -0,0 +1,111 @@
+// 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();
Modified: branches/bleeding_edge/test/mjsunit/const.js
==============================================================================
--- branches/bleeding_edge/test/mjsunit/const.js (original)
+++ branches/bleeding_edge/test/mjsunit/const.js Thu Jan 22 05:53:06 2009
@@ -52,17 +52,19 @@
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();
Added: branches/bleeding_edge/test/mjsunit/regress/regress-189.js
==============================================================================
--- (empty file)
+++ branches/bleeding_edge/test/mjsunit/regress/regress-189.js Thu Jan 22
05:53:06 2009
@@ -0,0 +1,36 @@
+// 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();
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
