Bug 700169 - Refactor code to use StringBuffer. r=Waldo
authorTom Schuster <evilpies@gmail.com>
Tue, 06 Dec 2011 11:31:00 +0100
changeset 82065 35938622cde018ad91abafd1853c44665c6aea06
parent 81497 bf8259fcab61b5966835b7cfc21ed5565c42fcfc
child 82066 41e3dd30ad8f49ddc997f65f679bbfbed10d57d3
push id21582
push userbmo@edmorley.co.uk
push dateWed, 07 Dec 2011 09:30:09 +0000
treeherdermozilla-central@489f2d51b011 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs700169
milestone11.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 700169 - Refactor code to use StringBuffer. r=Waldo
js/src/jit-test/tests/basic/error-toString.js
js/src/jsbool.cpp
js/src/jsdate.cpp
js/src/jsexn.cpp
js/src/jsnum.cpp
js/src/jsobj.cpp
js/src/jsstr.cpp
js/src/tests/js1_5/GC/regress-348532.js
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/error-toString.js
@@ -0,0 +1,9 @@
+var errorToString = Error.prototype.toString;
+
+
+assertEq(errorToString.call({message: "", name: ""}), "Error");
+assertEq(errorToString.call({name: undefined, message: ""}), "Error");
+assertEq(errorToString.call({name: "Test", message: undefined}), "Test");
+assertEq(errorToString.call({name: "Test", message: ""}), "Test");
+assertEq(errorToString.call({name: "", message: "Test"}), "Test");
+assertEq(errorToString.call({name: "Test", message: "it!"}), "Test: it!");
--- a/js/src/jsbool.cpp
+++ b/js/src/jsbool.cpp
@@ -71,30 +71,30 @@ Class js::BooleanClass = {
     JS_PropertyStub,         /* getProperty */
     JS_StrictPropertyStub,   /* setProperty */
     JS_EnumerateStub,
     JS_ResolveStub,
     JS_ConvertStub
 };
 
 #if JS_HAS_TOSOURCE
-#include "jsprf.h"
-
 static JSBool
 bool_toSource(JSContext *cx, uintN argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     bool b, ok;
     if (!BoxedPrimitiveMethodGuard(cx, args, bool_toSource, &b, &ok))
         return ok;
 
-    char buf[32];
-    JS_snprintf(buf, sizeof buf, "(new Boolean(%s))", JS_BOOLEAN_STR(b));
-    JSString *str = JS_NewStringCopyZ(cx, buf);
+    StringBuffer sb(cx);
+    if (!sb.append("(new Boolean(") || !BooleanToStringBuffer(cx, b, sb) || !sb.append("))"))
+        return false;
+
+    JSString *str = sb.finishString();
     if (!str)
         return false;
     args.rval().setString(str);
     return true;
 }
 #endif
 
 static JSBool
--- a/js/src/jsdate.cpp
+++ b/js/src/jsdate.cpp
@@ -73,16 +73,17 @@
 #include "jsobj.h"
 #include "jsstr.h"
 #include "jslibmath.h"
 
 #include "vm/GlobalObject.h"
 
 #include "jsinferinlines.h"
 #include "jsobjinlines.h"
