Upgrade Symbol implementation to match current ES6 behavior.

test/mjsunit/harmony/symbols.js

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 12 matching lines @@
 // DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 // THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 // Flags: --harmony-symbols --harmony-collections
 // Flags: --expose-gc --allow-natives-syntax
 
 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)", "Symbol()"].indexOf(s) >= 0;
+}
+
+
 // Test different forms of constructor calls, all equivalent.
 function TestNew() {
-  function IndirectSymbol() { return new Symbol }
-  function indirect() { return new IndirectSymbol() }
+  function IndirectSymbol() { return Symbol() }

[arv (Not doing code reviews), 2014/02/01 01:33:10]

Should be lower  case now since it is not a constructor.

+  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()))
+      assertThrows(function () { Symbol(Symbol()) }, TypeError)
+      assertThrows(function () { new Symbol(66) }, TypeError)
     }
     %OptimizeFunctionOnNextCall(indirect)
     indirect()  // Call once before GC throws away type feedback.
     gc()        // Promote existing symbols and then allocate some more.
   }
 }
 TestNew()
 
 
 function TestType() {
   for (var i in symbols) {
     assertEquals("symbol", typeof symbols[i])
     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])))
   }
 }
 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__)
   }
 }
 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)
   }
 }
 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())
+    assertThrows(function() { Symbol.prototype.valueOf.call(symbols[i]) }, TypeError)
   }
 }
-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)
+    assertThrows(function() { (Symbol(symbols[i])).toString() }, TypeError)
+    assertThrows(function() { Symbol.prototype.toString.call(symbols[i]) }, TypeError)
+
+    function checkToString(f) {
+      var stringValue
+      assertDoesNotThrow(function() { stringValue = f() })
+      assertTrue(isValidSymbolString(stringValue))
+    }
+    checkToString(function() { return String(Object(symbols[i])) })
+    checkToString(function() { return symbols[i].toString() })
+    checkToString(function() { return Object(symbols[i]).toString() })
+
     assertEquals("[object Symbol]", Object.prototype.toString.call(symbols[i]))
   }
 }
 TestToString()
 
 
 function TestToBoolean() {
   for (var i in symbols) {
     assertTrue(Boolean(symbols[i]).valueOf())
     assertFalse(!symbols[i])
@@ skipping 20 matching lines @@
 
 
 function TestEquality() {
   // Every symbol should equal itself, and non-strictly equal its wrapper.
   for (var i in symbols) {
     assertSame(symbols[i], symbols[i])
     assertEquals(symbols[i], symbols[i])
     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])
+    assertTrue(symbols[i] === symbols[i].valueOf())
+    assertTrue(symbols[i].valueOf() === symbols[i])
+    assertTrue(symbols[i] == symbols[i].valueOf())
+    assertTrue(symbols[i].valueOf() == symbols[i])
+    assertEquals(Object(symbols[i]), Object(symbols[i]))

[arv (Not doing code reviews), 2014/02/01 01:33:10]

This looks strange. If symbol[i] is a symbol (not a symbol wrapper) then this
should be false. Are all elements in symbols wrappers? It does not seem like
that is the case on line 49.

[sof, 2014/02/01 13:58:10]

They're all Symbol values. This is an equals check, not same, so why should
deepObjectEquals() return false? (Perhaps not a very interesting to test to add,
I can remove.)

[arv (Not doing code reviews), 2014/02/02 19:49:35]

If all elements in symbols are symbol values (not symbol wrapper objects) this
should be assertNotEquals since a new wrapper is always created when you do
Object(value).

[sof, 2014/02/02 19:59:57]

I know; the object also doesn't have any "keys" in the (Object.keys() sense), so
I don't think it much of a difference how you want to phrase that equality check
for these wrapper objects. Had assertNotEquals existed, that is.

The offer to remove the test still stands; I don't think there is anything
fundamentally wrong here in the interpretation of symbol values vs wrappers.

+    assertEquals(Object(symbols[i]).valueOf(), Object(symbols[i]).valueOf())
   }
 
   // All symbols should be distinct.
   for (var i = 0; i < symbols.length; ++i) {
     for (var j = i + 1; j < symbols.length; ++j) {
       assertFalse(Object.is(symbols[i], symbols[j]))
       assertFalse(symbols[i] === symbols[j])
       assertFalse(symbols[i] == symbols[j])
     }
   }
 
   // Symbols should not be equal to any other value (and the test terminates).
   var values = [347, 1.275, NaN, "string", null, undefined, {}, function() {}]
   for (var i in symbols) {
     for (var j in values) {
       assertFalse(symbols[i] === values[j])
       assertFalse(values[j] === symbols[i])
       assertFalse(symbols[i] == values[j])
       assertFalse(values[j] == symbols[i])
     }
   }
 }
 TestEquality()
 
 
 function TestGet() {
   for (var i in symbols) {
-    assertThrows(function() { symbols[i].toString() }, TypeError)
+    assertDoesNotThrow(function() { symbols[i].toString() })
     assertEquals(symbols[i], symbols[i].valueOf())
     assertEquals(undefined, symbols[i].a)
     assertEquals(undefined, symbols[i]["a" + "b"])
     assertEquals(undefined, symbols[i]["" + "1"])
     assertEquals(undefined, symbols[i][62])
   }
 }
 TestGet()
 
 
 function TestSet() {
   for (var i in symbols) {
     symbols[i].toString = 0
-    assertThrows(function() { symbols[i].toString() }, TypeError)
+    assertDoesNotThrow(function() { symbols[i].toString() })
     symbols[i].valueOf = 0
     assertEquals(symbols[i], symbols[i].valueOf())
     symbols[i].a = 0
     assertEquals(undefined, symbols[i].a)
     symbols[i]["a" + "b"] = 0
     assertEquals(undefined, symbols[i]["a" + "b"])
     symbols[i][62] = 0
     assertEquals(undefined, symbols[i][62])
   }
 }
 TestSet()
 
 
+// Test Symbol wrapping/boxing over non-builtins.
+Symbol.prototype.extra = function (m) { return (m + this.toString()) }
+function TestCall() {
+  for (var i in symbols) {
+    var stringValue
+    assertDoesNotThrow(function() { stringValue = symbols[i].extra("") })
+    assertTrue(isValidSymbolString(stringValue))
+  }
+}
+TestCall()
+
+
 function TestCollections() {
   var set = new Set
   var map = new Map
   var weakmap = new WeakMap
   for (var i in symbols) {
     set.add(symbols[i])
     map.set(symbols[i], i)
     weakmap.set(symbols[i], i)
   }
   assertEquals(symbols.length, set.size)
@@ skipping 65 matching lines @@
 
   var names = Object.getOwnPropertyNames(obj)
   for (var i in names) {
     assertEquals("string", typeof names[i])
   }
 }
 
 
 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)
     assertEquals(i % 2 == 0, desc.enumerable)
     assertEquals(i % 2 == 0,
         Object.prototype.propertyIsEnumerable.call(obj, symbols[i]))
   }
 }
 
 
@@ skipping 29 matching lines @@
   // but not between scavenges.  This must also apply for symbol keys.
   var key = Symbol("key");
   var a = {};
   a[key] = "abc";
 
   for (var i = 0; i < 100000; i++) {
     a[key] += "a";  // Allocations cause a scavenge.
   }
 }
 TestCachedKeyAfterScavenge();