Bug 551118. Reuse string-to-number code and fix bug with negative hex in strings being treated as a negative integer. r=brendan
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 09 Mar 2010 17:21:32 -0500
changeset 39941 fc1b3ed54cc2b77452acb5a667f7e1deee1f4c6d
parent 39940 f77c2e64382afae70e3b74b08eb18517a88376af
child 39942 24b2678b458d8345f06b40d0b08aa32f07fbc777
push idunknown
push userunknown
push dateunknown
reviewersbrendan
bugs551118
milestone1.9.3a2pre
Bug 551118. Reuse string-to-number code and fix bug with negative hex in strings being treated as a negative integer. r=brendan
js/src/jsbuiltins.cpp
js/src/jsnum.cpp
js/src/jsnum.h
js/src/trace-test/tests/basic/testStringToNumber.js
--- a/js/src/jsbuiltins.cpp
+++ b/js/src/jsbuiltins.cpp
@@ -182,55 +182,24 @@ js_DoubleToUint32(jsdouble d)
 {
     return js_DoubleToECMAUint32(d);
 }
 JS_DEFINE_CALLINFO_1(extern, UINT32, js_DoubleToUint32, DOUBLE, 1, ACC_NONE)
 
 jsdouble FASTCALL
 js_StringToNumber(JSContext* cx, JSString* str)
 {
-    const jschar* bp;
-    const jschar* end;
-    const jschar* ep;
-    jsdouble d;
-
-    str->getCharsAndEnd(bp, end);
-    if ((!js_strtod(cx, bp, end, &ep, &d) ||
-         js_SkipWhiteSpace(ep, end) != end) &&
-        (!js_strtointeger(cx, bp, end, &ep, 0, &d) ||
-         js_SkipWhiteSpace(ep, end) != end)) {
-        return js_NaN;
-    }
-    return d;
+    return StringToNumberType<jsdouble>(cx, str);
 }
 JS_DEFINE_CALLINFO_2(extern, DOUBLE, js_StringToNumber, CONTEXT, STRING, 1, ACC_NONE)
 
 int32 FASTCALL
 js_StringToInt32(JSContext* cx, JSString* str)
 {
-    const jschar* bp;
-    const jschar* end;
-    const jschar* ep;
-    jsdouble d;
-
-    if (str->length() == 1) {
-        jschar c = str->chars()[0];
-        if ('0' <= c && c <= '9')
-            return c - '0';
-        return 0;	
-    }
-
-    str->getCharsAndEnd(bp, end);
-    if ((!js_strtod(cx, bp, end, &ep, &d) ||
-         js_SkipWhiteSpace(ep, end) != end) &&
-        (!js_strtointeger(cx, bp, end, &ep, 0, &d) ||
-         js_SkipWhiteSpace(ep, end) != end)) {
-        return 0;
-    }
-    return js_DoubleToECMAInt32(d);
+    return StringToNumberType<int32>(cx, str);
 }
 JS_DEFINE_CALLINFO_2(extern, INT32, js_StringToInt32, CONTEXT, STRING, 1, ACC_NONE)
 
 JSBool FASTCALL
 js_AddProperty(JSContext* cx, JSObject* obj, JSScopeProperty* sprop)
 {
     JS_LOCK_OBJ(cx, obj);
 
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -68,16 +68,17 @@
 #include "jsobj.h"
 #include "jsopcode.h"
 #include "jsprf.h"
 #include "jsscope.h"
 #include "jsstr.h"
 #include "jsstrinlines.h"
 #include "jsvector.h"
 
+using namespace js;
 
 #ifndef JS_HAVE_STDINT_H /* Native support is innocent until proven guilty. */
 
 JS_STATIC_ASSERT(uint8_t(-1) == UINT8_MAX);
 JS_STATIC_ASSERT(uint16_t(-1) == UINT16_MAX);
 JS_STATIC_ASSERT(uint32_t(-1) == UINT32_MAX);
 JS_STATIC_ASSERT(uint64_t(-1) == UINT64_MAX);
 
@@ -938,49 +939,30 @@ js_NumberValueToCharBuffer(JSContext *cx
     return JS_TRUE;
 }
 
 jsdouble
 js_ValueToNumber(JSContext *cx, jsval *vp)
 {
     jsval v;
     JSString *str;
-    const jschar *bp, *end, *ep;
     jsdouble d;
     JSObject *obj;
 
     v = *vp;
     for (;;) {
         if (JSVAL_IS_INT(v))
             return (jsdouble) JSVAL_TO_INT(v);
         if (JSVAL_IS_DOUBLE(v))
             return *JSVAL_TO_DOUBLE(v);
         if (JSVAL_IS_STRING(v)) {
             str = JSVAL_TO_STRING(v);
 
-            /*
-             * Note that ECMA doesn't treat a string beginning with a '0' as
-             * an octal number here. This works because all such numbers will
-             * be interpreted as decimal by js_strtod and will never get
-             * passed to js_strtointeger (which would interpret them as
-             * octal).
-             */
-            str->getCharsAndEnd(bp, end);
-
-            /* ECMA doesn't allow signed hex numbers (bug 273467). */
-            bp = js_SkipWhiteSpace(bp, end);
-            if (bp + 2 < end && (*bp == '-' || *bp == '+') &&
-                bp[1] == '0' && (bp[2] == 'X' || bp[2] == 'x')) {
-                break;
-            }
-
-            if ((!js_strtod(cx, bp, end, &ep, &d) ||
-                 js_SkipWhiteSpace(ep, end) != end) &&
-                (!js_strtointeger(cx, bp, end, &ep, 0, &d) ||
-                 js_SkipWhiteSpace(ep, end) != end)) {
+            d = StringToNumberType<jsdouble>(cx, str);
+            if (JSDOUBLE_IS_NaN(d)) {
                 break;
             }
 
             /*
              * JSVAL_TRUE indicates that double jsval was never constructed
              * for the result.
              */
             *vp = JSVAL_TRUE;
--- a/js/src/jsnum.h
+++ b/js/src/jsnum.h
@@ -43,16 +43,18 @@
 #include <math.h>
 #if defined(XP_WIN) || defined(XP_OS2)
 #include <float.h>
 #endif
 #ifdef SOLARIS
 #include <ieeefp.h>
 #endif
 
+#include "jsstr.h"
+
 /*
  * JS number (IEEE double) interface.
  *
  * JS numbers are optimistically stored in the top 31 bits of 32-bit integers,
  * but floating point literals, results that overflow 31 bits, and division and
  * modulus operands and results require a 64-bit IEEE double.  These are GC'ed
  * and pointed to by 32-bit jsvals on the stack and in object properties.
  */
@@ -421,9 +423,67 @@ js_strtod(JSContext *cx, const jschar *s
  * Return false if out of memory.
  */
 extern JSBool
 js_strtointeger(JSContext *cx, const jschar *s, const jschar *send,
                 const jschar **ep, jsint radix, jsdouble *dp);
 
 JS_END_EXTERN_C
 
+namespace js {
+template<typename T> struct NumberTraits { };
+template<> struct NumberTraits<int32> {
+  static JS_ALWAYS_INLINE int32 NaN() { return 0; }
+  static JS_ALWAYS_INLINE int32 toSelfType(int32 i) { return i; }
+  static JS_ALWAYS_INLINE int32 toSelfType(jsdouble d) { return js_DoubleToECMAUint32(d); }
+};
+template<> struct NumberTraits<jsdouble> {
+  static JS_ALWAYS_INLINE jsdouble NaN() { return js_NaN; }
+  static JS_ALWAYS_INLINE jsdouble toSelfType(int32 i) { return i; }
+  static JS_ALWAYS_INLINE jsdouble toSelfType(jsdouble d) { return d; }
+};
+
+template<typename T>
+static JS_ALWAYS_INLINE T
+StringToNumberType(JSContext *cx, JSString *str)
+{
+    if (str->length() == 1) {
+        jschar c = str->chars()[0];
+        if ('0' <= c && c <= '9')
+            return NumberTraits<T>::toSelfType(int32(c - '0'));
+        return NumberTraits<T>::NaN();
+    }
+
+    const jschar* bp;
+    const jschar* end;
+    const jschar* ep;
+    jsdouble d;
+
+    str->getCharsAndEnd(bp, end);
+    bp = js_SkipWhiteSpace(bp, end);
+
+    /* ECMA doesn't allow signed hex numbers (bug 273467). */
+    if (end - bp >= 2 && bp[0] == '0' && (bp[1] == 'x' || bp[1] == 'X')) {
+        /* Looks like a hex number. */
+        if (!js_strtointeger(cx, bp, end, &ep, 16, &d) ||
+            js_SkipWhiteSpace(ep, end) != end) {
+            return NumberTraits<T>::NaN();
+        }
+        return NumberTraits<T>::toSelfType(d);
+    }
+
+    /*
+     * Note that ECMA doesn't treat a string beginning with a '0' as
+     * an octal number here. This works because all such numbers will
+     * be interpreted as decimal by js_strtod.  Also, any hex numbers
+     * that have made it here (which can only be negative ones) will
+     * be treated as 0 without consuming the 'x' by js_strtod.
+     */
+    if (!js_strtod(cx, bp, end, &ep, &d) ||
+        js_SkipWhiteSpace(ep, end) != end) {
+        return NumberTraits<T>::NaN();
+    }
+
+    return NumberTraits<T>::toSelfType(d);
+}
+}
+
 #endif /* jsnum_h___ */
new file mode 100644
--- /dev/null
+++ b/js/src/trace-test/tests/basic/testStringToNumber.js
@@ -0,0 +1,47 @@
+function convertToInt(str) {
+    return str | 0;
+}
+
+function convertToIntOnTrace(str) {
+    var z;
+    for (var i = 0; i < RUNLOOP; ++i) {
+        z = str | 0;
+    }
+    return z;
+}
+
+function convertToDouble(str) {
+    return str * 1.5;
+}
+
+function convertToDoubleOnTrace(str) {
+    var z;
+    for (var i = 0; i < RUNLOOP; ++i) {
+        z = str * 1.5;
+    }
+    return z;
+}
+
+assertEq(convertToInt("0x10"), 16);
+assertEq(convertToInt("-0x10"), 0);
+
+assertEq(convertToIntOnTrace("0x10"), 16);
+checkStats({
+        traceTriggered: 1
+});
+assertEq(convertToIntOnTrace("-0x10"), 0);
+checkStats({
+        traceTriggered: 2
+});
+
+assertEq(convertToDouble("0x10"), 24);
+assertEq(convertToDouble("-0x10"), NaN);
+
+assertEq(convertToDoubleOnTrace("0x10"), 24);
+checkStats({
+        traceTriggered: 3
+});
+assertEq(convertToDoubleOnTrace("-0x10"), NaN);
+checkStats({
+        traceTriggered: 4
+});