+#include "jsstrinlines.h"
 
 #include "vm/Stack-inl.h"
 
 using namespace mozilla;
 using namespace js;
 using namespace js::types;
 
 /*
@@ -2450,46 +2451,34 @@ date_toDateString(JSContext *cx, uintN a
     JSObject *obj = NonGenericMethodGuard(cx, args, date_toDateString, &DateClass, &ok);
     if (!obj)
         return ok;
 
     return date_format(cx, obj->getDateUTCTime().toNumber(), FORMATSPEC_DATE, args);
 }
 
 #if JS_HAS_TOSOURCE
-#include <string.h>
-#include "jsnum.h"
-
 static JSBool
 date_toSource(JSContext *cx, uintN argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     bool ok;
     JSObject *obj = NonGenericMethodGuard(cx, args, date_toSource, &DateClass, &ok);
     if (!obj)
         return ok;
 
-    double utctime = obj->getDateUTCTime().toNumber();
-
-    ToCStringBuf cbuf;
-    char *numStr = NumberToCString(cx, &cbuf, utctime);
-    if (!numStr) {
-        JS_ReportOutOfMemory(cx);
+    StringBuffer sb(cx);
+    if (!sb.append("(new Date(") || !NumberValueToStringBuffer(cx, obj->getDateUTCTime(), sb) ||
+        !sb.append("))"))
+    {
         return false;
     }
 
-    char *bytes = JS_smprintf("(new %s(%s))", js_Date_str, numStr);
-    if (!bytes) {
-        JS_ReportOutOfMemory(cx);
-        return false;
-    }
-
-    JSString *str = JS_NewStringCopyZ(cx, bytes);
-    cx->free_(bytes);
+    JSString *str = sb.finishString();
     if (!str)
         return false;
     args.rval().setString(str);
     return true;
 }
 #endif
 
 static JSBool
--- a/js/src/jsexn.cpp
+++ b/js/src/jsexn.cpp
@@ -771,215 +771,167 @@ Exception(JSContext *cx, uintN argc, Val
     intN exnType = args.callee().toFunction()->getExtendedSlot(0).toInt32();
     if (!InitExnPrivate(cx, obj, message, filename, lineno, NULL, exnType))
         return false;
 
     args.rval().setObject(*obj);
     return true;
 }
 
-/*
- * Convert to string.
- *
- * This method only uses JavaScript-modifiable properties name, message.  It
- * is left to the host to check for private data and report filename and line
- * number information along with this message.
- */
+/* ES5 15.11.4.4 (NB: with subsequent errata). */
 static JSBool
 exn_toString(JSContext *cx, uintN argc, Value *vp)
 {
-    jsval v;
-    JSString *name, *message, *result;
-    jschar *chars, *cp;
-    size_t name_length, message_length, length;
+    CallArgs args = CallArgsFromVp(argc, vp);
 
-    JSObject *obj = ToObject(cx, &vp[1]);
-    if (!obj)
-        return JS_FALSE;
-    if (!obj->getProperty(cx, cx->runtime->atomState.nameAtom, &v))
-        return JS_FALSE;
-    name = JSVAL_IS_STRING(v) ? JSVAL_TO_STRING(v) : cx->runtime->emptyString;
-    vp->setString(name);
+    /* Step 2. */
+    if (!args.thisv().isObject()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_PROTOTYPE, "Error");
+        return false;
+    }
 
-    if (!JS_GetProperty(cx, obj, js_message_str, &v))
-        return JS_FALSE;
-    message = JSVAL_IS_STRING(v) ? JSVAL_TO_STRING(v)
-                                 : cx->runtime->emptyString;
+    /* Step 1. */
+    JSObject &obj = args.thisv().toObject();
 
