Revision: 19490
Author:   [email protected]
Date:     Wed Feb 19 14:19:42 2014 UTC
Log:      Upgrade Symbol implementation to match current ES6 behavior.

Refresh the implementation of Symbols to catch up with what the
specification now mandates:

* The global Symbol() function manufactures new Symbol values,
  optionally with a string description attached.

* Invoking Symbol() as a constructor will now throw.

* ToString() over Symbol values still throws, and
  Object.prototype.toString() stringifies like before.

* A Symbol value is wrapped in a Symbol object either implicitly if
  it is the receiver, or explicitly done via Object(symbolValue) or
  (new Object(symbolValue).)

* The Symbol.prototype.toString() method no longer throws on Symbol
  wrapper objects (nor Symbol values.) Ditto for Symbol.prototype.valueOf().

* Symbol.prototype.toString() stringifies as "Symbol("<description>"),
  valueOf() returns the wrapper's Symbol value.

* ToPrimitive() over Symbol wrapper objects now throws.

Overall, this provides a stricter separation between Symbol values and
wrapper objects than before, and the explicit fetching out of the
description (nee name) via the "name" property is no longer supported
(by the spec nor the implementation.)

Adjusted existing Symbol test files to fit current, adding some extra
tests for new/changed behavior.

LOG=N
[email protected], [email protected], arv, rossberg
BUG=v8:3053

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

Patch from Sigbjorn Finne <[email protected]>.
http://code.google.com/p/v8/source/detail?r=19490

Modified:
 /branches/bleeding_edge/src/messages.js
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/runtime.cc
 /branches/bleeding_edge/src/runtime.h
 /branches/bleeding_edge/src/runtime.js
 /branches/bleeding_edge/src/symbol.js
 /branches/bleeding_edge/test/cctest/test-api.cc
 /branches/bleeding_edge/test/mjsunit/harmony/private.js
 /branches/bleeding_edge/test/mjsunit/harmony/symbols.js

=======================================
--- /branches/bleeding_edge/src/messages.js     Fri Feb 14 09:33:03 2014 UTC
+++ /branches/bleeding_edge/src/messages.js     Wed Feb 19 14:19:42 2014 UTC
@@ -176,7 +176,8 @@
cant_prevent_ext_external_array_elements: ["Cannot prevent extension of an object with external array elements"], redef_external_array_element: ["Cannot redefine a property of an object with external array elements"],
   harmony_const_assign:          ["Assignment to constant variable."],
-  symbol_to_string:              ["Conversion from symbol to string"],
+ symbol_to_string: ["Cannot convert a Symbol value to a string"], + symbol_to_primitive: ["Cannot convert a Symbol wrapper object to a primitive value"], invalid_module_path: ["Module does not export '", "%0", "', or export is not itself a module"],
   module_type_error:             ["Module '", "%0", "' used improperly"],
module_export_undefined: ["Export '", "%0", "' is not defined in module"]
=======================================
--- /branches/bleeding_edge/src/objects.cc      Wed Feb 19 14:03:48 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc      Wed Feb 19 14:19:42 2014 UTC
@@ -80,6 +80,8 @@
     return CreateJSValue(native_context->boolean_function(), this);
   } else if (IsString()) {
     return CreateJSValue(native_context->string_function(), this);
+  } else if (IsSymbol()) {
+    return CreateJSValue(native_context->symbol_function(), this);
   }
   ASSERT(IsJSObject());
   return this;
=======================================
--- /branches/bleeding_edge/src/runtime.cc      Wed Feb 19 13:55:25 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc      Wed Feb 19 14:19:42 2014 UTC
@@ -633,7 +633,14 @@
 }


