Bug 443074 - Incorrect decompilation (missing parens) with genexp in for-loop-condition (r=jorendorff).
authorBrendan Eich <brendan@mozilla.org>
Tue, 14 Oct 2008 16:13:56 -0700
changeset 20902 ee94be50279102c35f3bda321fc6773ef0ddc123
parent 20901 46ddf665b124f4d19de88e6b163156b759253fd9
child 20903 5dd892bec01d44223b9e1329a52b65f634cfcaa5
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs443074
milestone1.9.1b2pre
Bug 443074 - Incorrect decompilation (missing parens) with genexp in for-loop-condition (r=jorendorff).
js/src/jsemit.cpp
js/src/jsparse.cpp
js/src/jsparse.h
js/src/jsscan.h
--- a/js/src/jsemit.cpp
+++ b/js/src/jsemit.cpp
@@ -2073,40 +2073,43 @@ CheckSideEffects(JSContext *cx, JSCodeGe
          * in jsinterp.c.
          */
         fun = (JSFunction *) pn->pn_funpob->object;
         if (fun->atom)
             *answer = JS_TRUE;
         break;
 
       case PN_LIST:
-        if (pn->pn_type == TOK_NEW ||
-            pn->pn_type == TOK_LP ||
-            pn->pn_type == TOK_LB ||
-            pn->pn_type == TOK_RB ||
-            pn->pn_type == TOK_RC) {
+        if (pn->pn_op == JSOP_NOP ||
+            pn->pn_op == JSOP_OR || pn->pn_op == JSOP_AND ||
+            pn->pn_op == JSOP_STRICTEQ || pn->pn_op == JSOP_STRICTNE) {
+            /*
+             * Non-operators along with ||, &&, ===, and !== never invoke
+             * toString or valueOf.
+             */
+            for (pn2 = pn->pn_head; pn2; pn2 = pn2->pn_next)
+                ok &= CheckSideEffects(cx, cg, pn2, answer);
+        } else {
             /*
              * All invocation operations (construct: TOK_NEW, call: TOK_LP)
              * are presumed to be useful, because they may have side effects
              * even if their main effect (their return value) is discarded.
              *
              * TOK_LB binary trees of 3 or more nodes are flattened into lists
              * to avoid too much recursion.  All such lists must be presumed
              * to be useful because each index operation could invoke a getter
              * (the JSOP_ARGUMENTS special case below, in the PN_BINARY case,
              * does not apply here: arguments[i][j] might invoke a getter).
              *
-             * Array and object initializers (TOK_RB and TOK_RC lists) must be
-             * considered useful, because they are sugar for constructor calls
-             * (to Array and Object, respectively).
+             * Likewise, array and object initialisers may call prototype
+             * setters (the __defineSetter__ built-in, and writable __proto__
+             * on Array.prototype create this hazard). Initialiser list nodes
+             * have JSOP_NEWINIT in their pn_op.
              */
             *answer = JS_TRUE;
-        } else {
-            for (pn2 = pn->pn_head; pn2; pn2 = pn2->pn_next)
-                ok &= CheckSideEffects(cx, cg, pn2, answer);
         }
         break;
 
       case PN_TERNARY:
         ok = CheckSideEffects(cx, cg, pn->pn_kid1, answer) &&
              CheckSideEffects(cx, cg, pn->pn_kid2, answer) &&
              CheckSideEffects(cx, cg, pn->pn_kid3, answer);
         break;
@@ -2133,21 +2136,31 @@ CheckSideEffects(JSContext *cx, JSCodeGe
                 if (!*answer &&
                     (pn->pn_op != JSOP_NOP ||
                      pn2->pn_slot < 0 ||
                      !pn2->pn_const)) {
                     *answer = JS_TRUE;
                 }
             }
         } else {
-            /*
-             * We can't easily prove that neither operand ever denotes an
-             * object with a toString or valueOf method.
-             */
-            *answer = JS_TRUE;
+            if (pn->pn_op == JSOP_OR || pn->pn_op == JSOP_AND ||
+                pn->pn_op == JSOP_STRICTEQ || pn->pn_op == JSOP_STRICTNE) {
+                /*
+                 * ||, &&, ===, and !== do not convert their operands via
+                 * toString or valueOf method calls.
+                 */
+                ok = CheckSideEffects(cx, cg, pn->pn_left, answer) &&
+                     CheckSideEffects(cx, cg, pn->pn_right, answer);
+            } else {
+                /*
+                 * We can't easily prove that neither operand ever denotes an
+                 * object with a toString or valueOf method.
+                 */
+                *answer = JS_TRUE;
+            }
         }
         break;
 
       case PN_UNARY:
         switch (pn->pn_type) {
           case TOK_RP:
             ok = CheckSideEffects(cx, cg, pn->pn_kid, answer);
             break;
@@ -2168,16 +2181,24 @@ CheckSideEffects(JSContext *cx, JSCodeGe
                 *answer = JS_TRUE;
                 break;
               default:
                 ok = CheckSideEffects(cx, cg, pn2, answer);
                 break;
             }
             break;
 
+          case TOK_UNARYOP:
+            if (pn->pn_op == JSOP_NOT) {
+                /* ! does not convert its operand via toString or valueOf. */
+                ok = CheckSideEffects(cx, cg, pn->pn_kid, answer);
+                break;
+            }
+            /* FALL THROUGH */
+
           default:
             /*
              * All of TOK_INC, TOK_DEC, TOK_THROW, TOK_YIELD, and TOK_DEFSHARP
              * have direct effects. Of the remaining unary-arity node types,
              * we can't easily prove that the operand never denotes an object
              * with a toString or valueOf method.
              */
             *answer = JS_TRUE;
--- a/js/src/jsparse.cpp
+++ b/js/src/jsparse.cpp
@@ -2880,52 +2880,36 @@ Statement(JSContext *cx, JSTokenStream *
             tt = js_PeekToken(cx, ts);
             ts->flags &= ~TSF_OPERAND;
             if (tt == TOK_SEMI) {
                 pn2 = NULL;
             } else {
                 pn2 = Expr(cx, ts, tc);
                 if (!pn2)
                     return NULL;
-
-                if (pn2->pn_type == TOK_LP &&
-                    pn2->pn_head->pn_type == TOK_FUNCTION &&
-                    (pn2->pn_head->pn_flags & TCF_GENEXP_LAMBDA)) {
-                    /*
-                     * A generator expression as loop condition is useless.
-                     * It won't be called, and as an object it evaluates to
-                     * true in boolean contexts without any conversion hook
-                     * being called.
-                     *
-                     * This useless condition elimination is mandatory, to
-                     * help the decompiler. See bug 442342.
-                     */
-                    RecycleTree(pn2, tc);
-                    pn2 = NULL;
-                }
             }
 
             /* Parse the update expression or null into pn3. */
             MUST_MATCH_TOKEN(TOK_SEMI, JSMSG_SEMI_AFTER_FOR_COND);
             ts->flags |= TSF_OPERAND;
             tt = js_PeekToken(cx, ts);
             ts->flags &= ~TSF_OPERAND;
             if (tt == TOK_RP) {
                 pn3 = NULL;
             } else {
                 pn3 = Expr(cx, ts, tc);
                 if (!pn3)
                     return NULL;
             }
 
-            /* Build the RESERVED node to use as the left kid of pn. */
+            /* Build the FORHEAD node to use as the left kid of pn. */
             pn4 = NewParseNode(cx, ts, PN_TERNARY, tc);
             if (!pn4)
                 return NULL;
-            pn4->pn_type = TOK_RESERVED;
+            pn4->pn_type = TOK_FORHEAD;
             pn4->pn_op = JSOP_NOP;
             pn4->pn_kid1 = pn1;
             pn4->pn_kid2 = pn2;
             pn4->pn_kid3 = pn3;
             pn->pn_left = pn4;
         }
 
         MUST_MATCH_TOKEN(TOK_RP, JSMSG_PAREN_AFTER_FOR_CTRL);
@@ -4614,16 +4598,17 @@ MemberExpr(JSContext *cx, JSTokenStream 
                         pn2->pn_type = TOK_DOT;
                         pn2->pn_op = JSOP_GETPROP;
                         pn2->pn_arity = PN_NAME;
                         pn2->pn_expr = pn;
                         pn2->pn_atom = pn3->pn_atom;
                         break;
                     }
                     pn3->pn_type = TOK_NUMBER;
+                    pn3->pn_op = JSOP_DOUBLE;
                     pn3->pn_dval = index;
                 }
                 pn2->pn_op = JSOP_GETELEM;
                 pn2->pn_left = pn;
                 pn2->pn_right = pn3;
             } while (0);
         } else if (allowCallSyntax && tt == TOK_LP) {
             pn2 = NewParseNode(cx, ts, PN_LIST, tc);
@@ -5388,16 +5373,17 @@ PrimaryExpr(JSContext *cx, JSTokenStream
       {
         JSBool matched;
         jsuint index;
 
         pn = NewParseNode(cx, ts, PN_LIST, tc);
         if (!pn)
             return NULL;
         pn->pn_type = TOK_RB;
+        pn->pn_op = JSOP_NEWINIT;
 
 #if JS_HAS_SHARP_VARS
         if (defsharp) {
             PN_INIT_LIST_1(pn, defsharp);
             defsharp = NULL;
         } else
 #endif
             PN_INIT_LIST(pn);
@@ -5521,16 +5507,17 @@ PrimaryExpr(JSContext *cx, JSTokenStream
       {
         JSBool afterComma;
         JSParseNode *pnval;
 
         pn = NewParseNode(cx, ts, PN_LIST, tc);
         if (!pn)
             return NULL;
         pn->pn_type = TOK_RC;
+        pn->pn_op = JSOP_NEWINIT;
 
 #if JS_HAS_SHARP_VARS
         if (defsharp) {
             PN_INIT_LIST_1(pn, defsharp);
             defsharp = NULL;
         } else
 #endif
             PN_INIT_LIST(pn);
@@ -5850,16 +5837,17 @@ PrimaryExpr(JSContext *cx, JSTokenStream
         pn->pn_op = JSOP_REGEXP;
         break;
       }
 
       case TOK_NUMBER:
         pn = NewParseNode(cx, ts, PN_NULLARY, tc);
         if (!pn)
             return NULL;
+        pn->pn_op = JSOP_DOUBLE;
         pn->pn_dval = CURRENT_TOKEN(ts).t_dval;
 #if JS_HAS_SHARP_VARS
         notsharp = JS_TRUE;
 #endif
         break;
 
       case TOK_PRIMARY:
         pn = NewParseNode(cx, ts, PN_NULLARY, tc);
@@ -6281,18 +6269,62 @@ recur:
         if (pn->pn_type == TOK_DOT || pn->pn_type == TOK_DBLDOT)
             TAIL_RECURSE(pn->pn_expr);
         /* FALL THROUGH */
     }
     return JS_FALSE;
 #undef TAIL_RECURSE
 }
 
+static int
+Boolish(JSParseNode *pn)
+{
+    switch (pn->pn_op) {
+      case JSOP_DOUBLE:
+        return pn->pn_dval != 0;
+
+      case JSOP_STRING:
+        return JSSTRING_LENGTH(ATOM_TO_STRING(pn->pn_atom)) != 0;
+
+      case JSOP_CALL:
+        /*
+         * A generator expression as an if or loop condition has no effects, it
+         * simply results in a truthy object reference. This condition folding
+         * ing is needed for the decompiler. See bug 442342 and bug 443074.
+         */
+        if (pn->pn_count != 1)
+            break;
+        JSParseNode *pn2 = pn->pn_head;
+        if (pn2->pn_type != TOK_FUNCTION)
+            break;
+        if (!(pn2->pn_flags & TCF_GENEXP_LAMBDA))
+            break;
+        /* FALL THROUGH */
+
+      case JSOP_DEFFUN:
+      case JSOP_NAMEDFUNOBJ:
+      case JSOP_ANONFUNOBJ:
+      case JSOP_THIS:
+      case JSOP_TRUE:
+        return 1;
+
+      case JSOP_NULL:
+      case JSOP_FALSE:
+        return 0;
+
+      default:;
+    }
+    return -1;
+}
+
+#define Truthy(pn) (Boolish(pn) == 1)
+#define Falsy(pn)  (Boolish(pn) == 0)
+
 JSBool
-js_FoldConstants(JSContext *cx, JSParseNode *pn, JSTreeContext *tc)
+js_FoldConstants(JSContext *cx, JSParseNode *pn, JSTreeContext *tc, bool inCond)
 {
     JSParseNode *pn1 = NULL, *pn2 = NULL, *pn3 = NULL;
 
     JS_CHECK_RECURSION(cx, return JS_FALSE);
 
     switch (pn->pn_arity) {
       case PN_FUNC:
       {
@@ -6357,38 +6389,54 @@ js_FoldConstants(JSContext *cx, JSParseN
         }
         break;
 
       case PN_TERNARY:
         /* Any kid may be null (e.g. for (;;)). */
         pn1 = pn->pn_kid1;
         pn2 = pn->pn_kid2;
         pn3 = pn->pn_kid3;
-        if (pn1 && !js_FoldConstants(cx, pn1, tc))
+        if (pn1 && !js_FoldConstants(cx, pn1, tc, pn->pn_type == TOK_IF))
             return JS_FALSE;
-        if (pn2 && !js_FoldConstants(cx, pn2, tc))
-            return JS_FALSE;
+        if (pn2) {
+            if (!js_FoldConstants(cx, pn2, tc, pn->pn_type == TOK_FORHEAD))
+                return JS_FALSE;
+            if (pn->pn_type == TOK_FORHEAD && pn2->pn_op == JSOP_TRUE) {
+                RecycleTree(pn2, tc);
+                pn->pn_kid2 = NULL;
+            }
+        }
         if (pn3 && !js_FoldConstants(cx, pn3, tc))
             return JS_FALSE;
         break;
 
       case PN_BINARY:
-        /* First kid may be null (for default case in switch). */
         pn1 = pn->pn_left;
         pn2 = pn->pn_right;
-        if (pn1 && !js_FoldConstants(cx, pn1, tc))
+
+        /* Propagate inCond through logical connectives. */
+        if (pn->pn_type == TOK_OR || pn->pn_type == TOK_AND) {
+            if (!js_FoldConstants(cx, pn1, tc, inCond))
+                return JS_FALSE;
+            if (!js_FoldConstants(cx, pn2, tc, inCond))
+                return JS_FALSE;
+            break;
+        }
+
+        /* First kid may be null (for default case in switch). */
+        if (pn1 && !js_FoldConstants(cx, pn1, tc, pn->pn_type == TOK_WHILE))
             return JS_FALSE;
-        if (!js_FoldConstants(cx, pn2, tc))
+        if (!js_FoldConstants(cx, pn2, tc, pn->pn_type == TOK_DO))
             return JS_FALSE;
         break;
 
       case PN_UNARY:
         /* Our kid may be null (e.g. return; vs. return e;). */
         pn1 = pn->pn_kid;
-        if (pn1 && !js_FoldConstants(cx, pn1, tc))
+        if (pn1 && !js_FoldConstants(cx, pn1, tc, inCond))
             return JS_FALSE;
         break;
 
       case PN_NAME:
         /*
          * Skip pn1 down along a chain of dotted member expressions to avoid
          * excessive recursion.  Our only goal here is to fold constants (if
          * any) in the primary expression operand to the left of the first
@@ -6472,16 +6520,40 @@ js_FoldConstants(JSContext *cx, JSParseN
             pn->pn_arity = PN_LIST;
             PN_INIT_LIST(pn);
         }
         RecycleTree(pn2, tc);
         if (pn3 && pn3 != pn2)
             RecycleTree(pn3, tc);
         break;
 
+      case TOK_OR:
+        if (inCond) {
+            if (Truthy(pn1)) {
+                RecycleTree(pn2, tc);
+                PN_MOVE_NODE(pn, pn1);
+            } else if (Falsy(pn1)) {
+                RecycleTree(pn1, tc);
+                PN_MOVE_NODE(pn, pn2);
+            }
+        }
+        break;
+
+      case TOK_AND:
+        if (inCond) {
+            if (Falsy(pn1)) {
+                RecycleTree(pn2, tc);
+                PN_MOVE_NODE(pn, pn1);
+            } else if (Truthy(pn1)) {
+                RecycleTree(pn1, tc);
+                PN_MOVE_NODE(pn, pn2);
+            }
+        }
+        break;
+
       case TOK_ASSIGN:
         /*
          * Compound operators such as *= should be subject to folding, in case
          * the left-hand side is constant, and so that the decompiler produces
          * the same string that you get from decompiling a script or function
          * compiled from that same string.  As with +, += is special.
          */
         if (pn->pn_op == JSOP_NOP)
@@ -6702,10 +6774,26 @@ js_FoldConstants(JSContext *cx, JSParseN
             RecycleTree(pn1, tc);
         }
         break;
 #endif /* JS_HAS_XML_SUPPORT */
 
       default:;
     }
 
+    if (inCond) {
+        int cond = Boolish(pn);
+        if (cond >= 0) {
+            if (pn->pn_arity == PN_LIST) {
+                pn2 = pn->pn_head;
+                do {
+                    pn3 = pn2->pn_next;
+                    RecycleTree(pn2, tc);
+                } while ((pn2 = pn3) != NULL);
+            }
+            pn->pn_type = TOK_PRIMARY;
+            pn->pn_op = cond ? JSOP_TRUE : JSOP_FALSE;
+            pn->pn_arity = PN_NULLARY;
+        }
+    }
+
     return JS_TRUE;
 }
--- a/js/src/jsparse.h
+++ b/js/src/jsparse.h
@@ -459,17 +459,18 @@ js_CompileScript(JSContext *cx, JSObject
                  FILE *file, const char *filename, uintN lineno);
 
 extern JSBool
 js_CompileFunctionBody(JSContext *cx, JSFunction *fun, JSPrincipals *principals,
                        const jschar *chars, size_t length,
                        const char *filename, uintN lineno);
 
 extern JSBool
-js_FoldConstants(JSContext *cx, JSParseNode *pn, JSTreeContext *tc);
+js_FoldConstants(JSContext *cx, JSParseNode *pn, JSTreeContext *tc,
+                 bool inCond = false);
 
 #if JS_HAS_XML_SUPPORT
 JS_FRIEND_API(JSParseNode *)
 js_ParseXMLText(JSContext *cx, JSObject *chain, JSParseContext *pc,
                 JSBool allowList);
 #endif
 
 /*
--- a/js/src/jsscan.h
+++ b/js/src/jsscan.h
@@ -131,16 +131,17 @@ typedef enum JSTokenType {
     TOK_XMLLIST = 76,                   /* XML list node type (no token) */
     TOK_YIELD = 77,                     /* yield from generator function */
     TOK_ARRAYCOMP = 78,                 /* array comprehension initialiser */
     TOK_ARRAYPUSH = 79,                 /* array push within comprehension */
     TOK_LEXICALSCOPE = 80,              /* block scope AST node label */
     TOK_LET = 81,                       /* let keyword */
     TOK_SEQ = 82,                       /* synthetic sequence of statements,
                                            not a block */
+    TOK_FORHEAD = 83,                   /* head of for(;;)-style loop */
     TOK_RESERVED,                       /* reserved keywords */
     TOK_LIMIT                           /* domain size */
 } JSTokenType;
 
 #define IS_PRIMARY_TOKEN(tt) \
     ((uintN)((tt) - TOK_NAME) <= (uintN)(TOK_PRIMARY - TOK_NAME))
 
 #define TOKEN_TYPE_IS_XML(tt) \