Bug 921548 - Remove js_ValueToObjectOrNull -- people should use ToObject or explicit null/undefined tests, for readability. r=jorendorff
☠☠ backed out by 868f37d83a73 ☠ ☠
authorJeff Walden <jwalden@mit.edu>
Fri, 27 Sep 2013 11:19:47 -0700
changeset 149447 0a76209b85d2015bb94ae1a0d541b2ac4da0b96d
parent 149446 28eab6ee74256dba0dc5f47c36c20f872ad2013a
child 149448 eeff752012bd40e13ddfbfab1a5def4a0fce5271
push id25389
push userryanvm@gmail.com
push dateTue, 01 Oct 2013 20:35:30 +0000
treeherdermozilla-central@4364824a4cab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs921548
milestone27.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 921548 - Remove js_ValueToObjectOrNull -- people should use ToObject or explicit null/undefined tests, for readability. r=jorendorff
js/src/builtin/Object.cpp
js/src/jsapi.cpp
js/src/jsiter.cpp
js/src/jsobj.cpp
js/src/jsobj.h
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -25,24 +25,22 @@ using mozilla::ArrayLength;
 
 
 bool
 js::obj_construct(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     RootedObject obj(cx, nullptr);
-    if (args.length() > 0) {
-        /* If argv[0] is null or undefined, obj comes back null. */
-        if (!js_ValueToObjectOrNull(cx, args[0], &obj))
+    if (args.length() > 0 && !args[0].isNullOrUndefined()) {
+        obj = ToObject(cx, args[0]);
+        if (!obj)
             return false;
-    }
-    if (!obj) {
+    } else {
         /* Make an object whether this was called with 'new' or not. */
-        JS_ASSERT(!args.length() || args[0].isNullOrUndefined());
         if (!NewObjectScriptedCall(cx, &obj))
             return false;
     }
 
     args.rval().setObject(*obj);
     return true;
 }
 
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -280,19 +280,25 @@ JS_ConvertArgumentsVA(JSContext *cx, uns
                 if (!stable)
                     return false;
                 *va_arg(ap, const jschar **) = stable->chars().get();
             } else {
                 *va_arg(ap, JSString **) = str;
             }
             break;
           case 'o':
-            if (!js_ValueToObjectOrNull(cx, *sp, &obj))
-                return false;
-            *sp = OBJECT_TO_JSVAL(obj);
+            if (sp->isNullOrUndefined()) {
+                obj = nullptr;
+            } else {
+                RootedValue v(cx, *sp);
+                obj = ToObject(cx, v);
+                if (!obj)
+                    return false;
+            }
+            *sp = ObjectOrNullValue(obj);
             *va_arg(ap, JSObject **) = obj;
             break;
           case 'f':
             obj = ReportIfNotFunction(cx, *sp);
             if (!obj)
                 return false;
             *sp = OBJECT_TO_JSVAL(obj);
             *va_arg(ap, JSFunction **) = &obj->as<JSFunction>();
@@ -323,19 +329,24 @@ JS_ConvertValue(JSContext *cx, HandleVal
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, value);
     switch (type) {
       case JSTYPE_VOID:
         vp.setUndefined();
         ok = true;
         break;
       case JSTYPE_OBJECT:
-        ok = js_ValueToObjectOrNull(cx, value, &obj);
-        if (ok)
-            vp.setObjectOrNull(obj);
+        if (value.isNullOrUndefined()) {
+            obj.set(nullptr);
+        } else {
+            obj = ToObject(cx, value);
+            if (!obj)
+                return false;
+        }
+        ok = true;
         break;
       case JSTYPE_FUNCTION:
         vp.set(value);
         obj = ReportIfNotFunction(cx, vp);
         ok = (obj != NULL);
         break;
       case JSTYPE_STRING:
         str = ToString<CanGC>(cx, value);
@@ -363,17 +374,25 @@ JS_ConvertValue(JSContext *cx, HandleVal
 }
 
 JS_PUBLIC_API(bool)
 JS_ValueToObject(JSContext *cx, HandleValue value, MutableHandleObject objp)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, value);
-    return js_ValueToObjectOrNull(cx, value, objp);
+    if (value.isNullOrUndefined()) {
+        objp.set(nullptr);
+        return true;
+    }
+    JSObject *obj = ToObject(cx, value);
+    if (!obj)
+        return false;
+    objp.set(obj);
+    return true;
 }
 
 JS_PUBLIC_API(JSFunction *)
 JS_ValueToFunction(JSContext *cx, HandleValue value)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, value);
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -969,27 +969,21 @@ js::ValueToIterator(JSContext *cx, unsig
     cx->iterValue.setMagic(JS_NO_ITER_VALUE);
 
     RootedObject obj(cx);
     if (vp.isObject()) {
         /* Common case. */
         obj = &vp.toObject();
     } else {
         /*
-         * Enumerating over null and undefined gives an empty enumerator.
-         * This is contrary to ECMA-262 9.9 ToObject, invoked from step 3 of
-         * the first production in 12.6.4 and step 4 of the second production,
-         * but it's "web JS" compatible. ES5 fixed for-in to match this de-facto
-         * standard.
+         * Enumerating over null and undefined gives an empty enumerator, so
+         * that |for (var p in <null or undefined>) <loop>;| never executes
+         * <loop>, per ES5 12.6.4.
          */
-        if (flags & JSITER_ENUMERATE) {
-            if (!js_ValueToObjectOrNull(cx, vp, &obj))
-                return false;
-            /* fall through */
-        } else {
+        if (!(flags & JSITER_ENUMERATE) || !vp.isNullOrUndefined()) {
             obj = ToObject(cx, vp);
             if (!obj)
                 return false;
         }
     }
 
     return GetIterator(cx, obj, flags, vp);
 }
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -5142,34 +5142,16 @@ js::PrimitiveToObject(JSContext *cx, con
     }
     if (v.isNumber())
         return NumberObject::create(cx, v.toNumber());
 
     JS_ASSERT(v.isBoolean());
     return BooleanObject::create(cx, v.toBoolean());
 }
 
-bool
-js_ValueToObjectOrNull(JSContext *cx, const Value &v, MutableHandleObject objp)
-{
-    JSObject *obj;
-
-    if (v.isObjectOrNull()) {
-        obj = v.toObjectOrNull();
-    } else if (v.isUndefined()) {
-        obj = NULL;
-    } else {
-        obj = PrimitiveToObject(cx, v);
-        if (!obj)
-            return false;
-    }
-    objp.set(obj);
-    return true;
-}
-
 /* Callers must handle the already-object case . */
 JSObject *
 js::ToObjectSlow(JSContext *cx, HandleValue val, bool reportScanStack)
 {
     JS_ASSERT(!val.isMagic());
     JS_ASSERT(!val.isObject());
 
     if (val.isNullOrUndefined()) {
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1460,19 +1460,16 @@ bool
 GetObjectElementOperationPure(ThreadSafeContext *cx, JSObject *obj, const Value &prop, Value *vp);
 
 /* Wrap boolean, number or string as Boolean, Number or String object. */
 extern JSObject *
 PrimitiveToObject(JSContext *cx, const Value &v);
 
 } /* namespace js */
 
-extern bool
-js_ValueToObjectOrNull(JSContext *cx, const js::Value &v, JS::MutableHandleObject objp);
-
 namespace js {
 
 /*
  * Invokes the ES5 ToObject algorithm on vp, returning the result. If vp might
  * already be an object, use ToObject. reportCantConvert controls how null and
  * undefined errors are reported.
  */
 extern JSObject *