-RUNTIME_FUNCTION(MaybeObject*, Runtime_SymbolName) {
+RUNTIME_FUNCTION(MaybeObject*, Runtime_NewSymbolWrapper) {
+  ASSERT(args.length() == 1);
+  CONVERT_ARG_CHECKED(Symbol, symbol, 0);
+  return symbol->ToObject(isolate);
+}
+
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_SymbolDescription) {
   SealHandleScope shs(isolate);
   ASSERT(args.length() == 1);
   CONVERT_ARG_CHECKED(Symbol, symbol, 0);
=======================================
--- /branches/bleeding_edge/src/runtime.h       Wed Feb 19 13:49:59 2014 UTC
+++ /branches/bleeding_edge/src/runtime.h       Wed Feb 19 14:19:42 2014 UTC
@@ -321,7 +321,8 @@
   /* Harmony symbols */ \
   F(CreateSymbol, 1, 1) \
   F(CreatePrivateSymbol, 1, 1) \
-  F(SymbolName, 1, 1) \
+  F(NewSymbolWrapper, 1, 1) \
+  F(SymbolDescription, 1, 1) \
   F(SymbolIsPrivate, 1, 1) \
   \
   /* Harmony proxies */ \
=======================================
--- /branches/bleeding_edge/src/runtime.js      Tue Jan  7 10:46:39 2014 UTC
+++ /branches/bleeding_edge/src/runtime.js      Wed Feb 19 14:19:42 2014 UTC
@@ -75,11 +75,8 @@
         y = %ToPrimitive(y, NO_HINT);
       }
     } else if (IS_SYMBOL(x)) {
-      while (true) {
-        if (IS_SYMBOL(y)) return %_ObjectEquals(x, y) ? 0 : 1;
-        if (!IS_SPEC_OBJECT(y)) return 1;  // not equal
-        y = %ToPrimitive(y, NO_HINT);
-      }
+      if (IS_SYMBOL(y)) return %_ObjectEquals(x, y) ? 0 : 1;
+      return 1; // not equal
     } else if (IS_BOOLEAN(x)) {
       if (IS_BOOLEAN(y)) return %_ObjectEquals(x, y) ? 0 : 1;
       if (IS_NULL_OR_UNDEFINED(y)) return 1;
@@ -97,6 +94,7 @@
         return %_ObjectEquals(x, y) ? 0 : 1;
       }
       if (IS_NULL_OR_UNDEFINED(y)) return 1;  // not equal
+      if (IS_SYMBOL(y)) return 1;  // not equal
       if (IS_BOOLEAN(y)) y = %ToNumber(y);
       x = %ToPrimitive(x, NO_HINT);
     }
@@ -501,7 +499,7 @@
   if (IS_STRING(x)) return x;
   // Normal behavior.
   if (!IS_SPEC_OBJECT(x)) return x;
-  if (IS_SYMBOL_WRAPPER(x)) return %_ValueOf(x);
+  if (IS_SYMBOL_WRAPPER(x)) throw MakeTypeError('symbol_to_primitive', []);
   if (hint == NO_HINT) hint = (IS_DATE(x)) ? STRING_HINT : NUMBER_HINT;
   return (hint == NUMBER_HINT) ? %DefaultNumber(x) : %DefaultString(x);
 }
@@ -548,6 +546,7 @@
   if (IS_NUMBER(x)) return %_NumberToString(x);
   if (IS_BOOLEAN(x)) return x ? 'true' : 'false';
   if (IS_UNDEFINED(x)) return 'undefined';
+  if (IS_SYMBOL(x)) throw %MakeTypeError('symbol_to_string', []);
   return (IS_NULL(x)) ? 'null' : %ToString(%DefaultString(x));
 }

@@ -555,6 +554,7 @@
   if (IS_NUMBER(x)) return %_NumberToString(x);
   if (IS_BOOLEAN(x)) return x ? 'true' : 'false';
   if (IS_UNDEFINED(x)) return 'undefined';
+  if (IS_SYMBOL(x)) throw %MakeTypeError('symbol_to_string', []);
   return (IS_NULL(x)) ? 'null' : %ToString(%DefaultString(x));
 }

