From c5afb1902b9e2edc5b230c818dab741be0ef3624 Mon Sep 17 00:00:00 2001 From: "brendan%mozilla.org" Date: Thu, 29 Jul 2004 23:57:54 +0000 Subject: [PATCH] - Decorate PN_LIST nodes with pn_extra flags whose constant names describe their meaning better, adding a flag for 'for (var x ... in o)'. - Set these flags from the parser, in a future-proof way (|=, not =, given the zero initialized pn_extra flags member of pn_list). - Test list flags in the code generator, including PNX_FORINVAR in conjunction with whether the for..in loop is of the oddball form 'for (var x = i in o)'. - Thereby fix failure to emit a JSOP_DEFVAR in the case that a function wraps a for..in loop in a with statement (252892, r=shaver, a=asa). The same fix cured another bug, not reported, where 'for (var x = i in o)' in a function without any with statement would emit the wrong opcode (JSOP_FORNAME, not JSOP_FORVAR). git-svn-id: svn://10.0.0.236/branches/AVIARY_1_0_20040515_BRANCH@160099 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/js/src/jsemit.c | 73 +++++++++++++++++++++++++++++++--------- mozilla/js/src/jsparse.c | 16 +++++---- mozilla/js/src/jsparse.h | 12 +++++-- 3 files changed, 77 insertions(+), 24 deletions(-) diff --git a/mozilla/js/src/jsemit.c b/mozilla/js/src/jsemit.c index b3438579de3..370ff54cd54 100644 --- a/mozilla/js/src/jsemit.c +++ b/mozilla/js/src/jsemit.c @@ -2815,15 +2815,29 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) stmtInfo.type = STMT_FOR_IN_LOOP; noteIndex = -1; - /* If the left part is var x = i, bind x, evaluate i, and pop. */ + /* + * If the left part is 'var x', emit code to define x if necessary + * using a prolog opcode, but do not emit a pop. If the left part + * is 'var x = i', emit prolog code to define x if necessary; then + * emit code to evaluate i, assign the result to x, and pop the + * result off the stack. + * + * All the logic to do this is implemented in the outer switch's + * TOK_VAR case, conditioned on pn_extra flags set by the parser. + * + * In the 'for (var x = i in o) ...' case, the js_EmitTree(...pn3) + * called here will generate the SRC_VAR note for the assignment + * op that sets x = i, hoisting the initialized var declaration + * out of the loop: 'var x = i; for (x in o) ...'. + * + * In the 'for (var x in o) ...' case, nothing but the prolog op + * (if needed) should be generated here, we must emit the SRC_VAR + * just before the JSOP_FOR* opcode in the switch on pn3->pn_type + * a bit below, so nothing is hoisted: 'for (var x in o) ...'. + */ pn3 = pn2->pn_left; - if (pn3->pn_type == TOK_VAR && pn3->pn_head->pn_expr) { - if (!js_EmitTree(cx, cg, pn3)) - return JS_FALSE; - /* Set pn3 to the variable name, to avoid another var note. */ - pn3 = pn3->pn_head; - JS_ASSERT(pn3->pn_type == TOK_NAME); - } + if (pn3->pn_type == TOK_VAR && !js_EmitTree(cx, cg, pn3)) + return JS_FALSE; /* Emit a push to allocate the iterator. */ if (js_Emit1(cx, cg, JSOP_PUSH) < 0) @@ -2842,14 +2856,26 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) switch (pn3->pn_type) { case TOK_VAR: pn3 = pn3->pn_head; - if (js_NewSrcNote(cx, cg, SRC_VAR) < 0) + JS_ASSERT(pn3->pn_type == TOK_NAME); + if (!pn3->pn_expr && js_NewSrcNote(cx, cg, SRC_VAR) < 0) return JS_FALSE; /* FALL THROUGH */ case TOK_NAME: - pn3->pn_op = JSOP_FORNAME; - if (!LookupArgOrVar(cx, &cg->treeContext, pn3)) - return JS_FALSE; - op = pn3->pn_op; + if (pn3->pn_slot >= 0) { + op = pn3->pn_op; + switch (op) { + case JSOP_GETARG: /* FALL THROUGH */ + case JSOP_SETARG: op = JSOP_FORARG; break; + case JSOP_GETVAR: /* FALL THROUGH */ + case JSOP_SETVAR: op = JSOP_FORVAR; break; + default: JS_ASSERT(0); + } + } else { + pn3->pn_op = JSOP_FORNAME; + if (!LookupArgOrVar(cx, &cg->treeContext, pn3)) + return JS_FALSE; + op = pn3->pn_op; + } if (pn3->pn_slot >= 0) { if (pn3->pn_attrs & JSPROP_READONLY) op = JSOP_GETVAR; @@ -3424,6 +3450,23 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) return JS_FALSE; } } + + /* + * 'for (var x in o) ...' and 'for (var x = i in o) ...' call the + * TOK_VAR case, but only the initialized case (a strange one that + * falls out of ECMA-262's grammar) wants to run past this point. + * Both cases must conditionally emit a JSOP_DEFVAR, above. Note + * that the parser error-checks to ensure that pn->pn_count is 1. + * + * XXX Narcissus keeps track of variable declarations in the node + * for the script being compiled, so there's no need to share any + * conditional prolog code generation there. We could do likewise, + * but it's a big change, requiring extra allocation, so probably + * not worth the trouble for SpiderMonkey. + */ + if ((pn->pn_extra & PNX_FORINVAR) && !pn2->pn_expr) + break; + if (pn2 == pn->pn_head && js_NewSrcNote(cx, cg, (pn->pn_op == JSOP_DEFCONST) @@ -3451,7 +3494,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) return JS_FALSE; } } - if (pn->pn_extra) { + if (pn->pn_extra & PNX_POPVAR) { if (js_Emit1(cx, cg, JSOP_POP) < 0) return JS_FALSE; } @@ -4029,7 +4072,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) atomIndex++; } - if (pn->pn_extra) { + if (pn->pn_extra & PNX_ENDCOMMA) { /* Emit a source note so we know to decompile an extra comma. */ if (js_NewSrcNote(cx, cg, SRC_CONTINUE) < 0) return JS_FALSE; diff --git a/mozilla/js/src/jsparse.c b/mozilla/js/src/jsparse.c index bf1d5b1df54..61a855eed11 100644 --- a/mozilla/js/src/jsparse.c +++ b/mozilla/js/src/jsparse.c @@ -1506,9 +1506,13 @@ Statement(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc) } if (pn1->pn_type == TOK_VAR) { - /* Tell js_EmitTree(TOK_VAR) to generate a final POP. */ - pn1->pn_extra = JS_TRUE; + /* Tell js_EmitTree(TOK_VAR) that pn1 is part of a for/in. */ + pn1->pn_extra |= PNX_FORINVAR; + + /* Generate a final POP only if the var has an initializer. */ pn2 = pn1->pn_head; + if (pn2->pn_expr) + pn1->pn_extra |= PNX_POPVAR; } else { pn2 = pn1; } @@ -1828,7 +1832,7 @@ Statement(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc) return NULL; /* Tell js_EmitTree to generate a final POP. */ - pn->pn_extra = JS_TRUE; + pn->pn_extra |= PNX_POPVAR; break; case TOK_RETURN: @@ -1994,7 +1998,7 @@ Variables(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc) if (!pn) return NULL; pn->pn_op = CURRENT_TOKEN(ts).t_op; - pn->pn_extra = JS_FALSE; /* assume no JSOP_POP needed */ + pn->pn_extra = 0; /* assume no JSOP_POP needed */ PN_INIT_LIST(pn); /* @@ -2834,7 +2838,7 @@ PrimaryExpr(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc) if (!pn) return NULL; pn->pn_type = TOK_RB; - pn->pn_extra = JS_FALSE; + pn->pn_extra = 0; #if JS_HAS_SHARP_VARS if (defsharp) { @@ -2853,7 +2857,7 @@ PrimaryExpr(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc) tt = js_PeekToken(cx, ts); ts->flags &= ~TSF_REGEXP; if (tt == TOK_RB) { - pn->pn_extra = JS_TRUE; + pn->pn_extra |= PNX_ENDCOMMA; break; } diff --git a/mozilla/js/src/jsparse.h b/mozilla/js/src/jsparse.h index fd022138da8..5e28ad70933 100644 --- a/mozilla/js/src/jsparse.h +++ b/mozilla/js/src/jsparse.h @@ -89,6 +89,8 @@ JS_BEGIN_EXTERN_C * TOK_FOR binary pn_left: either * for/in loop: a binary TOK_IN node with * pn_left: TOK_VAR or TOK_NAME to left of 'in' + * if TOK_VAR, its pn_extra may have PNX_POPVAR + * and PNX_FORINVAR bits set * pn_right: object expr to right of 'in' * for(;;) loop: a ternary TOK_RESERVED node with * pn_kid1: init expr before first ';' @@ -163,7 +165,7 @@ JS_BEGIN_EXTERN_C * TOK_RB list pn_head: list of pn_count array element exprs * [,,] holes are represented by TOK_COMMA nodes * #n=[...] produces TOK_DEFSHARP at head of list - * pn_extra: true if extra comma at end + * pn_extra: PN_ENDCOMMA if extra comma at end * TOK_RC list pn_head: list of pn_count TOK_COLON nodes where * each has pn_left: property id, pn_right: value * #n={...} produces TOK_DEFSHARP at head of list @@ -257,8 +259,12 @@ struct JSParseNode { #define pn_dval pn_u.dval /* PN_LIST pn_extra flags. */ -#define PNX_STRCAT 0x1 /* TOK_PLUS list has string term */ -#define PNX_CANTFOLD 0x2 /* TOK_PLUS list has unfoldable term */ +#define PNX_STRCAT 0x01 /* TOK_PLUS list has string term */ +#define PNX_CANTFOLD 0x02 /* TOK_PLUS list has unfoldable term */ +#define PNX_POPVAR 0x04 /* TOK_VAR last result needs popping */ +#define PNX_FORINVAR 0x08 /* TOK_VAR is left kid of TOK_IN node, + which is left kid of TOK_FOR */ +#define PNX_ENDCOMMA 0x10 /* array literal has comma at end */ /* * Move pn2 into pn, preserving pn->pn_pos and pn->pn_offset and handing off