diff --git a/src/runtime.cc b/src/runtime.cc index e62c68afc2..37f5b85ae6 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -1211,46 +1211,17 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareGlobals) { LookupResult lookup; global->Lookup(*name, &lookup); if (lookup.IsProperty()) { - // Determine if the property is local by comparing the holder - // against the global object. The information will be used to - // avoid throwing re-declaration errors when declaring - // variables or constants that exist in the prototype chain. - bool is_local = (*global == lookup.holder()); - // Get the property attributes and determine if the property is - // read-only. - PropertyAttributes attributes = global->GetPropertyAttribute(*name); - bool is_read_only = (attributes & READ_ONLY) != 0; - if (lookup.type() == INTERCEPTOR) { - // If the interceptor says the property is there, we - // just return undefined without overwriting the property. - // Otherwise, we continue to setting the property. - if (attributes != ABSENT) { - // Check if the existing property conflicts with regards to const. - if (is_local && (is_read_only || is_const_property)) { - const char* type = (is_read_only) ? "const" : "var"; - return ThrowRedeclarationError(isolate, type, name); - }; - // The property already exists without conflicting: Go to - // the next declaration. - continue; - } - // 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(isolate, "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(isolate, type, name); - } - // The property already exists without conflicting: Go to - // the next declaration. + // We found an existing property. Unless it was an interceptor + // that claims the property is absent, skip this declaration. + if (lookup.type() != INTERCEPTOR) { continue; } + PropertyAttributes attributes = global->GetPropertyAttribute(*name); + if (attributes != ABSENT) { + continue; + } + // Fall-through and introduce the absent property by using + // SetProperty. } } else { is_function_declaration = true; @@ -1267,20 +1238,6 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareGlobals) { LookupResult lookup; global->LocalLookup(*name, &lookup); - // 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(isolate, type, name); - } - // Compute the property attributes. According to ECMA-262, section // 13, page 71, the property must be read-only and // non-deletable. However, neither SpiderMonkey nor KJS creates the @@ -1465,64 +1422,32 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_InitializeVarGlobal) { // to assign to the property. // 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; + Object* object = global; LookupResult lookup; - while (true) { - real_holder->LocalLookup(*name, &lookup); - if (lookup.IsProperty()) { - // Determine if this is a redeclaration of something read-only. - if (lookup.IsReadOnly()) { - // If we found readonly property on one of hidden prototypes, - // just shadow it. - if (real_holder != isolate->context()->global()) break; - return ThrowRedeclarationError(isolate, "const", name); - } - - // Determine if this is a redeclaration of an intercepted read-only - // property and figure out if the property exists at all. - bool found = true; - PropertyType type = lookup.type(); - if (type == INTERCEPTOR) { - HandleScope handle_scope(isolate); - Handle holder(real_holder); - PropertyAttributes intercepted = holder->GetPropertyAttribute(*name); - real_holder = *holder; - if (intercepted == ABSENT) { - // The interceptor claims the property isn't there. We need to - // make sure to introduce it. - found = false; - } else if ((intercepted & READ_ONLY) != 0) { - // The property is present, but read-only. Since we're trying to - // overwrite it with a variable declaration we must throw a - // re-declaration error. However if we found readonly property - // on one of hidden prototypes, just shadow it. - if (real_holder != isolate->context()->global()) break; - return ThrowRedeclarationError(isolate, "const", name); + while (object->IsJSObject() && + JSObject::cast(object)->map()->is_hidden_prototype()) { + JSObject* raw_holder = JSObject::cast(object); + raw_holder->LocalLookup(*name, &lookup); + if (lookup.IsProperty() && lookup.type() == INTERCEPTOR) { + HandleScope handle_scope(isolate); + Handle holder(raw_holder); + PropertyAttributes intercepted = holder->GetPropertyAttribute(*name); + // Update the raw pointer in case it's changed due to GC. + raw_holder = *holder; + if (intercepted != ABSENT && (intercepted & READ_ONLY) == 0) { + // Found an interceptor that's not read only. + if (assign) { + return raw_holder->SetProperty( + &lookup, *name, args[2], attributes, strict_mode); + } else { + return isolate->heap()->undefined_value(); } } - - if (found && !assign) { - // The global property is there and we're not assigning any value - // to it. Just return. - return isolate->heap()->undefined_value(); - } - - // Assign the value (or undefined) to the property. - Object* value = (assign) ? args[2] : isolate->heap()->undefined_value(); - return real_holder->SetProperty( - &lookup, *name, value, attributes, strict_mode); } - - Object* proto = real_holder->GetPrototype(); - if (!proto->IsJSObject()) - break; - - if (!JSObject::cast(proto)->map()->is_hidden_prototype()) - break; - - real_holder = JSObject::cast(proto); + object = raw_holder->GetPrototype(); } + // Reload global in case the loop above performed a GC. global = isolate->context()->global(); if (assign) { return global->SetProperty(*name, args[2], attributes, strict_mode); @@ -1560,25 +1485,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_InitializeConstGlobal) { attributes); } - // Determine if this is a redeclaration of something not - // read-only. In case the result is hidden behind an interceptor we - // need to ask it for the property attributes. if (!lookup.IsReadOnly()) { - if (lookup.type() != INTERCEPTOR) { - return ThrowRedeclarationError(isolate, "var", name); - } - - PropertyAttributes intercepted = global->GetPropertyAttribute(*name); - - // Throw re-declaration error if the intercepted property is present - // but not read-only. - if (intercepted != ABSENT && (intercepted & READ_ONLY) == 0) { - return ThrowRedeclarationError(isolate, "var", name); - } - // Restore global object from context (in case of GC) and continue - // with setting the value because the property is either absent or - // read-only. We also have to do redo the lookup. + // with setting the value. HandleScope handle_scope(isolate); Handle global(isolate->context()->global()); @@ -1595,19 +1504,20 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_InitializeConstGlobal) { return *value; } - // Set the value, but only we're assigning the initial value to a + // Set the value, but only if we're assigning the initial value to a // constant. For now, we determine this by checking if the // current value is the hole. - // Strict mode handling not needed (const disallowed in strict mode). + // Strict mode handling not needed (const is disallowed in strict mode). PropertyType type = lookup.type(); if (type == FIELD) { FixedArray* properties = global->properties(); int index = lookup.GetFieldIndex(); - if (properties->get(index)->IsTheHole()) { + if (properties->get(index)->IsTheHole() || !lookup.IsReadOnly()) { properties->set(index, *value); } } else if (type == NORMAL) { - if (global->GetNormalizedProperty(&lookup)->IsTheHole()) { + if (global->GetNormalizedProperty(&lookup)->IsTheHole() || + !lookup.IsReadOnly()) { global->SetNormalizedProperty(&lookup, *value); } } else { diff --git a/src/v8natives.js b/src/v8natives.js index 588bdb21bb..0d9cb74cbc 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -193,13 +193,14 @@ function GlobalEval(x) { function SetUpGlobal() { %CheckIsBootstrapping(); // ECMA 262 - 15.1.1.1. - %SetProperty(global, "NaN", $NaN, DONT_ENUM | DONT_DELETE); + %SetProperty(global, "NaN", $NaN, DONT_ENUM | DONT_DELETE | READ_ONLY); // ECMA-262 - 15.1.1.2. - %SetProperty(global, "Infinity", 1/0, DONT_ENUM | DONT_DELETE); + %SetProperty(global, "Infinity", 1/0, DONT_ENUM | DONT_DELETE | READ_ONLY); // ECMA-262 - 15.1.1.3. - %SetProperty(global, "undefined", void 0, DONT_ENUM | DONT_DELETE); + %SetProperty(global, "undefined", void 0, + DONT_ENUM | DONT_DELETE | READ_ONLY); // Set up non-enumerable function on the global object. InstallFunctions(global, DONT_ENUM, $Array( diff --git a/test/cctest/test-decls.cc b/test/cctest/test-decls.cc index 619839185e..aa733c70bc 100644 --- a/test/cctest/test-decls.cc +++ b/test/cctest/test-decls.cc @@ -232,7 +232,7 @@ TEST(Unknown) { context.Check("const x; x", 1, // access 2, // declaration + initialization - 2, // declaration + initialization + 1, // declaration EXPECT_RESULT, Undefined()); } @@ -240,7 +240,7 @@ TEST(Unknown) { context.Check("const x = 0; x", 1, // access 2, // declaration + initialization - 2, // declaration + initialization + 1, // declaration EXPECT_RESULT, Undefined()); // SB 0 - BUG 1213579 } } @@ -285,18 +285,18 @@ TEST(Present) { { PresentPropertyContext context; context.Check("const x; x", - 0, - 0, + 1, // access + 1, // initialization 1, // (re-)declaration - EXPECT_EXCEPTION); // x has already been declared! + EXPECT_RESULT, Undefined()); } { PresentPropertyContext context; context.Check("const x = 0; x", - 0, - 0, + 1, // access + 1, // initialization 1, // (re-)declaration - EXPECT_EXCEPTION); // x has already been declared! + EXPECT_RESULT, Number::New(0)); } } @@ -341,7 +341,7 @@ TEST(Absent) { context.Check("const x; x", 1, // access 2, // declaration + initialization - 2, // declaration + initializetion + 1, // declaration EXPECT_RESULT, Undefined()); } @@ -349,7 +349,7 @@ TEST(Absent) { context.Check("const x = 0; x", 1, // access 2, // declaration + initialization - 2, // declaration + initialization + 1, // declaration EXPECT_RESULT, Undefined()); // SB 0 - BUG 1213579 } @@ -429,18 +429,20 @@ TEST(Appearing) { { AppearingPropertyContext context; context.Check("const x; x", - 0, - 1, // declaration + 1, // access 2, // declaration + initialization - EXPECT_EXCEPTION); // x has already been declared! + 1, // declaration + EXPECT_RESULT, Undefined()); } { AppearingPropertyContext context; context.Check("const x = 0; x", - 0, - 1, // declaration + 1, // access 2, // declaration + initialization - EXPECT_EXCEPTION); // x has already been declared! + 1, // declaration + EXPECT_RESULT, Undefined()); + // Result is undefined because declaration succeeded but + // initialization to 0 failed (due to context behavior). } } @@ -496,9 +498,9 @@ TEST(Reappearing) { { ReappearingPropertyContext context; context.Check("const x; var x = 0", 0, - 2, // var declaration + const initialization - 4, // 2 x declaration + 2 x initialization - EXPECT_EXCEPTION); // x has already been declared! + 3, // const declaration+initialization, var initialization + 3, // 2 x declaration + var initialization + EXPECT_RESULT, Undefined()); } } diff --git a/test/es5conform/es5conform.status b/test/es5conform/es5conform.status index d095a2471d..e35acfcb13 100644 --- a/test/es5conform/es5conform.status +++ b/test/es5conform/es5conform.status @@ -41,16 +41,6 @@ chapter10/10.4/10.4.2/10.4.2-2-c-1: FAIL_OK # We are compatible with Safari and Firefox. chapter11/11.1/11.1.5: UNIMPLEMENTED -# We do not have a global object called 'global' as required by tests. -chapter15/15.1: FAIL_OK - -# NaN is writable. We are compatible with JSC. -chapter15/15.2/15.2.3/15.2.3.3/15.2.3.3-4-178: FAIL_OK -# Infinity is writable. We are compatible with JSC. -chapter15/15.2/15.2.3/15.2.3.3/15.2.3.3-4-179: FAIL_OK -# undefined is writable. We are compatible with JSC. -chapter15/15.2/15.2.3/15.2.3.3/15.2.3.3-4-180: FAIL_OK - # Our Function object has an "arguments" property which is used as a # non-property in the test. chapter15/15.2/15.2.3/15.2.3.3/15.2.3.3-4-183: FAIL_OK diff --git a/test/mjsunit/const-redecl.js b/test/mjsunit/const-redecl.js index 945970891b..c0b97e6ced 100644 --- a/test/mjsunit/const-redecl.js +++ b/test/mjsunit/const-redecl.js @@ -98,7 +98,8 @@ function TestAll(expected,s,opt_e) { var msg = s; if (opt_e) { e = opt_e; msg += "; " + opt_e; } assertEquals(expected, TestLocal(s,e), "local:'" + msg + "'"); - assertEquals(expected, TestGlobal(s,e), "global:'" + msg + "'"); + // Redeclarations of global consts do not throw, they are silently ignored. + assertEquals(42, TestGlobal(s, 42), "global:'" + msg + "'"); assertEquals(expected, TestContext(s,e), "context:'" + msg + "'"); } @@ -218,3 +219,62 @@ TestAll(0, "var a,b,c,d,e,f,g,h; " + loop, "x"); // Test that const inside with behaves correctly. TestAll(87, "with ({x:42}) { const x = 87; }", "x"); TestAll(undefined, "with ({x:42}) { const x; }", "x"); + + +// Additional tests for how various combinations of re-declarations affect +// the values of the var/const in question. +try { + eval("var undefined;"); +} catch (ex) { + assertUnreachable("undefined (1) has thrown"); +} + +var original_undef = undefined; +var undefined = 1; // Should be silently ignored. +assertEquals(original_undef, undefined, "undefined got overwritten"); +undefined = original_undef; + +var a; const a; const a = 1; +assertEquals(1, a, "a has wrong value"); +a = 2; +assertEquals(2, a, "a should be writable"); + +var b = 1; const b = 2; +assertEquals(2, b, "b has wrong value"); + +var c = 1; const c = 2; const c = 3; +assertEquals(3, c, "c has wrong value"); + +const d = 1; const d = 2; +assertEquals(1, d, "d has wrong value"); + +const e = 1; var e = 2; +assertEquals(1, e, "e has wrong value"); + +const f = 1; const f; +assertEquals(1, f, "f has wrong value"); + +var g; const g = 1; +assertEquals(1, g, "g has wrong value"); +g = 2; +assertEquals(2, g, "g should be writable"); + +const h; var h = 1; +assertEquals(undefined,h, "h has wrong value"); + +eval("Object.defineProperty(this, 'i', { writable: true });" + + "const i = 7;" + + "assertEquals(7, i, \"i has wrong value\");"); + +var global = this; +assertThrows(function() { + Object.defineProperty(global, 'j', { writable: true }) +}, TypeError); +const j = 2; // This is what makes the function above throw, because the +// const declaration gets hoisted and makes the property non-configurable. +assertEquals(2, j, "j has wrong value"); + +var k = 1; const k; +// You could argue about the expected result here. For now, the winning +// argument is that "const k;" is equivalent to "const k = undefined;". +assertEquals(undefined, k, "k has wrong value"); diff --git a/test/mjsunit/global-const-var-conflicts.js b/test/mjsunit/global-const-var-conflicts.js index d38d0ee813..2fca96f9f8 100644 --- a/test/mjsunit/global-const-var-conflicts.js +++ b/test/mjsunit/global-const-var-conflicts.js @@ -26,7 +26,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. // Check that dynamically introducing conflicting consts/vars -// leads to exceptions. +// is silently ignored (and does not lead to exceptions). var caught = 0; @@ -46,12 +46,12 @@ eval("var c"); try { eval("const c"); } catch (e) { caught++; assertTrue(e instanceof TypeError); } assertTrue(typeof c == 'undefined'); try { eval("const c = 1"); } catch (e) { caught++; assertTrue(e instanceof TypeError); } -assertTrue(typeof c == 'undefined'); +assertEquals(1, c); eval("var d = 0"); try { eval("const d"); } catch (e) { caught++; assertTrue(e instanceof TypeError); } -assertEquals(0, d); +assertEquals(undefined, d); try { eval("const d = 1"); } catch (e) { caught++; assertTrue(e instanceof TypeError); } -assertEquals(0, d); +assertEquals(1, d); -assertEquals(8, caught); +assertEquals(0, caught); diff --git a/test/mjsunit/regress/regress-1170.js b/test/mjsunit/regress/regress-1170.js index 95684c5418..66ed9f29e2 100644 --- a/test/mjsunit/regress/regress-1170.js +++ b/test/mjsunit/regress/regress-1170.js @@ -49,7 +49,7 @@ try { exception = true; assertTrue(/TypeError/.test(e)); } -assertTrue(exception); +assertFalse(exception); exception = false; try { diff --git a/test/mjsunit/regress/regress-1213575.js b/test/mjsunit/regress/regress-1213575.js index 9d82064e47..f3a11dbaab 100644 --- a/test/mjsunit/regress/regress-1213575.js +++ b/test/mjsunit/regress/regress-1213575.js @@ -25,17 +25,16 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -// Make sure that a const definition always -// conflicts with a defined setter. This avoid -// trying to pass 'the hole' to the setter. +// Make sure that a const definition does not try +// to pass 'the hole' to a defined setter. -this.__defineSetter__('x', function(value) { assertTrue(false); }); +this.__defineSetter__('x', function(value) { assertTrue(value === 1); }); var caught = false; try { - eval('const x'); + eval('const x = 1'); } catch(e) { assertTrue(e instanceof TypeError); caught = true; } -assertTrue(caught); +assertFalse(caught); diff --git a/test/mjsunit/undeletable-functions.js b/test/mjsunit/undeletable-functions.js index 04fd06068d..bbb798f351 100644 --- a/test/mjsunit/undeletable-functions.js +++ b/test/mjsunit/undeletable-functions.js @@ -76,6 +76,8 @@ array = [ "execScript"]; CheckEcmaSemantics(this, array, "Global"); CheckReadOnlyAttr(this, "Infinity"); +CheckReadOnlyAttr(this, "NaN"); +CheckReadOnlyAttr(this, "undefined"); array = ["exec", "test", "toString", "compile"]; CheckEcmaSemantics(RegExp.prototype, array, "RegExp prototype"); @@ -189,7 +191,7 @@ function CheckReadOnlyAttr(type, prop) { assertFalse(deleted, "delete operator returned true: " + prop); assertTrue(type.hasOwnProperty(prop), "not there after delete: " + prop); type[prop] = "foo"; - assertEquals("foo", type[prop], "overwritable: " + prop); + assertEquals(old, type[prop], "overwritable: " + prop); } print("OK"); diff --git a/test/sputnik/sputnik.status b/test/sputnik/sputnik.status index 868509d7c5..2373fb53c4 100644 --- a/test/sputnik/sputnik.status +++ b/test/sputnik/sputnik.status @@ -176,6 +176,14 @@ S15.5.4.13_A1_T3: FAIL_OK S15.5.4.14_A1_T3: FAIL_OK S15.5.4.15_A1_T3: FAIL_OK +# NaN, Infinity and undefined are read-only according to ES5. +S15.1.1.1_A2_T1: FAIL_OK # NaN +S15.1.1.1_A2_T2: FAIL_OK # NaN +S15.1.1.2_A2_T1: FAIL_OK # Infinity +# S15.1.1.2_A2_T2 would fail if it weren't bogus in r97. sputnik bug #45. +S15.1.1.3_A2_T1: FAIL_OK # undefined +S15.1.1.3_A2_T2: FAIL_OK # undefined + ##################### SKIPPED TESTS ##################### # These tests take a looong time to run in debug mode.