-    if (message->length() != 0) {
-        name_length = name->length();
-        message_length = message->length();
-        length = (name_length ? name_length + 2 : 0) + message_length;
-        cp = chars = (jschar *) cx->malloc_((length + 1) * sizeof(jschar));
-        if (!chars)
-            return JS_FALSE;
+    /* Step 3. */
+    Value nameVal;
+    if (!obj.getProperty(cx, cx->runtime->atomState.nameAtom, &nameVal))
+        return false;
 
-        if (name_length) {
-            const jschar *name_chars = name->getChars(cx);
-            if (!name_chars)
-                return JS_FALSE;
-            js_strncpy(cp, name_chars, name_length);
-            cp += name_length;
-            *cp++ = ':'; *cp++ = ' ';
-        }
-        const jschar *message_chars = message->getChars(cx);
-        if (!message_chars)
-            return JS_FALSE;
-        js_strncpy(cp, message_chars, message_length);
-        cp += message_length;
-        *cp = 0;
-
-        result = js_NewString(cx, chars, length);
-        if (!result) {
-            cx->free_(chars);
-            return JS_FALSE;
-        }
+    /* Step 4. */
+    JSString *name;
+    if (nameVal.isUndefined()) {
+        name = CLASS_ATOM(cx, Error);
     } else {
-        result = name;
+        name = js_ValueToString(cx, nameVal);
+        if (!name)
+            return false;
     }
 
-    vp->setString(result);
-    return JS_TRUE;
+    /* Step 5. */
+    Value msgVal;
+    if (!obj.getProperty(cx, cx->runtime->atomState.messageAtom, &msgVal))
+        return false;
+
+    /* Step 6. */
+    JSString *message;
+    if (msgVal.isUndefined()) {
+        message = cx->runtime->emptyString;
+    } else {
+        message = js_ValueToString(cx, msgVal);
+        if (!message)
+            return false;
+    }
+
+    /* Step 7. */
+    if (name->empty() && message->empty()) {
+        args.rval().setString(CLASS_ATOM(cx, Error));
+        return true;
+    }
+
+    /* Step 8. */
+    if (name->empty()) {
+        args.rval().setString(message);
+        return true;
+    }
+
+    /* Step 9. */
+    if (message->empty()) {
+        args.rval().setString(name);
+        return true;
+    }
+
+    /* Step 10. */
+    StringBuffer sb(cx);
+    if (!sb.append(name) || !sb.append(": ") || !sb.append(message))
+        return false;
+
+    JSString *str = sb.finishString();
+    if (!str)
+        return false;
+    args.rval().setString(str);
+    return true;
 }
 
 #if JS_HAS_TOSOURCE
 /*
  * Return a string that may eval to something similar to the original object.
  */
 static JSBool
 exn_toSource(JSContext *cx, uintN argc, Value *vp)
 {
-    JSString *name, *message, *filename, *lineno_as_str, *result;
-    jsval localroots[3] = {JSVAL_NULL, JSVAL_NULL, JSVAL_NULL};
-    size_t lineno_length, name_length, message_length, filename_length, length;
-    jschar *chars, *cp;
+    CallArgs args = CallArgsFromVp(argc, vp);
 
     JSObject *obj = ToObject(cx, &vp[1]);
     if (!obj)
         return false;
+
     if (!obj->getProperty(cx, cx->runtime->atomState.nameAtom, vp))
         return false;
-    name = js_ValueToString(cx, *vp);
+
+    JSString *name = js_ValueToString(cx, *vp);
     if (!name)
         return false;
     vp->setString(name);
 
+    Value messageVal;
+    JSString *message;
+    if (!obj->getProperty(cx, cx->runtime->atomState.messageAtom, &messageVal) ||
+        !(message = js_ValueToSource(cx, messageVal)))
     {
-        AutoArrayRooter tvr(cx, ArrayLength(localroots), localroots);
-
-#ifdef __GNUC__
-        message = filename = NULL;
-#endif
-        if (!JS_GetProperty(cx, obj, js_message_str, &localroots[0]) ||
-            !(message = js_ValueToSource(cx, localroots[0]))) {
-            return false;
-        }
-        localroots[0] = STRING_TO_JSVAL(message);
-
-        if (!JS_GetProperty(cx, obj, js_fileName_str, &localroots[1]) ||
-            !(filename = js_ValueToSource(cx, localroots[1]))) {
-            return false;
-        }
-        localroots[1] = STRING_TO_JSVAL(filename);
-
-        if (!JS_GetProperty(cx, obj, js_lineNumber_str, &localroots[2]))
-            return false;
-        uint32_t lineno;
-        if (!ValueToECMAUint32(cx, localroots[2], &lineno))
-            return false;
+        return false;
+    }
 
-        if (lineno != 0) {
-            lineno_as_str = js_ValueToString(cx, localroots[2]);
-            if (!lineno_as_str)
-                return false;
-            lineno_length = lineno_as_str->length();
-        } else {
-            lineno_as_str = NULL;
-            lineno_length = 0;
-        }
-
-        /* Magic 8, for the characters in ``(new ())''. */
-        name_length = name->length();
-        message_length = message->length();
-        length = 8 + name_length + message_length;
+    Value filenameVal;
+    JSString *filename;
+    if (!obj->getProperty(cx, cx->runtime->atomState.fileNameAtom, &filenameVal) ||
+        !(filename = js_ValueToSource(cx, filenameVal)))
+    {
+        return false;
+    }
 
-        filename_length = filename->length();
-        if (filename_length != 0) {
-            /* append filename as ``, {filename}'' */
-            length += 2 + filename_length;
-            if (lineno_as_str) {
-                /* append lineno as ``, {lineno_as_str}'' */
-                length += 2 + lineno_length;
-            }
-        } else {
-            if (lineno_as_str) {
-                /*
-                 * no filename, but have line number,
-                 * need to append ``, "", {lineno_as_str}''
-                 */
-                length += 6 + lineno_length;
-            }
-        }
+    Value linenoVal;
+    uint32_t lineno;
+    if (!obj->getProperty(cx, cx->runtime->atomState.lineNumberAtom, &linenoVal) ||
+        !ValueToECMAUint32(cx, linenoVal, &lineno))
+    {
+        return false;
+    }
+
+    StringBuffer sb(cx);
+    if (!sb.append("(new ") || !sb.append(name) || !sb.append("("))
+        return false;
 
-        cp = chars = (jschar *) cx->malloc_((length + 1) * sizeof(jschar));
-        if (!chars)
-            return false;
+    if (!sb.append(message))
+        return false;
 
-        *cp++ = '('; *cp++ = 'n'; *cp++ = 'e'; *cp++ = 'w'; *cp++ = ' ';
-        const jschar *name_chars = name->getChars(cx);
-        if (!name_chars)
+    if (!filename->empty()) {
+        if (!sb.append(", ") || !sb.append(filename))
             return false;
-        js_strncpy(cp, name_chars, name_length);
-        cp += name_length;
-        *cp++ = '(';
-        const jschar *message_chars = message->getChars(cx);
-        if (!message_chars)
-            return false;
-        if (message_length != 0) {
-            js_strncpy(cp, message_chars, message_length);
-            cp += message_length;
-        }
-
-        if (filename_length != 0) {
-            /* append filename as ``, {filename}'' */
-            *cp++ = ','; *cp++ = ' ';
-            const jschar *filename_chars = filename->getChars(cx);
-            if (!filename_chars)
+    }
+    if (lineno != 0) {
+        /* We have a line, but no filename, add empty string */
+        if (filename->empty() && !sb.append(", \"\""))
                 return false;
-            js_strncpy(cp, filename_chars, filename_length);
-            cp += filename_length;
-        } else {
-            if (lineno_as_str) {
-                /*
-                 * no filename, but have line number,
-                 * need to append ``, "", {lineno_as_str}''
-                 */
-                *cp++ = ','; *cp++ = ' '; *cp++ = '"'; *cp++ = '"';
-            }
-        }
-        if (lineno_as_str) {
-            /* append lineno as ``, {lineno_as_str}'' */
-            *cp++ = ','; *cp++ = ' ';
-            const jschar *lineno_chars = lineno_as_str->getChars(cx);
-            if (!lineno_chars)
-                return false;
-            js_strncpy(cp, lineno_chars, lineno_length);
-            cp += lineno_length;
-        }
+
+        JSString *linenumber = js_ValueToString(cx, linenoVal);
+        if (!linenumber)
+            return false;
+        if (!sb.append(", ") || !sb.append(linenumber))
+            return false;
+    }
 
-        *cp++ = ')'; *cp++ = ')'; *cp = 0;
+    if (!sb.append("))"))
+        return false;
 
-        result = js_NewString(cx, chars, length);
-        if (!result) {
-            cx->free_(chars);
-            return false;
-        }
-        vp->setString(result);
-        return true;
-    }
+    JSString *str = sb.finishString();
+    if (!str)
+        return false;
+    vp->setString(str);
+    return true;
 }
 #endif
 
 static JSFunctionSpec exception_methods[] = {
 #if JS_HAS_TOSOURCE
     JS_FN(js_toSource_str,   exn_toSource,           0,0),
 #endif
     JS_FN(js_toString_str,   exn_toString,           0,0),
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -528,26 +528,24 @@ num_toSource(JSContext *cx, uintN argc, 
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     double d;
     bool ok;
     if (!BoxedPrimitiveMethodGuard(cx, args, num_toSource, &d, &ok))
         return ok;
 
-    ToCStringBuf cbuf;
-    char *numStr = NumberToCString(cx, &cbuf, d);
-    if (!numStr) {
-        JS_ReportOutOfMemory(cx);
+    StringBuffer sb(cx);
+    if (!sb.append("(new Number(") || !NumberValueToStringBuffer(cx, NumberValue(d), sb) ||
+        !sb.append("))"))
+    {
         return false;
     }
 
-    char buf[64];
-    JS_snprintf(buf, sizeof buf, "(new %s(%s))", NumberClass.name, numStr);
-    JSString *str = js_NewStringCopyZ(cx, buf);
+    JSString *str = sb.finishString();
     if (!str)
         return false;
     args.rval().setString(str);
     return true;
 }
 #endif
 
 ToCStringBuf::ToCStringBuf() :dbuf(NULL)
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -80,19 +80,20 @@
 #include "jswrapper.h"
 
 #include "frontend/BytecodeCompiler.h"
 #include "frontend/BytecodeEmitter.h"
 #include "frontend/Parser.h"
 
 #include "jsarrayinlines.h"
 #include "jsinterpinlines.h"
+#include "jsobjinlines.h"
 #include "jsscopeinlines.h"
 #include "jsscriptinlines.h"
-#include "jsobjinlines.h"
+#include "jsstrinlines.h"
 
 #include "vm/BooleanObject-inl.h"
 #include "vm/NumberObject-inl.h"
 #include "vm/StringObject-inl.h"
 
 #if JS_HAS_GENERATORS
 #include "jsiter.h"
 #endif
@@ -838,35 +839,24 @@ obj_toSource(JSContext *cx, uintN argc, 
 namespace js {
 
 JSString *
 obj_toStringHelper(JSContext *cx, JSObject *obj)
 {
     if (obj->isProxy())
         return Proxy::obj_toString(cx, obj);
 
-    const char *clazz = obj->getClass()->name;
-    size_t nchars = 9 + strlen(clazz); /* 9 for "[object ]" */
-    jschar *chars = (jschar *) cx->malloc_((nchars + 1) * sizeof(jschar));
-    if (!chars)
+    StringBuffer sb(cx);
+    const char *className = obj->getClass()->name;
+    if (!sb.append("[object ") || !sb.appendInflated(className, strlen(className)) || 
+        !sb.append("]"))
+    {
         return NULL;
-
-    const char *prefix = "[object ";
-    nchars = 0;
-    while ((chars[nchars] = (jschar)*prefix) != 0)
-        nchars++, prefix++;
-    while ((chars[nchars] = (jschar)*clazz) != 0)
-        nchars++, clazz++;
-    chars[nchars++] = ']';
-    chars[nchars] = 0;
-
-    JSString *str = js_NewString(cx, chars, nchars);
-    if (!str)
-        cx->free_(chars);
-    return str;
+    }
+    return sb.finishString();
 }
 
 JSObject *
 NonNullObject(JSContext *cx, const Value &v)
 {
     if (v.isPrimitive()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_NOT_NONNULL_OBJECT);
         return NULL;
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -465,44 +465,23 @@ str_toSource(JSContext *cx, uintN argc, 
     bool ok;
     if (!BoxedPrimitiveMethodGuard(cx, args, str_toSource, &str, &ok))
         return ok;
 
     str = js_QuoteString(cx, str, '"');
     if (!str)
         return false;
 
-    char buf[16];
-    size_t j = JS_snprintf(buf, sizeof buf, "(new String(");
-
-    JS::Anchor<JSString *> anchor(str);
-    size_t k = str->length();
-    const jschar *s = str->getChars(cx);
-    if (!s)
-        return false;
-
-    size_t n = j + k + 2;
-    jschar *t = (jschar *) cx->malloc_((n + 1) * sizeof(jschar));
-    if (!t)
+    StringBuffer sb(cx);
+    if (!sb.append("(new String(") || !sb.append(str) || !sb.append("))"))
         return false;
 
-    size_t i;
-    for (i = 0; i < j; i++)
-        t[i] = buf[i];
-    for (j = 0; j < k; i++, j++)
-        t[i] = s[j];
-    t[i++] = ')';
-    t[i++] = ')';
-    t[i] = 0;
-
-    str = js_NewString(cx, t, n);
-    if (!str) {
-        cx->free_(t);
+    str = sb.finishString();
+    if (!str)
         return false;
-    }
     args.rval().setString(str);
     return true;
 }
 
 #endif /* JS_HAS_TOSOURCE */
 
 JSBool
 js_str_toString(JSContext *cx, uintN argc, Value *vp)
--- a/js/src/tests/js1_5/GC/regress-348532.js
+++ b/js/src/tests/js1_5/GC/regress-348532.js
@@ -49,17 +49,16 @@ test();
 function test()
 {
   enterFunc ('test');
   printBugNumber(BUGNUMBER);
   printStatus (summary);
 
   expectExitCode(0);
   expectExitCode(3);
-
   actual = 0;
  
   // construct string of 1<<23 characters
   var s = Array((1<<23)+1).join('x');
 
   var recursionDepth = 0;
   function err() {
     try {