Revert of ES6: Make function name configurable (patchset #10 id:220001 of...

Revert of ES6: Make function name configurable (patchset #10 id:220001 of https://codereview.chromium.org/960343002/)

Reason for revert:
Breaks Chrome browser test that checks Object.name

[16509:16509:0228/030150:INFO:CONSOLE(43)] "Uncaught Error: Clobbered Object.name getter", source: http://www.chromium.org:33611/assertions.js (43)

http://build.chromium.org/p/client.v8/builders/Linux%20Tests%20%28dbg%29%281%29/builds/2328/steps/browser_tests/logs/stdio

Original issue's description:
> ES6: Make function name configurable
>
> This is partially based on r21609 but that CL was incomplete.
>
> Function name is still non writable so one has to use defineProperty
> to change the actual value.
>
> BUG=v8:3333
> LOG=N
> R=adamk, mstarzinger@chromium.org
>
> Committed: https://crrev.com/f7790f7670c8d859455a98fcb90ff1b66af1eca7
> Cr-Commit-Position: refs/heads/master@{#26924}

TBR=adamk@chromium.org,mstarzinger@chromium.org,verwaest@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:3333

Review URL: https://codereview.chromium.org/969683002

Cr-Commit-Position: refs/heads/master@{#26933}

src/accessors.cc

@@ -1131,44 +1131,12 @@ void Accessors::FunctionNameGetter(
 }
 
 
-MUST_USE_RESULT static MaybeHandle<Object> SetFunctionName(
-    Isolate* isolate, Handle<JSFunction> function, Handle<Object> value) {
-  Handle<Object> old_value;
-  bool is_observed = function->map()->is_observed();
-  if (is_observed) {
-    old_value = handle(function->shared()->name(), isolate);
-  }
-
-  Handle<Name> 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<v8::Name> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
-  HandleScope scope(isolate);
-  Handle<Object> value = Utils::OpenHandle(*val);
-
-  if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) return;
-
-  Handle<JSFunction> object =
-      Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
-  if (SetFunctionName(isolate, object, value).is_null()) {
-    isolate->OptionalRescheduleException(false);
-  }
+  // Function name is non writable, non configurable.
+  UNREACHABLE();
 }
 
 

src/bootstrapper.cc

@@ -382,47 +382,45 @@ void Genesis::SetFunctionInstanceDescriptor(
   int size = IsFunctionModeWithPrototype(function_mode) ? 5 : 4;
   Map::EnsureDescriptorSlack(map, size);
 
-  PropertyAttributes ro_attribs =
-      static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY);
-  PropertyAttributes roc_attribs =
-      static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY);
+  PropertyAttributes attribs = static_cast<PropertyAttributes>(
+      DONT_ENUM | DONT_DELETE | READ_ONLY);
 
   Handle<AccessorInfo> length =
-      Accessors::FunctionLengthInfo(isolate(), ro_attribs);
+      Accessors::FunctionLengthInfo(isolate(), attribs);
   {  // Add length.
     AccessorConstantDescriptor d(Handle<Name>(Name::cast(length->name())),
-                                 length, ro_attribs);
+                                 length, attribs);
     map->AppendDescriptor(&d);
   }
   Handle<AccessorInfo> name =
-      Accessors::FunctionNameInfo(isolate(), ro_attribs);
+      Accessors::FunctionNameInfo(isolate(), attribs);
   {  // Add name.
     AccessorConstantDescriptor d(Handle<Name>(Name::cast(name->name())), name,
-                                 roc_attribs);
+                                 attribs);
     map->AppendDescriptor(&d);
   }
   Handle<AccessorInfo> args =
-      Accessors::FunctionArgumentsInfo(isolate(), ro_attribs);
+      Accessors::FunctionArgumentsInfo(isolate(), attribs);
   {  // Add arguments.
     AccessorConstantDescriptor d(Handle<Name>(Name::cast(args->name())), args,
-                                 ro_attribs);
+                                 attribs);
     map->AppendDescriptor(&d);
   }
   Handle<AccessorInfo> caller =
