Bug 627859: Use the standard placeholder-making function when re-scoping variable references in generator 'yield' expressions. r=brendan

CompExprTransplanter::transplant did a bunch of bespoke pointer-fiddling to
take the free variable references in the 'yield' expression and re-scope
them within the comprehension tail clauses. That code incorrectly
constructed placeholders for function references; this was fixed in bug 576847.

However, it would also work to simply use the same placeholder construction
function everybody else does. This patch makes it so.
This commit is contained in:
Jim Blandy 2011-06-29 02:11:08 -07:00
parent 6af272b7e5
commit 7134b29a2e
3 changed files with 58 additions and 25 deletions

View File

@ -6910,20 +6910,25 @@ CompExprTransplanter::transplant(JSParseNode *pn)
if (genexp && PN_OP(dn) != JSOP_CALLEE) {
JS_ASSERT(!tc->decls.lookupFirst(atom));
if (dn->pn_pos < root->pn_pos || dn->isPlaceholder()) {
if (dn->pn_pos >= root->pn_pos) {
tc->parent->lexdeps->remove(atom);
} else {
JSDefinition *dn2 = (JSDefinition *)NameNode::create(atom, tc);
if (dn->pn_pos < root->pn_pos) {
/*
* The variable originally appeared to be a use of a
* definition or placeholder outside the generator, but now
* we know it is scoped within the comprehension tail's
* clauses. Make it (along with any other uses within the
* generator) a use of a new placeholder in the generator's
* lexdeps.
*/
AtomDefnAddPtr p = tc->lexdeps->lookupForAdd(atom);
JSDefinition *dn2 = MakePlaceholder(p, pn, tc);
if (!dn2)
return false;
dn2->pn_type = TOK_NAME;
dn2->pn_op = JSOP_NOP;
dn2->pn_defn = true;
dn2->pn_dflags |= PND_PLACEHOLDER;
dn2->pn_pos = root->pn_pos;
/*
* Change all uses of |dn| that lie within the generator's
* |yield| expression into uses of dn2.
*/
JSParseNode **pnup = &dn->dn_uses;
JSParseNode *pnu;
while ((pnu = *pnup) != NULL && pnu->pn_pos >= root->pn_pos) {
@ -6934,9 +6939,13 @@ CompExprTransplanter::transplant(JSParseNode *pn)
dn2->dn_uses = dn->dn_uses;
dn->dn_uses = *pnup;
*pnup = NULL;
dn = dn2;
}
} else if (dn->isPlaceholder()) {
/*
* The variable first occurs free in the 'yield' expression;
* move the existing placeholder node (and all its uses)
* from the parent's lexdeps into the generator's lexdeps.
*/
tc->parent->lexdeps->remove(atom);
if (!tc->lexdeps->put(atom, dn))
return false;
}

View File

@ -29,6 +29,7 @@ skip-if(!xulRuntime.shell) script clone-forge.js
skip-if(!xulRuntime.shell) script clone-complex-object.js
script set-property-non-extensible.js
script recursion.js
script regress-627859.js
script regress-627984-1.js
script regress-627984-2.js
script regress-627984-3.js

View File

@ -0,0 +1,23 @@
// -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/
var x = 42;
function a() {
var x;
function b() {
x = 43;
// When jsparse.cpp's CompExprTransplanter transplants the
// comprehension expression 'x' into the scope of the 'for' loop,
// it must not bring the placeholder definition node for the
// assignment to x above along with it. If it does, x won't appear
// in b's lexdeps, we'll never find out that the assignment refers
// to a's x, and we'll generate an assignment to the global x.
(x for (x in []));
}
b();
}
a();
assertEq(x, 42);
reportCompare(true, true);