From d5d7862a78bc70a569d4f53fcf3cb69fa975350a Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Tue, 1 Jul 2008 13:05:11 -0700 Subject: [PATCH] Eliminate useless genexp for(;;) conditions (442342, r=jorendorff). --- js/src/jsemit.cpp | 6 ------ js/src/jsopcode.cpp | 3 --- js/src/jsopcode.tbl | 4 ++-- js/src/jsparse.cpp | 16 ++++++++++++++++ js/tests/js1_8/genexps/regress-380237-04.js | 16 +++++++++------- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index 200784e1cf4..e7fa5b46a2a 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -4627,12 +4627,6 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) } if (pn2->pn_kid2) { - if (pn2->pn_kid2->pn_type == TOK_LP && - pn2->pn_kid2->pn_head->pn_type == TOK_FUNCTION && - (pn2->pn_kid2->pn_head->pn_flags & TCF_GENEXP_LAMBDA) && - js_NewSrcNote(cx, cg, SRC_GENEXP) < 0) { - return JS_FALSE; - } beq = EmitJump(cx, cg, JSOP_IFNE, top - CG_OFFSET(cg)); if (beq < 0) return JS_FALSE; diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index f4eeb229c5f..2d6f467353b 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -3868,9 +3868,6 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop) } else { op = (JSOp) *pc2; op = ((js_CodeSpec[op].format & JOF_PARENHEAD) || - ((op == JSOP_IFNE || op == JSOP_IFNEX) && - (!(sn2 = js_GetSrcNote(outer, pc2)) || - SN_TYPE(sn2) != SRC_GENEXP)) || ((js_CodeSpec[op].format & JOF_INVOKE) && GET_ARGC(pc2) == 1) || ((op == JSOP_IFEQ || op == JSOP_IFEQX) && diff --git a/js/src/jsopcode.tbl b/js/src/jsopcode.tbl index 00f6f049589..f9ea46a03a5 100644 --- a/js/src/jsopcode.tbl +++ b/js/src/jsopcode.tbl @@ -105,7 +105,7 @@ OPDEF(JSOP_LEAVEWITH, 4, "leavewith", NULL, 1, 1, 0, 0, JOF_BYTE) OPDEF(JSOP_RETURN, 5, "return", NULL, 1, 1, 0, 2, JOF_BYTE) OPDEF(JSOP_GOTO, 6, "goto", NULL, 3, 0, 0, 0, JOF_JUMP) OPDEF(JSOP_IFEQ, 7, "ifeq", NULL, 3, 1, 0, 4, JOF_JUMP|JOF_DETECTING) -OPDEF(JSOP_IFNE, 8, "ifne", NULL, 3, 1, 0, 0, JOF_JUMP) +OPDEF(JSOP_IFNE, 8, "ifne", NULL, 3, 1, 0, 0, JOF_JUMP|JOF_PARENHEAD) /* Get the arguments object for the current, lightweight function activation. */ OPDEF(JSOP_ARGUMENTS, 9, js_arguments_str, js_arguments_str, 1, 0, 1, 18, JOF_BYTE) @@ -353,7 +353,7 @@ OPDEF(JSOP_DEFLOCALFUN, 138,"deflocalfun",NULL, 5, 0, 0, 0, JOF_SLOTOB /* Extended jumps. */ OPDEF(JSOP_GOTOX, 139,"gotox", NULL, 5, 0, 0, 0, JOF_JUMPX) OPDEF(JSOP_IFEQX, 140,"ifeqx", NULL, 5, 1, 0, 4, JOF_JUMPX|JOF_DETECTING) -OPDEF(JSOP_IFNEX, 141,"ifnex", NULL, 5, 1, 0, 0, JOF_JUMPX) +OPDEF(JSOP_IFNEX, 141,"ifnex", NULL, 5, 1, 0, 0, JOF_JUMPX|JOF_PARENHEAD) OPDEF(JSOP_ORX, 142,"orx", NULL, 5, 1, 0, 5, JOF_JUMPX|JOF_DETECTING) OPDEF(JSOP_ANDX, 143,"andx", NULL, 5, 1, 0, 6, JOF_JUMPX|JOF_DETECTING) OPDEF(JSOP_GOSUBX, 144,"gosubx", NULL, 5, 0, 0, 0, JOF_JUMPX) diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index 54185147906..a9de36ecbfb 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -2860,6 +2860,22 @@ Statement(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc) 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. */ diff --git a/js/tests/js1_8/genexps/regress-380237-04.js b/js/tests/js1_8/genexps/regress-380237-04.js index 6c9994e2e73..a4e3a39a8b1 100644 --- a/js/tests/js1_8/genexps/regress-380237-04.js +++ b/js/tests/js1_8/genexps/regress-380237-04.js @@ -78,7 +78,7 @@ needParens("switch (x) { case xx: }"); needParens("return xx;"); needParens("yield xx;"); needParens("for (xx;;) { }"); -needParens("for (;xx;) { }"); +needParens("for (;xx;) { }", "function anonymous() {\n for (;;) {\n }\n}"); needParens("for (;;xx) { }"); needParens("for (i in xx) { }"); needParens("throw xx"); @@ -211,7 +211,7 @@ function doesNotNeedParens(pat) // print("Skipping the over-parenthesization test, because I don't know how to test for over-parenthesization when the pattern doesn't have parens snugly around it.") } -function needParens(pat) +function needParens(pat, exp) { print("Testing " + pat); @@ -242,8 +242,8 @@ function needParens(pat) } reportCompare(expect, actual, summary + ': needParens ' + ft); - roundTripTest(f); - overParenTest(f); + roundTripTest(f, exp); + overParenTest(f, exp); } function rejectLHS(pat) @@ -268,9 +268,11 @@ function rejectLHS(pat) } -function overParenTest(f) +function overParenTest(f, exp) { var uf = "" + f; + if (uf == exp) + return; reportCompare(false, uf.indexOf(genexpParened) == -1, summary + ': overParenTest genexp snugly in parentheses: ' + uf); @@ -301,7 +303,7 @@ function sanityCheck(pat) reportCompare(expect, actual, summary + ': sanityCheck ' + pat); } -function roundTripTest(f) +function roundTripTest(f, exp) { // Decompile var uf = "" + f; @@ -321,7 +323,7 @@ function roundTripTest(f) } // Decompile again and make sure the decompilations match exactly. - expect = uf; + expect = exp || uf; actual = "" + euf; reportCompare(expect, actual, summary + ': roundTripTest no round-trip change'); }