Fix destructuring binding to follow the cheezy dominance relation rules of the upvar analysis (496134, r=mrbkap; take 2).
authorBrendan Eich <brendan@mozilla.org>
Mon, 08 Jun 2009 13:03:50 -0700
changeset 28995 1965e3b7a40693f4092d8b82131ee17b9a57daa8
parent 28994 594138cd96e33f84173907066800de071c8ea02a
child 28996 2b57dcfdb13db2c3ec3ea26a6ed73a9701f1e40a
push id7365
push userrsayre@mozilla.com
push dateTue, 09 Jun 2009 02:39:30 +0000
treeherderautoland@66a40d5fda11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap, take
bugs496134
milestone1.9.2a1pre
Fix destructuring binding to follow the cheezy dominance relation rules of the upvar analysis (496134, r=mrbkap; take 2).
js/src/jsparse.cpp
--- a/js/src/jsparse.cpp
+++ b/js/src/jsparse.cpp
@@ -3490,20 +3490,21 @@ typedef struct FindPropValData {
 
 typedef struct FindPropValEntry {
     JSDHashEntryHdr hdr;
     JSParseNode     *pnkey;
     JSParseNode     *pnval;
 } FindPropValEntry;
 
 #define ASSERT_VALID_PROPERTY_KEY(pnkey)                                      \
-    JS_ASSERT((pnkey)->pn_arity == PN_NULLARY &&                              \
-              ((pnkey)->pn_type == TOK_NUMBER ||                              \
-               (pnkey)->pn_type == TOK_STRING ||                              \
-               (pnkey)->pn_type == TOK_NAME))
+    JS_ASSERT(((pnkey)->pn_arity == PN_NULLARY &&                             \
+               ((pnkey)->pn_type == TOK_NUMBER ||                             \
+                (pnkey)->pn_type == TOK_STRING ||                             \
+                (pnkey)->pn_type == TOK_NAME)) ||                             \
+               (pnkey)->pn_arity == PN_NAME && (pnkey)->pn_type == TOK_NAME)
 
 static JSDHashNumber
 HashFindPropValKey(JSDHashTable *table, const void *key)
 {
     const JSParseNode *pnkey = (const JSParseNode *)key;
 
     ASSERT_VALID_PROPERTY_KEY(pnkey);
     return (pnkey->pn_type == TOK_NUMBER)
@@ -3621,16 +3622,19 @@ FindPropertyValue(JSParseNode *pn, JSPar
     }
     return pnhit->pn_right;
 }
 
 /*
  * If data is null, the caller is AssignExpr and instead of binding variables,
  * we specialize lvalues in the propery value positions of the left-hand side.
  * If right is null, just check for well-formed lvalues.
+ *
+ * See also UndominateInitializers, immediately below. If you change either of
+ * these functions, you might have to change the other to match.
  */
 static JSBool
 CheckDestructuring(JSContext *cx, BindData *data,
                    JSParseNode *left, JSParseNode *right,
                    JSTreeContext *tc)
 {
     JSBool ok;
     FindPropValData fpvd;
@@ -3773,16 +3777,96 @@ CheckDestructuring(JSContext *cx, BindDa
 
   no_var_name:
     js_ReportCompileErrorNumber(cx, TS(tc->compiler), pn, JSREPORT_ERROR,
                                 JSMSG_NO_VARIABLE_NAME);
     ok = JS_FALSE;
     goto out;
 }
 
+/*
+ * This is a greatly pared down version of CheckDestructuring that extends the
+ * pn_pos.end source coordinate of each name in a destructuring binding such as
+ * 
+ *   var [x, y] = [function () y, 42];
+ *
+ * to cover its corresponding initializer, so that the initialized binding does
+ * not appear to dominate any closures in its initializer. See bug 496134.
+ *
+ * The quick-and-dirty dominance computation in JSCompiler::setFunctionKinds is
+ * not very precise. With one-pass SSA construction from structured source code
+ * (see "Single-Pass Generation of Static Single Assignment Form for Structured
+ * Languages", Brandis and Mössenböck), we could do much better.
+ *
+ * See CheckDestructuring, immediately above. If you change either of these
+ * functions, you might have to change the other to match.
+ */
+static JSBool
+UndominateInitializers(JSParseNode *left, JSParseNode *right, JSTreeContext *tc)
+{
+    FindPropValData fpvd;
+    JSParseNode *lhs, *rhs;
+
+    JS_ASSERT(left->pn_type != TOK_ARRAYCOMP);
+    JS_ASSERT(right);
+
+#if JS_HAS_DESTRUCTURING_SHORTHAND
+    if (right->pn_arity == PN_LIST && (right->pn_xflags & PNX_DESTRUCT)) {
+        js_ReportCompileErrorNumber(tc->compiler->context, TS(tc->compiler), right,
+                                    JSREPORT_ERROR, JSMSG_BAD_OBJECT_INIT);
+        return JS_FALSE;
+    }
+#endif
+
+    if (right->pn_type != left->pn_type)
+        return JS_TRUE;
+
+    fpvd.table.ops = NULL;
+    lhs = left->pn_head;
+    if (left->pn_type == TOK_RB) {
+        rhs = right->pn_head;
+
+        while (lhs && rhs) {
+            /* Nullary comma is an elision; binary comma is an expression.*/
+            if (lhs->pn_type != TOK_COMMA || lhs->pn_arity != PN_NULLARY) {
+                if (lhs->pn_type == TOK_RB || lhs->pn_type == TOK_RC) {
+                    if (!UndominateInitializers(lhs, rhs, tc))
+                        return JS_FALSE;
+                } else {
+                    lhs->pn_pos.end = rhs->pn_pos.end;
+                }
+            }
+
+            lhs = lhs->pn_next;
+            rhs = rhs->pn_next;
+        }
+    } else {
+        JS_ASSERT(left->pn_type == TOK_RC);
+        fpvd.numvars = left->pn_count;
+        fpvd.maxstep = 0;
+
+        while (lhs) {
+            JS_ASSERT(lhs->pn_type == TOK_COLON);
+            JSParseNode *pn = lhs->pn_right;
+
+            rhs = FindPropertyValue(right, lhs->pn_left, &fpvd);
+            if (pn->pn_type == TOK_RB || pn->pn_type == TOK_RC) {
+                if (rhs && !UndominateInitializers(pn, rhs, tc))
+                    return JS_FALSE;
+            } else {
+                if (rhs)
+                    pn->pn_pos.end = rhs->pn_pos.end;
+            }
+
+            lhs = lhs->pn_next;
+        }
+    }
+    return JS_TRUE;
+}
+
 static JSParseNode *
 DestructuringExpr(JSContext *cx, BindData *data, JSTreeContext *tc,
                   JSTokenType tt)
 {
     JSTokenStream *ts;
     JSParseNode *pn;
 
     ts = TS(tc->compiler);
@@ -5483,20 +5567,20 @@ Variables(JSContext *cx, JSTokenStream *
 #if JS_HAS_DESTRUCTURING
         if (tt == TOK_LB || tt == TOK_LC) {
             ts->flags |= TSF_DESTRUCTURING;
             pn2 = PrimaryExpr(cx, ts, tc, tt, JS_FALSE);
             ts->flags &= ~TSF_DESTRUCTURING;
             if (!pn2)
                 return NULL;
 
+            if (!CheckDestructuring(cx, &data, pn2, NULL, tc))
+                return NULL;
             if ((tc->flags & TCF_IN_FOR_INIT) &&
                 js_PeekToken(cx, ts) == TOK_IN) {
-                if (!CheckDestructuring(cx, &data, pn2, NULL, tc))
-                    return NULL;
                 pn->append(pn2);
                 continue;
             }
 
             MUST_MATCH_TOKEN(TOK_ASSIGN, JSMSG_BAD_DESTRUCT_DECL);
             if (CURRENT_TOKEN(ts).t_op != JSOP_NOP)
                 goto bad_var_init;
 
@@ -5509,23 +5593,22 @@ Variables(JSContext *cx, JSTokenStream *
             JSParseNode *init = AssignExpr(cx, ts, tc);
 #if JS_HAS_BLOCK_SCOPE
             if (popScope) {
                 tc->topStmt = save;
                 tc->topScopeStmt = saveScope;
             }
 #endif
 
+            if (!init || !UndominateInitializers(pn2, init, tc))
+                return NULL;
+
             pn2 = NewBinary(TOK_ASSIGN, JSOP_NOP, pn2, init, tc);
-            if (!pn2 ||
-                !CheckDestructuring(cx, &data,
-                                    pn2->pn_left, pn2->pn_right,
-                                    tc)) {
+            if (!pn2)
                 return NULL;
-            }
             pn->append(pn2);
             continue;
         }
 #endif /* JS_HAS_DESTRUCTURING */
 
         if (tt != TOK_NAME) {
             if (tt != TOK_ERROR) {
                 js_ReportCompileErrorNumber(cx, ts, NULL, JSREPORT_ERROR,