From f6cd009efd7a032907b11265e74102b0d45207a1 Mon Sep 17 00:00:00 2001 From: arv Date: Wed, 4 Mar 2015 08:56:55 -0800 Subject: [PATCH] ES6: Make function name configurable Function name property is now standardized in ES6. It was a Mozilla proprietary extension before. With ES6, the property was made configurable, so that it can be used instead of another proprietary property, displayName. This is a revert of revert https://chromium.googlesource.com/v8/v8.git/+/c791d8411225aa95518c880defd9a5b1ea243afc. Last time this broke a Chrome browser test which has since been updated: https://chromium.googlesource.com/chromium/src/+/5f75a3be4c9334aba627d6c5eb884adf965e1d78 BUG=v8:3333 LOG=N R=mstarzinger@chromium.org,verwaest@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/977003004 Cr-Commit-Position: refs/heads/master@{#26996} --- src/accessors.cc | 36 +++++++- src/bootstrapper.cc | 34 ++++---- test/mjsunit/es6/function-name-configurable.js | 115 +++++++++++++++++++++++++ test/mjsunit/es7/object-observe.js | 28 +++++- test/mjsunit/regress/regress-1530.js | 5 +- test/mjsunit/regress/regress-270142.js | 2 +- 6 files changed, 199 insertions(+), 21 deletions(-) create mode 100644 test/mjsunit/es6/function-name-configurable.js diff --git a/src/accessors.cc b/src/accessors.cc index 052108a..51fdee6 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -1131,12 +1131,44 @@ void Accessors::FunctionNameGetter( } +MUST_USE_RESULT static MaybeHandle SetFunctionName( + Isolate* isolate, Handle function, Handle value) { + Handle old_value; + bool is_observed = function->map()->is_observed(); + if (is_observed) { + old_value = handle(function->shared()->name(), isolate); + } + + Handle name = isolate->factory()->name_string(); + LookupIterator it(function, name); + CHECK_EQ(LookupIterator::ACCESSOR, it.state()); + DCHECK(it.HolderIsReceiverOrHiddenPrototype()); + it.ReconfigureDataProperty(value, it.property_details().attributes()); + value = it.WriteDataValue(value); + + if (is_observed && !old_value->SameValue(*value)) { + return JSObject::EnqueueChangeRecord(function, "update", name, old_value); + } + + return value; +} + + void Accessors::FunctionNameSetter( v8::Local name, v8::Local val, const v8::PropertyCallbackInfo& info) { - // Function name is non writable, non configurable. - UNREACHABLE(); + i::Isolate* isolate = reinterpret_cast(info.GetIsolate()); + HandleScope scope(isolate); + Handle value = Utils::OpenHandle(*val); + + if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) return; + + Handle object = + Handle::cast(Utils::OpenHandle(*info.Holder())); + if (SetFunctionName(isolate, object, value).is_null()) { + isolate->OptionalRescheduleException(false); + } } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 3360026..2f73e08 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -382,45 +382,47 @@ void Genesis::SetFunctionInstanceDescriptor( int size = IsFunctionModeWithPrototype(function_mode) ? 5 : 4; Map::EnsureDescriptorSlack(map, size); - PropertyAttributes attribs = static_cast( - DONT_ENUM | DONT_DELETE | READ_ONLY); + PropertyAttributes ro_attribs = + static_cast(DONT_ENUM | DONT_DELETE | READ_ONLY); + PropertyAttributes roc_attribs = + static_cast(DONT_ENUM | READ_ONLY); Handle length = - Accessors::FunctionLengthInfo(isolate(), attribs); + Accessors::FunctionLengthInfo(isolate(), ro_attribs); { // Add length. AccessorConstantDescriptor d(Handle(Name::cast(length->name())), - length, attribs); + length, ro_attribs); map->AppendDescriptor(&d); } Handle name = - Accessors::FunctionNameInfo(isolate(), attribs); + Accessors::FunctionNameInfo(isolate(), ro_attribs); { // Add name. AccessorConstantDescriptor d(Handle(Name::cast(name->name())), name, - attribs); + roc_attribs); map->AppendDescriptor(&d); } Handle args = - Accessors::FunctionArgumentsInfo(isolate(), attribs); + Accessors::FunctionArgumentsInfo(isolate(), ro_attribs); { // Add arguments. AccessorConstantDescriptor d(Handle(Name::cast(args->name())), args, - attribs); + ro_attribs); map->AppendDescriptor(&d); } Handle caller = - Accessors::FunctionCallerInfo(isolate(), attribs); + Accessors::FunctionCallerInfo(isolate(), ro_attribs); { // Add caller. AccessorConstantDescriptor d(Handle(Name::cast(caller->name())), - caller, attribs); + caller, ro_attribs); map->AppendDescriptor(&d); } if (IsFunctionModeWithPrototype(function_mode)) { if (function_mode == FUNCTION_WITH_WRITEABLE_PROTOTYPE) { - attribs = static_cast(attribs & ~READ_ONLY); + ro_attribs = static_cast(ro_attribs & ~READ_ONLY); } Handle prototype = - Accessors::FunctionPrototypeInfo(isolate(), attribs); + Accessors::FunctionPrototypeInfo(isolate(), ro_attribs); AccessorConstantDescriptor d(Handle(Name::cast(prototype->name())), - prototype, attribs); + prototype, ro_attribs); map->AppendDescriptor(&d); } } @@ -540,6 +542,8 @@ void Genesis::SetStrictFunctionInstanceDescriptor( static_cast(DONT_ENUM | DONT_DELETE); PropertyAttributes ro_attribs = static_cast(DONT_ENUM | DONT_DELETE | READ_ONLY); + PropertyAttributes roc_attribs = + static_cast(DONT_ENUM | READ_ONLY); // Add length. if (function_mode == BOUND_FUNCTION) { @@ -557,10 +561,10 @@ void Genesis::SetStrictFunctionInstanceDescriptor( map->AppendDescriptor(&d); } Handle name = - Accessors::FunctionNameInfo(isolate(), ro_attribs); + Accessors::FunctionNameInfo(isolate(), roc_attribs); { // Add name. AccessorConstantDescriptor d(Handle(Name::cast(name->name())), name, - ro_attribs); + roc_attribs); map->AppendDescriptor(&d); } { // Add arguments. diff --git a/test/mjsunit/es6/function-name-configurable.js b/test/mjsunit/es6/function-name-configurable.js new file mode 100644 index 0000000..f0ff406 --- /dev/null +++ b/test/mjsunit/es6/function-name-configurable.js @@ -0,0 +1,115 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +function getStrictF() { + 'use strict'; + return function f() {}; +} + + +function getSloppyF() { + return function f() {}; +} + + +function test(testFunction) { + testFunction(getStrictF()); + testFunction(getSloppyF()); +} + + +function testDescriptor(f) { + var descr = Object.getOwnPropertyDescriptor(f, 'name'); + assertTrue(descr.configurable); + assertFalse(descr.enumerable); + assertEquals('f', descr.value); + assertFalse(descr.writable); +} +test(testDescriptor); + + +function testSet(f) { + f.name = 'g'; + assertEquals('f', f.name); +} +test(testSet); + + +function testSetStrict(f) { + 'use strict'; + assertThrows(function() { + f.name = 'g'; + }, TypeError); +} +test(testSetStrict); + + +function testReconfigureAsDataProperty(f) { + Object.defineProperty(f, 'name', { + value: 'g', + }); + assertEquals('g', f.name); + Object.defineProperty(f, 'name', { + writable: true + }); + f.name = 'h'; + assertEquals('h', f.name); + + f.name = 42; + assertEquals(42, f.name); +} +test(testReconfigureAsDataProperty); + + +function testReconfigureAsAccessorProperty(f) { + var name = 'g'; + Object.defineProperty(f, 'name', { + get: function() { return name; }, + set: function(v) { name = v; } + }); + assertEquals('g', f.name); + f.name = 'h'; + assertEquals('h', f.name); +} +test(testReconfigureAsAccessorProperty); + + +function testFunctionToString(f) { + Object.defineProperty(f, 'name', { + value: {toString: function() { assertUnreachable(); }}, + }); + assertEquals('function f() {}', f.toString()); +} +test(testFunctionToString); + + +(function testSetOnInstance() { + // This needs to come before testDelete below + assertTrue(Function.prototype.hasOwnProperty('name')); + + function f() {} + delete f.name; + assertEquals('Empty', f.name); + + f.name = 42; + assertEquals('Empty', f.name); // non writable prototype property. + assertFalse(f.hasOwnProperty('name')); + + Object.defineProperty(Function.prototype, 'name', {writable: true}); + + f.name = 123; + assertTrue(f.hasOwnProperty('name')); + assertEquals(123, f.name); +})(); + + +(function testDelete() { + function f() {} + assertTrue(delete f.name); + assertFalse(f.hasOwnProperty('name')); + assertEquals('Empty', f.name); + + assertTrue(delete Function.prototype.name); + assertEquals(undefined, f.name); +})(); diff --git a/test/mjsunit/es7/object-observe.js b/test/mjsunit/es7/object-observe.js index 5af205e..99fa38a 100644 --- a/test/mjsunit/es7/object-observe.js +++ b/test/mjsunit/es7/object-observe.js @@ -1142,7 +1142,8 @@ var properties = ["a", "1", 1, "length", "setPrototype", "name", "caller"]; function blacklisted(obj, prop) { return (obj instanceof Int32Array && prop == 1) || (obj instanceof Int32Array && prop === "length") || - (obj instanceof ArrayBuffer && prop == 1) + (obj instanceof ArrayBuffer && prop == 1) || + (obj instanceof Function && prop === "name"); // Has its own test. } for (var i in objects) for (var j in properties) { @@ -1798,3 +1799,28 @@ for (var b1 = 0; b1 < 2; ++b1) for (var n = 0; n < 3; ++n) for (var i in mutationByIncr) TestFastElementsLength(mutationByIncr[i], b1 != 0, b2 != 0, 7*n, 7*n+1); + + +(function TestFunctionName() { + reset(); + + function fun() {} + Object.observe(fun, observer.callback); + fun.name = 'x'; // No change. Not writable. + Object.defineProperty(fun, 'name', {value: 'a'}); + Object.defineProperty(fun, 'name', {writable: true}); + fun.name = 'b'; + delete fun.name; + fun.name = 'x'; // No change. Function.prototype.name is non writable + Object.defineProperty(Function.prototype, 'name', {writable: true}); + fun.name = 'c'; + fun.name = 'c'; // Same, no update. + Object.deliverChangeRecords(observer.callback); + observer.assertCallbackRecords([ + { object: fun, type: 'update', name: 'name', oldValue: 'fun' }, + { object: fun, type: 'reconfigure', name: 'name'}, + { object: fun, type: 'update', name: 'name', oldValue: 'a' }, + { object: fun, type: 'delete', name: 'name', oldValue: 'b' }, + { object: fun, type: 'add', name: 'name' }, + ]); +})(); diff --git a/test/mjsunit/regress/regress-1530.js b/test/mjsunit/regress/regress-1530.js index 20d1f26..b0daa2c 100644 --- a/test/mjsunit/regress/regress-1530.js +++ b/test/mjsunit/regress/regress-1530.js @@ -80,8 +80,9 @@ assertFalse(Object.getOwnPropertyDescriptor(f, 'prototype').writable); assertThrows("'use strict'; f.prototype = {}"); assertThrows("Object.defineProperty(f, 'prototype', { value: {} })"); -// Verify that non-writability of other properties is respected. -assertThrows("Object.defineProperty(f, 'name', { value: {} })"); +// Verify that non-configurability of other properties is respected, but +// non-writability is ignored by Object.defineProperty(). +Object.defineProperty(f, 'name', { value: {} }); assertThrows("Object.defineProperty(f, 'length', { value: {} })"); assertThrows("Object.defineProperty(f, 'caller', { value: {} })"); assertThrows("Object.defineProperty(f, 'arguments', { value: {} })"); diff --git a/test/mjsunit/regress/regress-270142.js b/test/mjsunit/regress/regress-270142.js index 6e0865c..63f4d14 100644 --- a/test/mjsunit/regress/regress-270142.js +++ b/test/mjsunit/regress/regress-270142.js @@ -39,7 +39,7 @@ function g(x) { function checkNameDescriptor(f) { var descriptor = Object.getOwnPropertyDescriptor(f, "name"); - assertFalse(descriptor.configurable); + assertTrue(descriptor.configurable); assertFalse(descriptor.enumerable); assertFalse(descriptor.writable); } -- 2.7.4