Bug 1031632 - Make Map.prototype.set, WeakMap.prototype.set and Set.prototype.add chainable. r=till
authorRishab Arora <ra.rishab@gmail.com>
Thu, 03 Jul 2014 22:55:26 +0530
changeset 192368 aabe2de625c6f372cc75543206be6109f144de29
parent 192367 00a68fd085d5682d3f15877973c2d9c4887825d2
child 192369 123397342cd186ccdc19eec554fb0d0aca5d849f
push id45811
push usertschneidereit@gmail.com
push dateFri, 04 Jul 2014 09:37:40 +0000
treeherdermozilla-inbound@aabe2de625c6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill
bugs1031632
milestone33.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 1031632 - Make Map.prototype.set, WeakMap.prototype.set and Set.prototype.add chainable. r=till
js/src/builtin/MapObject.cpp
js/src/jit-test/tests/basic/testCrossCompartmentTransparency.js
js/src/jit-test/tests/collections/Map-delete.js
js/src/jit-test/tests/collections/Map-get.js
js/src/jit-test/tests/collections/Map-scale.js
js/src/jit-test/tests/collections/Map-set-returns-this.js
js/src/jit-test/tests/collections/Map-surfaces-3.js
js/src/jit-test/tests/collections/Set-add-returns-this.js
js/src/jit-test/tests/collections/Set-scale.js
js/src/jit-test/tests/collections/Set-surfaces-3.js
js/src/jit-test/tests/collections/WeakMap-set-returns-this.js
js/src/jsweakmap.cpp
js/src/tests/js1_8_5/extensions/regress-697515.js
--- a/js/src/builtin/MapObject.cpp
+++ b/js/src/builtin/MapObject.cpp
@@ -1310,17 +1310,17 @@ MapObject::set_impl(JSContext *cx, CallA
     ValueMap &map = extract(args);
     ARG0_KEY(cx, args, key);
     RelocatableValue rval(args.get(1));
     if (!map.put(key, rval)) {
         js_ReportOutOfMemory(cx);
         return false;
     }
     WriteBarrierPost(cx->runtime(), &map, key.get());
-    args.rval().setUndefined();
+    args.rval().set(args.thisv());
     return true;
 }
 
 bool
 MapObject::set(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<MapObject::is, MapObject::set_impl>(cx, args);
@@ -1772,17 +1772,17 @@ SetObject::add_impl(JSContext *cx, CallA
 
     ValueSet &set = extract(args);
     ARG0_KEY(cx, args, key);
     if (!set.put(key)) {
         js_ReportOutOfMemory(cx);
         return false;
     }
     WriteBarrierPost(cx->runtime(), &set, key.get());
-    args.rval().setUndefined();
+    args.rval().set(args.thisv());
     return true;
 }
 
 bool
 SetObject::add(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<SetObject::is, SetObject::add_impl>(cx, args);
--- a/js/src/jit-test/tests/basic/testCrossCompartmentTransparency.js
+++ b/js/src/jit-test/tests/basic/testCrossCompartmentTransparency.js
@@ -78,23 +78,23 @@ test("new RegExp('1')", function(r) asse
 test("new RegExp('1')", function(r) assertEq("a1".replace(r, 'A'), 'aA'));
 test("new RegExp('1')", function(r) assertEq(String("a1b".split(r)), "a,b"));
 test("new RegExp('1')", function(r) assertEq(r, RegExp(r)));
 test("new RegExp('1')", function(r) assertEq(String(new RegExp(r)), String(r)));
 test("new RegExp('1')", function(r) assertEq(String(/x/.compile(r)), String(r)));
 test("new WeakMap()", function(w) WeakMap.prototype.has.call(w, {}));
 test("new WeakMap()", function(w) WeakMap.prototype.get.call(w, {}));
 test("new WeakMap()", function(w) WeakMap.prototype.delete.call(w, {}));
-test("new WeakMap()", function(w) WeakMap.prototype.set.call(w, {}));
+test("new WeakMap()", function(w) WeakMap.prototype.set.call(w, {}).toString());
 test("new Map()", function(w) Map.prototype.has.call(w, {}));
 test("new Map()", function(w) Map.prototype.get.call(w, {}));
 test("new Map()", function(w) Map.prototype.delete.call(w, {}));
-test("new Map()", function(w) Map.prototype.set.call(w, {}));
+test("new Map()", function(w) Map.prototype.set.call(w, {}).toString());
 test("new Set()", function(w) Set.prototype.has.call(w, {}));
-test("new Set()", function(w) Set.prototype.add.call(w, {}));
+test("new Set()", function(w) Set.prototype.add.call(w, {}).toString());
 test("new Set()", function(w) Set.prototype.delete.call(w, {}));
 
 test("new Int8Array(1)", function(a) Int8Array.prototype.subarray.call(a).toString());
 test("new Uint8Array(1)", function(a) Uint8Array.prototype.subarray.call(a).toString());
 test("new Int16Array(1)", function(a) Int16Array.prototype.subarray.call(a).toString());
 test("new Uint16Array(1)", function(a) Uint16Array.prototype.subarray.call(a).toString());
 test("new Int32Array(1)", function(a) Int32Array.prototype.subarray.call(a).toString());
 test("new Uint32Array(1)", function(a) Uint32Array.prototype.subarray.call(a).toString());
--- a/js/src/jit-test/tests/collections/Map-delete.js
+++ b/js/src/jit-test/tests/collections/Map-delete.js
@@ -3,16 +3,16 @@
 var m = new Map;
 var key = {};
 
 // when the map is new
 assertEq(m.delete(key), false);
 assertEq(m.has(key), false);
 
 // when the key is present
-assertEq(m.set(key, 'x'), undefined);
+assertEq(m.set(key, 'x'), m);
 assertEq(m.delete(key), true);
 assertEq(m.has(key), false);
 assertEq(m.get(key), undefined);
 
 // when the key has already been deleted
 assertEq(m.delete(key), false);
 assertEq(m.has(key), false);
--- a/js/src/jit-test/tests/collections/Map-get.js
+++ b/js/src/jit-test/tests/collections/Map-get.js
@@ -20,17 +20,17 @@ var keys = [undefined, null, true, false
 for (var i = 0; i < keys.length; i++) {
     // without being set
     var key = keys[i];
     assertEq(m.has(key), false);
     assertEq(m.get(key), undefined);
 
     // after being set
     var v = {};
-    assertEq(m.set(key, v), undefined);
+    assertEq(m.set(key, v), m);
     assertEq(m.has(key), true);
     assertEq(m.get(key), v);
 
     // after being deleted
     assertEq(m.delete(key), true);
     assertEq(m.has(key), false);
     assertEq(m.get(key), undefined);
 
--- a/js/src/jit-test/tests/collections/Map-scale.js
+++ b/js/src/jit-test/tests/collections/Map-scale.js
@@ -1,8 +1,8 @@
 // Maps can hold at least 64K values.
 
 var N = 1 << 16;
 var m = new Map;
 for (var i = 0; i < N; i++)
-    assertEq(m.set(i, i), undefined);
+    assertEq(m.set(i, i), m);
 for (var i = 0; i < N; i++)
     assertEq(m.get(i), i);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-set-returns-this.js
@@ -0,0 +1,7 @@
+// Bug 1031632 - Map.prototype.set, WeakMap.prototype.set and
+// Set.prototype.add should be chainable
+
+var m = new Map();
+assertEq(m.set('oof', 'RAB'), m);
+var a = m.set('foo', 'BAR').get('foo');
+assertEq(a, 'BAR');
--- a/js/src/jit-test/tests/collections/Map-surfaces-3.js
+++ b/js/src/jit-test/tests/collections/Map-surfaces-3.js
@@ -1,14 +1,14 @@
 // Map methods work when arguments are omitted.
 
 var m = new Map;
 assertEq(m.has(), false);
 assertEq(m.get(), undefined);
 assertEq(m.delete(), false);
 assertEq(m.has(), false);
 assertEq(m.get(), undefined);
-assertEq(m.set(), undefined);
+assertEq(m.set(), m);
 assertEq(m.has(), true);
 assertEq(m.get(), undefined);
 assertEq(m.delete(), true);
 assertEq(m.has(), false);
 assertEq(m.get(), undefined);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Set-add-returns-this.js
@@ -0,0 +1,7 @@
+// Bug 1031632 - Map.prototype.set, WeakMap.prototype.set and
+// Set.prototype.add should be chainable
+
+var s = new Set();
+assertEq(s.add('BAR'), s);
+var b = s.add('foo').has('foo');
+assertEq(b, true);
--- a/js/src/jit-test/tests/collections/Set-scale.js
+++ b/js/src/jit-test/tests/collections/Set-scale.js
@@ -1,8 +1,8 @@
 // Sets can hold at least 64K values.
 
 var N = 1 << 16;
 var s = new Set;
 for (var i = 0; i < N; i++)
-    assertEq(s.add(i), undefined);
+    assertEq(s.add(i), s);
 for (var i = 0; i < N; i++)
     assertEq(s.has(i), true);
--- a/js/src/jit-test/tests/collections/Set-surfaces-3.js
+++ b/js/src/jit-test/tests/collections/Set-surfaces-3.js
@@ -1,10 +1,10 @@
 // Set methods work when arguments are omitted.
 
 var s = new Set;
 assertEq(s.has(), false);
 assertEq(s.delete(), false);
 assertEq(s.has(), false);
-assertEq(s.add(), undefined);
+assertEq(s.add(), s);
 assertEq(s.has(), true);
 assertEq(s.delete(), true);
 assertEq(s.has(), false);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-set-returns-this.js
@@ -0,0 +1,9 @@
+// Bug 1031632 - Map.prototype.set, WeakMap.prototype.set and
+// Set.prototype.add should be chainable
+
+var wm = new WeakMap();
+var bar = {};
+assertEq(wm.set(bar, 'BAR'), wm);
+var foo = {};
+var a = wm.set(foo, 'FOO').get(foo);
+assertEq(a, 'FOO');
--- a/js/src/jsweakmap.cpp
+++ b/js/src/jsweakmap.cpp
@@ -415,18 +415,20 @@ WeakMap_set_impl(JSContext *cx, CallArgs
     RootedObject key(cx, GetKeyArg(cx, args));
     if (!key)
         return false;
 
     RootedValue value(cx, (args.length() > 1) ? args[1] : UndefinedValue());
     Rooted<JSObject*> thisObj(cx, &args.thisv().toObject());
     Rooted<WeakMapObject*> map(cx, &thisObj->as<WeakMapObject>());
 
-    args.rval().setUndefined();
-    return SetWeakMapEntryInternal(cx, map, key, value);
+    if (!SetWeakMapEntryInternal(cx, map, key, value))
+        return false;
+    args.rval().set(args.thisv());
+    return true;
 }
 
 static bool
 WeakMap_set(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<IsWeakMap, WeakMap_set_impl>(cx, args);
 }
deleted file mode 100644
--- a/js/src/tests/js1_8_5/extensions/regress-697515.js
+++ /dev/null
@@ -1,6 +0,0 @@
-// Any copyright is dedicated to the Public Domain.
-// http://creativecommons.org/licenses/publicdomain/
-
-assertEq(new WeakMap().set({}, 0), undefined);
-
-reportCompare(0, 0, 'ok');