From: ager@chromium.org Date: Thu, 17 Feb 2011 16:54:49 +0000 (+0000) Subject: Revert change to const and global variable declarations. It causes X-Git-Tag: upstream/4.7.83~20167 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=65addc516541b34c0ddc90e054e69682aa1a2741;p=platform%2Fupstream%2Fv8.git Revert change to const and global variable declarations. It causes may WebKit layout test failures. I will look into it tomorrow. TBR=kmillikin@chromium.org Review URL: http://codereview.chromium.org/6537021 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6843 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/runtime.cc b/src/runtime.cc index a6670bf..48ff69f 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -1051,12 +1051,6 @@ static MaybeObject* Runtime_DeclareGlobals(Arguments args) { // 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 MaybeObject* Runtime_DeclareGlobals(Arguments args) { ? static_cast(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); + 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)); } - RETURN_IF_EMPTY_HANDLE(SetProperty(global, name, value, attributes)); } ASSERT(!Top::has_pending_exception()); @@ -1182,20 +1186,6 @@ static MaybeObject* Runtime_DeclareContextSlot(Arguments args) { ASSERT(!context_ext->HasLocalProperty(*name)); Handle 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 @@ static MaybeObject* Runtime_InitializeVarGlobal(Arguments args) { // 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 @@ static MaybeObject* Runtime_InitializeVarGlobal(Arguments args) { } 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(); } diff --git a/test/mjsunit/regress/regress-1170.js b/test/mjsunit/regress/regress-1170.js deleted file mode 100644 index 090e22e..0000000 --- a/test/mjsunit/regress/regress-1170.js +++ /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)); -}