-      Accessors::FunctionCallerInfo(isolate(), ro_attribs);
+      Accessors::FunctionCallerInfo(isolate(), attribs);
   {  // Add caller.
     AccessorConstantDescriptor d(Handle<Name>(Name::cast(caller->name())),
-                                 caller, ro_attribs);
+                                 caller, attribs);
     map->AppendDescriptor(&d);
   }
   if (IsFunctionModeWithPrototype(function_mode)) {
     if (function_mode == FUNCTION_WITH_WRITEABLE_PROTOTYPE) {
-      ro_attribs = static_cast<PropertyAttributes>(ro_attribs & ~READ_ONLY);
+      attribs = static_cast<PropertyAttributes>(attribs & ~READ_ONLY);
     }
     Handle<AccessorInfo> prototype =
-        Accessors::FunctionPrototypeInfo(isolate(), ro_attribs);
+        Accessors::FunctionPrototypeInfo(isolate(), attribs);
     AccessorConstantDescriptor d(Handle<Name>(Name::cast(prototype->name())),
-                                 prototype, ro_attribs);
+                                 prototype, attribs);
     map->AppendDescriptor(&d);
   }
 }
@@ -542,8 +540,6 @@ void Genesis::SetStrictFunctionInstanceDescriptor(
       static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE);
   PropertyAttributes ro_attribs =
       static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY);
-  PropertyAttributes roc_attribs =
-      static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY);
 
   // Add length.
   if (function_mode == BOUND_FUNCTION) {
@@ -561,10 +557,10 @@ void Genesis::SetStrictFunctionInstanceDescriptor(
     map->AppendDescriptor(&d);
   }
   Handle<AccessorInfo> name =
-      Accessors::FunctionNameInfo(isolate(), roc_attribs);
+      Accessors::FunctionNameInfo(isolate(), ro_attribs);
   {  // Add name.
     AccessorConstantDescriptor d(Handle<Name>(Name::cast(name->name())), name,
-                                 roc_attribs);
+                                 ro_attribs);
     map->AppendDescriptor(&d);
   }
   {  // Add arguments.

test/mjsunit/es6/function-name-configurable.js

-// 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);
-})();

test/mjsunit/es7/object-observe.js

@@ -1142,8 +1142,7 @@ 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 Function && prop === "name");  // Has its own test.
+         (obj instanceof ArrayBuffer && prop == 1)
 }
 
 for (var i in objects) for (var j in properties) {
@@ -1799,28 +1798,3 @@ 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' },
-  ]);
-})();

test/mjsunit/regress/regress-1530.js

@@ -80,9 +80,8 @@ assertFalse(Object.getOwnPropertyDescriptor(f, 'prototype').writable);
 assertThrows("'use strict'; f.prototype = {}");
 assertThrows("Object.defineProperty(f, 'prototype', { value: {} })");
 
-// Verify that non-configurability of other properties is respected, but
-// non-writability is ignored by Object.defineProperty().
-Object.defineProperty(f, 'name', { value: {} });
+// Verify that non-writability of other properties is respected.
+assertThrows("Object.defineProperty(f, 'name', { value: {} })");
 assertThrows("Object.defineProperty(f, 'length', { value: {} })");
 assertThrows("Object.defineProperty(f, 'caller', { value: {} })");
 assertThrows("Object.defineProperty(f, 'arguments', { value: {} })");

test/mjsunit/regress/regress-270142.js

@@ -39,7 +39,7 @@ function g(x) {
 
 function checkNameDescriptor(f) {
   var descriptor = Object.getOwnPropertyDescriptor(f, "name");
-  assertTrue(descriptor.configurable);
+  assertFalse(descriptor.configurable);
   assertFalse(descriptor.enumerable);
   assertFalse(descriptor.writable);
 }