Revision: 6843
Author: [email protected]
Date: Thu Feb 17 08:54:49 2011
Log: Revert change to const and global variable declarations. It causes
may WebKit layout test failures.
I will look into it tomorrow.
[email protected]
Review URL: http://codereview.chromium.org/6537021
http://code.google.com/p/v8/source/detail?r=6843
Deleted:
/branches/bleeding_edge/test/mjsunit/regress/regress-1170.js
Modified:
/branches/bleeding_edge/src/runtime.cc
=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-1170.js Thu Feb 17
08:30:15 2011
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright 2011 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.
-
-var setter_value = 0;
-
-__proto__.__defineSetter__("a", function(v) { setter_value = v; });
-eval("var a = 1");
-assertEquals(1, setter_value);
-assertFalse(hasOwnProperty("a"));
-
-__proto__.__defineSetter__("b", function(v) { setter_value = v; });
-eval("with({}) { eval('var b = 2') }");
-assertEquals(2, setter_value);
-assertFalse(hasOwnProperty("b"));
-
-__proto__.__defineSetter__("c", function(v) { assertTrue(false); });
-try {
- eval("const c = 23");
- assertTrue(false);
-} catch(e) {
- assertTrue(/TypeError/.test(e));
-}
-try {
- eval("with({}) { eval('const c = 23') }");
- assertTrue(false);
-} catch(e) {
- assertTrue(/TypeError/.test(e));
-}
=======================================
--- /branches/bleeding_edge/src/runtime.cc Thu Feb 17 08:30:15 2011
+++ /branches/bleeding_edge/src/runtime.cc Thu Feb 17 08:54:49 2011
@@ -1051,12 +1051,6 @@
// Fall-through and introduce the absent property by using
// SetProperty.
} else {
- // For const properties, we treat a callback with this name
- // even in the prototype as a conflicting declaration.
- if (is_const_property && (lookup.type() == CALLBACKS)) {
- return ThrowRedeclarationError("const", name);
- }
- // Otherwise, we check for locally conflicting declarations.
if (is_local && (is_read_only || is_const_property)) {
const char* type = (is_read_only) ? "const" : "var";
return ThrowRedeclarationError(type, name);
@@ -1082,20 +1076,30 @@
? static_cast<PropertyAttributes>(base | READ_ONLY)
: base;
- // There's a local property that we need to overwrite because
- // we're either declaring a function or there's an interceptor
- // that claims the property is absent.
- //
- // Check for conflicting re-declarations. We cannot have
- // conflicting types in case of intercepted properties because
- // they are absent.
- if (lookup.IsProperty() &&
- (lookup.type() != INTERCEPTOR) &&
- (lookup.IsReadOnly() || is_const_property)) {
- const char* type = (lookup.IsReadOnly()) ? "const" : "var";
- return ThrowRedeclarationError(type, name);
- }
- RETURN_IF_EMPTY_HANDLE(SetProperty(global, name, value, attributes));
+ if (lookup.IsProperty()) {
+ // There's a local property that we need to overwrite because
+ // we're either declaring a function or there's an interceptor
+ // that claims the property is absent.
+
+ // Check for conflicting re-declarations. We cannot have
+ // conflicting types in case of intercepted properties because
+ // they are absent.
+ if (lookup.type() != INTERCEPTOR &&
+ (lookup.IsReadOnly() || is_const_property)) {
+ const char* type = (lookup.IsReadOnly()) ? "const" : "var";
+ return ThrowRedeclarationError(type, name);
+ }
+ RETURN_IF_EMPTY_HANDLE(SetProperty(global, name, value, attributes));
+ } else {
+ // If a property with this name does not already exist on the
+ // global object add the property locally. We take special
+ // precautions to always add it as a local property even in case
+ // of callbacks in the prototype chain (this rules out using
+ // SetProperty). Also, we must use the handle-based version to
+ // avoid GC issues.
+ RETURN_IF_EMPTY_HANDLE(
+ SetLocalPropertyIgnoreAttributes(global, name, value,
attributes));
+ }
}
ASSERT(!Top::has_pending_exception());
@@ -1182,20 +1186,6 @@
ASSERT(!context_ext->HasLocalProperty(*name));
Handle<Object> value(Heap::undefined_value());
if (*initial_value != NULL) value = initial_value;
- // Declaring a const context slot is a conflicting declaration if
- // there is a callback with that name in a prototype. It is
- // allowed to introduce const variables in
- // JSContextExtensionObjects. They are treated specially in
- // SetProperty and no setters are invoked for those since they are
- // not real JSObjects.
- if (initial_value->IsTheHole() &&
- !context_ext->IsJSContextExtensionObject()) {
- LookupResult lookup;
- context_ext->Lookup(*name, &lookup);
- if (lookup.IsProperty() && (lookup.type() == CALLBACKS)) {
- return ThrowRedeclarationError("const", name);
- }
- }
RETURN_IF_EMPTY_HANDLE(SetProperty(context_ext, name, value, mode));
}
@@ -1222,7 +1212,11 @@
// there, there is a property with this name in the prototype chain.
// We follow Safari and Firefox behavior and only set the property
// locally if there is an explicit initialization value that we have
- // to assign to the property.
+ // to assign to the property. When adding the property we take
+ // special precautions to always add it as a local property even in
+ // case of callbacks in the prototype chain (this rules out using
+ // SetProperty). We have SetLocalPropertyIgnoreAttributes for
+ // this.
// Note that objects can have hidden prototypes, so we need to traverse
// the whole chain of hidden prototypes to do a 'local' lookup.
JSObject* real_holder = global;
@@ -1283,7 +1277,11 @@
}
global = Top::context()->global();
- if (assign) return global->SetProperty(*name, args[1], attributes);
+ if (assign) {
+ return global->SetLocalPropertyIgnoreAttributes(*name,
+ args[1],
+ attributes);
+ }
return Heap::undefined_value();
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
