Bug 1155900 - Pass destructuring right-hand-side expressions through ToObject before properties are destructured out of them. r=shu
☠☠ backed out by c119270176bb ☠ ☠
authorJeff Walden <jwalden@mit.edu>
Fri, 17 Apr 2015 21:57:50 -0700
changeset 273331 5d7b3b8cce5ab70e7f6280e9b46ffd2e413994ab
parent 273330 2e212218ba40e9d842fcee6b64364b4ddde2b6b2
child 273332 195a3736c87724bfc4ae4a4a932b2cb40734b736
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1155900
milestone40.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 1155900 - Pass destructuring right-hand-side expressions through ToObject before properties are destructured out of them. r=shu
browser/devtools/netmonitor/test/browser_net_charts-01.js
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/BytecodeEmitter.h
js/src/jit-test/tests/basic/destructuring-default.js
js/src/jit-test/tests/basic/destructuring-toobject.js
js/src/jit-test/tests/basic/expression-autopsy.js
js/src/tests/js1_8_5/regress/regress-592202-1.js
js/src/vm/CommonPropertyNames.h
--- a/browser/devtools/netmonitor/test/browser_net_charts-01.js
+++ b/browser/devtools/netmonitor/test/browser_net_charts-01.js
@@ -1,17 +1,17 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 ///////////////////
 //
 // Whitelisting this test.
 // As part of bug 1077403, the leaking uncaught rejection should be fixed.
 //