@@ -568,9 +568,9 @@
 // ECMA-262, section 9.9, page 36.
 function ToObject(x) {
   if (IS_STRING(x)) return new $String(x);
-  if (IS_SYMBOL(x)) return new $Symbol(x);
   if (IS_NUMBER(x)) return new $Number(x);
   if (IS_BOOLEAN(x)) return new $Boolean(x);
+  if (IS_SYMBOL(x)) return %NewSymbolWrapper(x);
   if (IS_NULL_OR_UNDEFINED(x) && !IS_UNDETECTABLE(x)) {
     throw %MakeTypeError('undefined_or_null_to_object', []);
   }
=======================================
--- /branches/bleeding_edge/src/symbol.js       Thu Jan  9 15:57:30 2014 UTC
+++ /branches/bleeding_edge/src/symbol.js       Wed Feb 19 14:19:42 2014 UTC
@@ -36,34 +36,28 @@
 // -------------------------------------------------------------------

 function SymbolConstructor(x) {
-  var value =
-    IS_SYMBOL(x) ? x : %CreateSymbol(IS_UNDEFINED(x) ? x : ToString(x));
   if (%_IsConstructCall()) {
-    %_SetValueOf(this, value);
-  } else {
-    return value;
+    throw MakeTypeError('not_constructor', ["Symbol"]);
   }
+  // NOTE: Passing in a Symbol value will throw on ToString().
+  return %CreateSymbol(IS_UNDEFINED(x) ? x : ToString(x));
 }

-function SymbolGetName() {
-  var symbol = IS_SYMBOL_WRAPPER(this) ? %_ValueOf(this) : this;
-  if (!IS_SYMBOL(symbol)) {
+
+function SymbolToString() {
+  if (!(IS_SYMBOL(this) || IS_SYMBOL_WRAPPER(this))) {
     throw MakeTypeError(
-        'incompatible_method_receiver', ["Symbol.prototype.name", this]);
+      'incompatible_method_receiver', ["Symbol.prototype.toString", this]);
   }
-  return %SymbolName(symbol);
+  var description = %SymbolDescription(%_ValueOf(this));
+  return "Symbol(" + (IS_UNDEFINED(description) ? "" : description) + ")";
 }

-function SymbolToString() {
-  throw MakeTypeError('symbol_to_string');
-}

 function SymbolValueOf() {
-  // NOTE: Both Symbol objects and values can enter here as
-  // 'this'. This is not as dictated by ECMA-262.
-  if (!IS_SYMBOL(this) && !IS_SYMBOL_WRAPPER(this)) {
+  if (!(IS_SYMBOL(this) || IS_SYMBOL_WRAPPER(this))) {
     throw MakeTypeError(
- 'incompatible_method_receiver', ["Symbol.prototype.valueOf", this]);
+      'incompatible_method_receiver', ["Symbol.prototype.valueOf", this]);
   }
   return %_ValueOf(this);
 }
@@ -88,10 +82,9 @@
   %CheckIsBootstrapping();

   %SetCode($Symbol, SymbolConstructor);
-  %FunctionSetPrototype($Symbol, new $Symbol());
+  %FunctionSetPrototype($Symbol, new $Object());
+
   %SetProperty($Symbol.prototype, "constructor", $Symbol, DONT_ENUM);
-
-  InstallGetter($Symbol.prototype, "name", SymbolGetName);
   InstallFunctions($Symbol.prototype, DONT_ENUM, $Array(
     "toString", SymbolToString,
     "valueOf", SymbolValueOf
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Tue Feb 18 16:34:52 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-api.cc Wed Feb 19 14:19:42 2014 UTC
@@ -2786,7 +2786,7 @@
   CHECK(sym_obj->IsSymbolObject());
   CHECK(!sym2->IsSymbolObject());
   CHECK(!obj->IsSymbolObject());
-  CHECK(sym_obj->Equals(sym2));
+  CHECK(!sym_obj->Equals(sym2));
   CHECK(!sym_obj->StrictEquals(sym2));
   CHECK(v8::SymbolObject::Cast(*sym_obj)->Equals(sym_obj));
   CHECK(v8::SymbolObject::Cast(*sym_obj)->ValueOf()->Equals(sym2));
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/private.js Tue Nov 26 11:32:39 2013 UTC +++ /branches/bleeding_edge/test/mjsunit/harmony/private.js Wed Feb 19 14:19:42 2014 UTC
@@ -30,6 +30,16 @@

 var symbols = []

+
+// Returns true if the string is a valid
+// serialization of Symbols added to the 'symbols'
+// array. Adjust if you extend 'symbols' with other
+// values.
+function isValidSymbolString(s) {
+  return ["Symbol(66)"].indexOf(s) >= 0;
+}
+
+
 // Test different forms of constructor calls, all equivalent.
 function TestNew() {
   for (var i = 0; i < 2; ++i) {
@@ -49,7 +59,6 @@
     assertTrue(typeof symbols[i] === "symbol")
     assertTrue(%SymbolIsPrivate(symbols[i]))
     assertEquals(null, %_ClassOf(symbols[i]))
-    assertEquals("Symbol", %_ClassOf(new Symbol(symbols[i])))
     assertEquals("Symbol", %_ClassOf(Object(symbols[i])))
   }
 }
@@ -67,28 +76,21 @@
 function TestConstructor() {
   for (var i in symbols) {
     assertSame(Symbol, symbols[i].__proto__.constructor)
+    assertSame(Symbol, Object(symbols[i]).__proto__.constructor)
   }
 }
 TestConstructor()


-function TestName() {
-  for (var i in symbols) {
-    var name = symbols[i].name
-    assertTrue(name === "66")
-  }
-}
-TestName()
-
-
 function TestToString() {
   for (var i in symbols) {
     assertThrows(function() { String(symbols[i]) }, TypeError)
     assertThrows(function() { symbols[i] + "" }, TypeError)
-    assertThrows(function() { symbols[i].toString() }, TypeError)
- assertThrows(function() { (new Symbol(symbols[i])).toString() }, TypeError)
-    assertThrows(function() { Object(symbols[i]).toString() }, TypeError)
- assertEquals("[object Symbol]", Object.prototype.toString.call(symbols[i]))
+    assertTrue(isValidSymbolString(symbols[i].toString()))
+    assertTrue(isValidSymbolString(Object(symbols[i]).toString()))
+ assertTrue(isValidSymbolString(Symbol.prototype.toString.call(symbols[i])))
+    assertEquals(
+        "[object Symbol]", Object.prototype.toString.call(symbols[i]))
   }
 }
 TestToString()
@@ -128,10 +130,14 @@
     assertTrue(Object.is(symbols[i], symbols[i]))
     assertTrue(symbols[i] === symbols[i])
     assertTrue(symbols[i] == symbols[i])
-    assertFalse(symbols[i] === new Symbol(symbols[i]))
-    assertFalse(new Symbol(symbols[i]) === symbols[i])
-    assertTrue(symbols[i] == new Symbol(symbols[i]))
-    assertTrue(new Symbol(symbols[i]) == symbols[i])
+    assertFalse(symbols[i] === Object(symbols[i]))
+    assertFalse(Object(symbols[i]) === symbols[i])
+    assertFalse(symbols[i] == Object(symbols[i]))
+    assertFalse(Object(symbols[i]) == symbols[i])
+    assertTrue(symbols[i] === symbols[i].valueOf())
+    assertTrue(symbols[i].valueOf() === symbols[i])
+    assertTrue(symbols[i] == symbols[i].valueOf())
+    assertTrue(symbols[i].valueOf() == symbols[i])
   }

   // All symbols should be distinct.
@@ -159,7 +165,7 @@

 function TestGet() {
   for (var i in symbols) {
-    assertThrows(function() { symbols[i].toString() }, TypeError)
+    assertTrue(isValidSymbolString(symbols[i].toString()))
     assertEquals(symbols[i], symbols[i].valueOf())
     assertEquals(undefined, symbols[i].a)
     assertEquals(undefined, symbols[i]["a" + "b"])
@@ -173,7 +179,7 @@
 function TestSet() {
   for (var i in symbols) {
     symbols[i].toString = 0
-    assertThrows(function() { symbols[i].toString() }, TypeError)
+    assertTrue(isValidSymbolString(symbols[i].toString()))
     symbols[i].valueOf = 0
     assertEquals(symbols[i], symbols[i].valueOf())
     symbols[i].a = 0
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/symbols.js Thu Jan 9 15:57:30 2014 UTC +++ /branches/bleeding_edge/test/mjsunit/harmony/symbols.js Wed Feb 19 14:19:42 2014 UTC
@@ -30,27 +30,35 @@

 var symbols = []

-// Test different forms of constructor calls, all equivalent.
+
+// Returns true if the string is a valid
+// serialization of Symbols added to the 'symbols'
+// array. Adjust if you extend 'symbols' with other
+// values.
+function isValidSymbolString(s) {
+  return ["Symbol(66)", "Symbol()"].indexOf(s) >= 0;
+}
+
+
+// Test different forms of constructor calls.
 function TestNew() {
-  function IndirectSymbol() { return new Symbol }
-  function indirect() { return new IndirectSymbol() }
+  function indirectSymbol() { return Symbol() }
+  function indirect() { return indirectSymbol() }
   for (var i = 0; i < 2; ++i) {
     for (var j = 0; j < 5; ++j) {
       symbols.push(Symbol())
       symbols.push(Symbol(undefined))
       symbols.push(Symbol("66"))
       symbols.push(Symbol(66))
-      symbols.push(Symbol(Symbol()))
-      symbols.push((new Symbol).valueOf())
-      symbols.push((new Symbol()).valueOf())
-      symbols.push((new Symbol(Symbol())).valueOf())
-      symbols.push(Object(Symbol()).valueOf())
-      symbols.push((indirect()).valueOf())
+      symbols.push(Symbol().valueOf())
+      symbols.push(indirect())
     }
     %OptimizeFunctionOnNextCall(indirect)
     indirect()  // Call once before GC throws away type feedback.
     gc()        // Promote existing symbols and then allocate some more.
   }
+  assertThrows(function () { Symbol(Symbol()) }, TypeError)
+  assertThrows(function () { new Symbol(66) }, TypeError)
 }
 TestNew()

@@ -61,7 +69,6 @@
     assertTrue(typeof symbols[i] === "symbol")
     assertFalse(%SymbolIsPrivate(symbols[i]))
     assertEquals(null, %_ClassOf(symbols[i]))
-    assertEquals("Symbol", %_ClassOf(new Symbol(symbols[i])))
     assertEquals("Symbol", %_ClassOf(Object(symbols[i])))
   }
 }
@@ -71,10 +78,6 @@
 function TestPrototype() {
   assertSame(Object.prototype, Symbol.prototype.__proto__)
   assertSame(Symbol.prototype, Symbol().__proto__)
-  assertSame(Symbol.prototype, Symbol(Symbol()).__proto__)
-  assertSame(Symbol.prototype, (new Symbol).__proto__)
-  assertSame(Symbol.prototype, (new Symbol()).__proto__)
-  assertSame(Symbol.prototype, (new Symbol(Symbol())).__proto__)
   assertSame(Symbol.prototype, Object(Symbol()).__proto__)
   for (var i in symbols) {
     assertSame(Symbol.prototype, symbols[i].__proto__)
@@ -84,14 +87,11 @@


 function TestConstructor() {
+  assertSame(Function.prototype, Symbol.__proto__)
   assertFalse(Object === Symbol.prototype.constructor)
   assertFalse(Symbol === Object.prototype.constructor)
   assertSame(Symbol, Symbol.prototype.constructor)
   assertSame(Symbol, Symbol().__proto__.constructor)
-  assertSame(Symbol, Symbol(Symbol()).__proto__.constructor)
-  assertSame(Symbol, (new Symbol).__proto__.constructor)
-  assertSame(Symbol, (new Symbol()).__proto__.constructor)
-  assertSame(Symbol, (new Symbol(Symbol())).__proto__.constructor)
   assertSame(Symbol, Object(Symbol()).__proto__.constructor)
   for (var i in symbols) {
     assertSame(Symbol, symbols[i].__proto__.constructor)
@@ -100,23 +100,26 @@
 TestConstructor()


-function TestName() {
+function TestValueOf() {
   for (var i in symbols) {
-    var name = symbols[i].name
-    assertTrue(name === undefined || name === "66")
+    assertTrue(symbols[i] === symbols[i].valueOf())
+    assertTrue(Symbol.prototype.valueOf.call(symbols[i]) === symbols[i])
   }
 }
-TestName()
+TestValueOf()


 function TestToString() {
   for (var i in symbols) {
     assertThrows(function() { String(symbols[i]) }, TypeError)
     assertThrows(function() { symbols[i] + "" }, TypeError)
-    assertThrows(function() { symbols[i].toString() }, TypeError)
- assertThrows(function() { (new Symbol(symbols[i])).toString() }, TypeError)
-    assertThrows(function() { Object(symbols[i]).toString() }, TypeError)
- assertEquals("[object Symbol]", Object.prototype.toString.call(symbols[i]))
+    assertTrue(isValidSymbolString(String(Object(symbols[i]))))
+    assertTrue(isValidSymbolString(symbols[i].toString()))
+    assertTrue(isValidSymbolString(Object(symbols[i]).toString()))
+    assertTrue(
+      isValidSymbolString(Symbol.prototype.toString.call(symbols[i])))
+    assertEquals(
+      "[object Symbol]", Object.prototype.toString.call(symbols[i]))
   }
 }
 TestToString()
@@ -156,10 +159,16 @@
     assertTrue(Object.is(symbols[i], symbols[i]))
     assertTrue(symbols[i] === symbols[i])
     assertTrue(symbols[i] == symbols[i])
-    assertFalse(symbols[i] === new Symbol(symbols[i]))
-    assertFalse(new Symbol(symbols[i]) === symbols[i])
-    assertTrue(symbols[i] == new Symbol(symbols[i]))
-    assertTrue(new Symbol(symbols[i]) == symbols[i])
+    assertFalse(symbols[i] === Object(symbols[i]))
+    assertFalse(Object(symbols[i]) === symbols[i])
+    assertFalse(symbols[i] == Object(symbols[i]))
+    assertFalse(Object(symbols[i]) == symbols[i])
+    assertTrue(symbols[i] === symbols[i].valueOf())
+    assertTrue(symbols[i].valueOf() === symbols[i])
+    assertTrue(symbols[i] == symbols[i].valueOf())
+    assertTrue(symbols[i].valueOf() == symbols[i])
+    assertFalse(Object(symbols[i]) === Object(symbols[i]))
+ assertEquals(Object(symbols[i]).valueOf(), Object(symbols[i]).valueOf())
   }

   // All symbols should be distinct.
@@ -187,7 +196,7 @@

 function TestGet() {
   for (var i in symbols) {
-    assertThrows(function() { symbols[i].toString() }, TypeError)
+    assertTrue(isValidSymbolString(symbols[i].toString()))
     assertEquals(symbols[i], symbols[i].valueOf())
     assertEquals(undefined, symbols[i].a)
     assertEquals(undefined, symbols[i]["a" + "b"])
@@ -201,7 +210,7 @@
 function TestSet() {
   for (var i in symbols) {
     symbols[i].toString = 0
-    assertThrows(function() { symbols[i].toString() }, TypeError)
+    assertTrue(isValidSymbolString(symbols[i].toString()))
     symbols[i].valueOf = 0
     assertEquals(symbols[i], symbols[i].valueOf())
     symbols[i].a = 0
@@ -215,6 +224,18 @@
 TestSet()


+// Test Symbol wrapping/boxing over non-builtins.
+Symbol.prototype.getThisProto = function () {
+  return Object.getPrototypeOf(this);
+}
+function TestCall() {
+  for (var i in symbols) {
+    assertTrue(symbols[i].getThisProto() === Symbol.prototype)
+  }
+}
+TestCall()
+
+
 function TestCollections() {
   var set = new Set
   var map = new Map
@@ -309,7 +330,7 @@

 function TestKeyDescriptor(obj) {
   for (var i in symbols) {
-    var desc = Object.getOwnPropertyDescriptor(obj, symbols[i]);
+    var desc = Object.getOwnPropertyDescriptor(obj, symbols[i])
     assertEquals(i|0, desc.value)
     assertTrue(desc.configurable)
     assertEquals(i % 2 == 0, desc.writable)

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to