Reviewers: Dmitry Lomov (chromium), adamk,

Message:
PTAL

Description:
ToNumber(Symbol) should throw TypeError

https://people.mozilla.org/~jorendorff/es6-draft.html#sec-tonumber

Based on patch from caitp <[email protected]>
https://codereview.chromium.org/454233002/

BUG=v8:3499
LOG=Y

Please review this at https://codereview.chromium.org/458753004/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+36, -30 lines):
  M src/messages.js
  M src/runtime.js
  M src/v8natives.js
  M test/mjsunit/es6/symbols.js
  M test/mjsunit/harmony/private.js
  M test/mjsunit/object-toprimitive.js


Index: src/messages.js
diff --git a/src/messages.js b/src/messages.js
index 90a756cee1abc64cdd09c492bde4777f4497d3d0..eba1e16ece04e8603ff1ca67a5e9ab8ddcaec061 100644
--- a/src/messages.js
+++ b/src/messages.js
@@ -164,6 +164,7 @@ var kMessages = {
   harmony_const_assign:          ["Assignment to constant variable."],
symbol_to_string: ["Cannot convert a Symbol value to a string"], symbol_to_primitive: ["Cannot convert a Symbol wrapper object to a primitive value"], + symbol_to_number: ["Cannot convert a Symbol value to a number"], 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"]
Index: src/runtime.js
diff --git a/src/runtime.js b/src/runtime.js
index c070a1119e7bdadec9679c56e71b435538baa70e..d9e1fe5c994030f40461167faac8b3df4d60ff22 100644
--- a/src/runtime.js
+++ b/src/runtime.js
@@ -36,6 +36,7 @@ function EQUALS(y) {
       while (true) {
         if (IS_NUMBER(y)) return %NumberEquals(x, y);
         if (IS_NULL_OR_UNDEFINED(y)) return 1;  // not equal
+        if (IS_SYMBOL(y)) return 1;  // not equal
         if (!IS_SPEC_OBJECT(y)) {
           // String or boolean.
           return %NumberEquals(x, %ToNumber(y));
@@ -501,7 +502,7 @@ function ToNumber(x) {
   }
   if (IS_BOOLEAN(x)) return x ? 1 : 0;
   if (IS_UNDEFINED(x)) return NAN;
-  if (IS_SYMBOL(x)) return NAN;
+  if (IS_SYMBOL(x)) throw MakeTypeError('symbol_to_number', []);
   return (IS_NULL(x)) ? 0 : ToNumber(%DefaultNumber(x));
 }

@@ -512,7 +513,7 @@ function NonNumberToNumber(x) {
   }
   if (IS_BOOLEAN(x)) return x ? 1 : 0;
   if (IS_UNDEFINED(x)) return NAN;
-  if (IS_SYMBOL(x)) return NAN;
+  if (IS_SYMBOL(x)) throw MakeTypeError('symbol_to_number', []);
   return (IS_NULL(x)) ? 0 : ToNumber(%DefaultNumber(x));
 }

Index: src/v8natives.js
diff --git a/src/v8natives.js b/src/v8natives.js
index 4c56fd16efb66a224a94cad9357bd246e3d9b073..7082e2d949e43c0fe37cf71142c7d258de34ed64 100644
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -925,34 +925,36 @@ function DefineArrayProperty(obj, p, desc, should_throw) {
   }

   // Step 4 - Special handling for array index.
-  var index = ToUint32(p);
-  var emit_splice = false;
-  if (ToString(index) == p && index != 4294967295) {
-    var length = obj.length;
-    if (index >= length && %IsObserved(obj)) {
-      emit_splice = true;
-      BeginPerformSplice(obj);
-    }
+  if (typeof p !== "symbol") {
+    var index = ToUint32(p);
+    var emit_splice = false;
+    if (ToString(index) == p && index != 4294967295) {
+      var length = obj.length;
+      if (index >= length && %IsObserved(obj)) {
+        emit_splice = true;
+        BeginPerformSplice(obj);
+      }

-    var length_desc = GetOwnPropertyJS(obj, "length");
-    if ((index >= length && !length_desc.isWritable()) ||
-        !DefineObjectProperty(obj, p, desc, true)) {
-      if (emit_splice)
+      var length_desc = GetOwnPropertyJS(obj, "length");
+      if ((index >= length && !length_desc.isWritable()) ||
+          !DefineObjectProperty(obj, p, desc, true)) {
+        if (emit_splice)
+          EndPerformSplice(obj);
+        if (should_throw) {
+          throw MakeTypeError("define_disallowed", [p]);
+        } else {
+          return false;
+        }
+      }
+      if (index >= length) {
+        obj.length = index + 1;
+      }
+      if (emit_splice) {
         EndPerformSplice(obj);
-      if (should_throw) {
-        throw MakeTypeError("define_disallowed", [p]);
-      } else {
-        return false;
+        EnqueueSpliceRecord(obj, length, [], index + 1 - length);
       }
+      return true;
     }
-    if (index >= length) {
-      obj.length = index + 1;
-    }
-    if (emit_splice) {
-      EndPerformSplice(obj);
-      EnqueueSpliceRecord(obj, length, [], index + 1 - length);
-    }
-    return true;
   }

   // Step 5 - Fallback to default implementation.
Index: test/mjsunit/es6/symbols.js
diff --git a/test/mjsunit/es6/symbols.js b/test/mjsunit/es6/symbols.js
index f53251811a74088c023b45b047759719aa34d0e4..0b0700270025c8e4cd09d29146939acd8e5ae70f 100644
--- a/test/mjsunit/es6/symbols.js
+++ b/test/mjsunit/es6/symbols.js
@@ -149,8 +149,8 @@ function TestToNumber() {
   for (var i in symbols) {
     assertThrows(function() { Number(Object(symbols[i])) }, TypeError)
     assertThrows(function() { +Object(symbols[i]) }, TypeError)
-    assertSame(NaN, Number(symbols[i]).valueOf())
-    assertSame(NaN, symbols[i] + 0)
+    assertThrows(function() { Number(symbols[i]) }, TypeError)
+    assertThrows(function() { symbols[i] + 0 }, TypeError)
   }
 }
 TestToNumber()
Index: test/mjsunit/harmony/private.js
diff --git a/test/mjsunit/harmony/private.js b/test/mjsunit/harmony/private.js index 3d95d54626921c548615c7c69b16facdd263b683..4b29fd863ed385e35533365646f6bd08d6608e84 100644
--- a/test/mjsunit/harmony/private.js
+++ b/test/mjsunit/harmony/private.js
@@ -114,8 +114,8 @@ TestToBoolean()

 function TestToNumber() {
   for (var i in symbols) {
-    assertSame(NaN, Number(symbols[i]).valueOf())
-    assertSame(NaN, symbols[i] + 0)
+    assertThrows(function() { Number(symbols[i]); }, TypeError);
+    assertThrows(function() { symbols[i] + 0; }, TypeError);
   }
 }
 TestToNumber()
Index: test/mjsunit/object-toprimitive.js
diff --git a/test/mjsunit/object-toprimitive.js b/test/mjsunit/object-toprimitive.js index 3a67ced47e72ba359f0be71801eb1a6d013a7484..34803ec93482668a13b3ec27303054505110ab93 100644
--- a/test/mjsunit/object-toprimitive.js
+++ b/test/mjsunit/object-toprimitive.js
@@ -102,3 +102,5 @@ trace = [];
 var nt = Number(ot);
 assertEquals(87, nt);
 assertEquals(["gvo", "gts", "ts"], trace);
+
+assertThrows('Number(Symbol())', TypeError);


--
--
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/d/optout.

Reply via email to