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
R=arv@chromium.org, rossberg@chromium.org, arv, rossberg
BUG=v8:3053

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

Patch from Sigbjorn Finne <sigbjornf@opera.com>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19490 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/messages.js b/src/messages.js
index 733fe95..ca2da30 100644 (file)
--- a/src/messages.js
+++ b/src/messages.js
@@ -176,7 +176,8 @@ var kMessages = {
   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"]

diff --git a/src/objects.cc b/src/objects.cc
index f4673a1..9ecbe6a 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -80,6 +80,8 @@ MaybeObject* Object::ToObject(Context* native_context) {
     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;

diff --git a/src/runtime.cc b/src/runtime.cc
index 7eaf5a7..55013f1 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -633,7 +633,14 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreatePrivateSymbol) {
 }
 
 
-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);

diff --git a/src/runtime.h b/src/runtime.h
index 8fe6db0..823285f 100644 (file)
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -321,7 +321,8 @@ namespace internal {
   /* 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 */ \

diff --git a/src/runtime.js b/src/runtime.js
index 2a949ae..a49bc84 100644 (file)
--- a/src/runtime.js
+++ b/src/runtime.js
@@ -75,11 +75,8 @@ function EQUALS(y) {
         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 @@ function EQUALS(y) {
         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 @@ function ToPrimitive(x, hint) {
   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 @@ function ToString(x) {
   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 @@ function NonStringToString(x) {
   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 @@ function ToName(x) {
 // 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', []);
   }

diff --git a/src/symbol.js b/src/symbol.js
index be308d9..8cb1927 100644 (file)
--- a/src/symbol.js
+++ b/src/symbol.js
@@ -36,34 +36,28 @@ var $Symbol = global.Symbol;
 // -------------------------------------------------------------------
 
 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 @@ function SetUpSymbol() {
   %CheckIsBootstrapping();
 
   %SetCode($Symbol, SymbolConstructor);
-  %FunctionSetPrototype($Symbol, new $Symbol());
-  %SetProperty($Symbol.prototype, "constructor", $Symbol, DONT_ENUM);
+  %FunctionSetPrototype($Symbol, new $Object());
 
-  InstallGetter($Symbol.prototype, "name", SymbolGetName);
+  %SetProperty($Symbol.prototype, "constructor", $Symbol, DONT_ENUM);
   InstallFunctions($Symbol.prototype, DONT_ENUM, $Array(
     "toString", SymbolToString,
     "valueOf", SymbolValueOf

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 9312057..c5c932b 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -2786,7 +2786,7 @@ THREADED_TEST(SymbolProperties) {
   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));

diff --git a/test/mjsunit/harmony/private.js b/test/mjsunit/harmony/private.js
index 09cf7f7..c906632 100644 (file)
--- a/test/mjsunit/harmony/private.js
+++ b/test/mjsunit/harmony/private.js
@@ -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 @@ function TestType() {
     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 @@ TestPrototype()
 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 @@ function TestEquality() {
     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 @@ TestEquality()
 
 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 @@ TestGet()
 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

diff --git a/test/mjsunit/harmony/symbols.js b/test/mjsunit/harmony/symbols.js
index ce02a05..154eca0 100644 (file)
--- a/test/mjsunit/harmony/symbols.js
+++ b/test/mjsunit/harmony/symbols.js
@@ -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 @@ function TestType() {
     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 @@ TestType()
 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 @@ TestPrototype()
 
 
 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 @@ function TestConstructor() {
 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 @@ function TestEquality() {
     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 @@ TestEquality()
 
 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 @@ TestGet()
 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 @@ function TestSet() {
 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 TestGetOwnPropertySymbols(obj) {
 
 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)