-thisTestLeaksUncaughtRejectionsAndShouldBeFixed("TypeError: aValue.content is undefined");
+thisTestLeaksUncaughtRejectionsAndShouldBeFixed("TypeError: can't convert undefined to object");
 
 /**
  * Makes sure Pie Charts have the right internal structure.
  */
 
 function test() {
   initNetMonitor(SIMPLE_URL).then(([aTab, aDebuggee, aMonitor]) => {
     info("Starting test... ");
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -3721,17 +3721,20 @@ BytecodeEmitter::emitDestructuringOpsArr
 }
 
 bool
 BytecodeEmitter::emitDestructuringOpsObjectHelper(ParseNode* pattern, VarEmitOption emitOption)
 {
     MOZ_ASSERT(pattern->isKind(PNK_OBJECT));
     MOZ_ASSERT(pattern->isArity(PN_LIST));
 
-    MOZ_ASSERT(this->stackDepth != 0);                            // ... OBJ
+    MOZ_ASSERT(this->stackDepth != 0);                            // ... RHS
+
+    if (!emitToObject())                                          // ... OBJ
+        return false;
 
     for (ParseNode* member = pattern->pn_head; member; member = member->pn_next) {
         // Duplicate the value being destructured to use as a reference base.
         if (!emit1(JSOP_DUP))                                     // ... OBJ OBJ
             return false;
 
         // Now push the property name currently being matched, which is the
         // current property name "label" on the left of a colon in the object
@@ -4948,16 +4951,31 @@ BytecodeEmitter::emitWith(ParseNode* pn)
     if (!emitTree(pn->pn_right))
         return false;
     if (!leaveNestedScope(&stmtInfo))
         return false;
     return true;
 }
 
 bool
+BytecodeEmitter::emitToObject()
+{
+    if (!emitAtomOp(cx->names().ToObject, JSOP_GETINTRINSIC)) // VAL TOOBJECT
+        return false;
+    if (!emit1(JSOP_UNDEFINED))                               // VAL TOOBJECT UNDEFINED
+        return false;
+    if (!emit2(JSOP_PICK, (jsbytecode)2))                     // TOOBJECT UNDEFINED VAL
+        return false;
+    if (!emitCall(JSOP_CALL, 1))                              // OBJ
+        return false;
+    checkTypeSet(JSOP_CALL);
+    return true;
+}
+
+bool
 BytecodeEmitter::emitIterator()
 {
     // Convert iterable to iterator.
     if (!emit1(JSOP_DUP))                                 // OBJ OBJ
         return false;
     if (!emit2(JSOP_SYMBOL, jsbytecode(JS::SymbolCode::iterator))) // OBJ OBJ @@ITERATOR
         return false;
     if (!emitElemOpBase(JSOP_CALLELEM))                   // OBJ ITERFN
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -519,16 +519,19 @@ struct BytecodeEmitter
     bool emitDestructuringDeclsWithEmitter(JSOp prologueOp, ParseNode* pattern);
 
     bool emitDestructuringDecls(JSOp prologueOp, ParseNode* pattern);
 
     // Emit code to initialize all destructured names to the value on the top of
     // the stack.
     bool emitInitializeDestructuringDecls(JSOp prologueOp, ParseNode* pattern);
 
+    // Convert the value atop the stack to an object using ToObject.
+    bool emitToObject();
+
     // emitIterator expects the iterable to already be on the stack.
     // It will replace that stack value with the corresponding iterator
     bool emitIterator();
 
     // Pops iterator from the top of the stack. Pushes the result of |.next()|
     // onto the stack.
     bool emitIteratorNext(ParseNode* pn);
 
--- a/js/src/jit-test/tests/basic/destructuring-default.js
+++ b/js/src/jit-test/tests/basic/destructuring-default.js
@@ -152,17 +152,17 @@ assertEq(a, 2);
 // assignment to properties of default params
 [a = {y: 2}, a.x = 1] = [];
 assertEq(typeof a, 'object');
 assertEq(a.x, 1);
 assertEq(a.y, 2);
 
 // defaults are evaluated even if there is no binding
 var evaled = false;
-({a: {} = (evaled = true, null)}) = {};
+({a: {} = (evaled = true, {})}) = {};
 assertEq(evaled, true);
 evaled = false;
 assertThrowsInstanceOf(() => { [[] = (evaled = true, 2)] = [] }, TypeError);
 assertEq(evaled, true);
 
 assertThrowsInstanceOf(() => new Function('var [...rest = defaults] = [];'), SyntaxError);
 
 // bug 1124480: destructuring defaults should work correctly as part of for-in
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/destructuring-toobject.js
@@ -0,0 +1,36 @@
+load(libdir + 'asserts.js');
+load(libdir + 'iteration.js');
+
+function f(v)
+{
+  if (v + "")
+    ({} = v);
+}
+
+f(true);
+f({});
+assertThrowsInstanceOf(() => f(null), TypeError);
+assertThrowsInstanceOf(() => f(undefined), TypeError);
+
+function g(v)
+{
+  if (v + "")
+    ({} = v);
+}
+
+g(true);
+g({});
+assertThrowsInstanceOf(() => g(undefined), TypeError);
+assertThrowsInstanceOf(() => g(null), TypeError);
+
+function h(v)
+{
+  if (v + "")
+    ([] = v);
+}
+
+h([true]);
+h("foo");
+assertThrowsInstanceOf(() => h(undefined), TypeError);
+assertThrowsInstanceOf(() => h(null), TypeError);
+
--- a/js/src/jit-test/tests/basic/expression-autopsy.js
+++ b/js/src/jit-test/tests/basic/expression-autopsy.js
@@ -106,17 +106,32 @@ check("o[!(o)]");
 check("o[~(o)]");
 check("o[+ (o)]");
 check("o[- (o)]");
 
 
 // A few one off tests
 check_one("6", (function () { 6() }), " is not a function");
 check_one("0", (function () { Array.prototype.reverse.call('123'); }), " is read-only");
-check_one(`(intermediate value)[Symbol.iterator](...).next(...).value`,
-          function () { var [{ x }] = [null, {}]; }, " is null");
-check_one(`(intermediate value)[Symbol.iterator](...).next(...).value`,
+check_one("(intermediate value)[Symbol.iterator](...).next(...).value",
           function () { ieval("let (x) { var [a, b, [c0, c1]] = [x, x, x]; }") }, " is undefined");
 check_one("void 1", function() { (void 1)(); }, " is not a function");
 check_one("void o[1]", function() { var o = []; (void o[1])() }, " is not a function");
 
+// Manual testing for this case: the only way to trigger an error is *not* on
+// an attempted property access during destructuring, and the error message
+// invoking ToObject(null) is different: "can't convert {0} to object".
+try
+{
+  (function() {
+    var [{x}] = [null, {}];
+   })();
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "expected TypeError, got " + e);
+  assertEq(e.message, "can't convert null to object");
+}
+
 // Check fallback behavior
 assertThrowsInstanceOf(function () { for (let x of undefined) {} }, TypeError);
--- a/js/src/tests/js1_8_5/regress/regress-592202-1.js
+++ b/js/src/tests/js1_8_5/regress/regress-592202-1.js
@@ -1,7 +1,15 @@
 /*
  * Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/licenses/publicdomain/
  */
-i = 42
-eval("let(y){(function(){let({}=y){(function(){let({}=y=[])(i)})()}})()}")
+i = 42;
+eval("let (y) { \n" +
+     "  (function() { \n" +
+     "    let ({} = (y, {})) { \n" +
+     "      (function() { \n" +
+     "         let ({} = y = []) (i); \n" +
+     "       })(); \n" +
+     "    } \n" +
+     "   })(); \n" +
+     "}");
 reportCompare(0, 0, "ok");
--- a/js/src/vm/CommonPropertyNames.h
+++ b/js/src/vm/CommonPropertyNames.h
@@ -196,16 +196,17 @@
     macro(test, test, "test") \
     macro(throw, throw_, "throw") \
     macro(timestamp, timestamp, "timestamp") \
     macro(timeZone, timeZone, "timeZone") \
     macro(toGMTString, toGMTString, "toGMTString") \
     macro(toISOString, toISOString, "toISOString") \
     macro(toJSON, toJSON, "toJSON") \
     macro(toLocaleString, toLocaleString, "toLocaleString") \
+    macro(ToObject, ToObject, "ToObject") \
     macro(toSource, toSource, "toSource") \
     macro(toString, toString, "toString") \
     macro(toUTCString, toUTCString, "toUTCString") \
     macro(true, true_, "true") \
     macro(unescape, unescape, "unescape") \
     macro(uneval, uneval, "uneval") \
     macro(unicode, unicode, "unicode") \
     macro(uninitialized, uninitialized, "uninitialized") \