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.

js/src/jsparse.cpp

@@ -6910,33 +6910,42 @@ 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 (!dn2)
-                            return false;
+                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_pos = root->pn_pos;
 
-                        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;
-
-                        JSParseNode **pnup = &dn->dn_uses;
-                        JSParseNode *pnu;
-                        while ((pnu = *pnup) != NULL && pnu->pn_pos >= root->pn_pos) {
-                            pnu->pn_lexdef = dn2;
-                            dn2->pn_dflags |= pnu->pn_dflags & PND_USE2DEF_FLAGS;
-                            pnup = &pnu->pn_link;
-                        }
-                        dn2->dn_uses = dn->dn_uses;
-                        dn->dn_uses = *pnup;
-                        *pnup = NULL;
-
-                        dn = dn2;
+                    /* 
+                     * 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) {
+                        pnu->pn_lexdef = dn2;
+                        dn2->pn_dflags |= pnu->pn_dflags & PND_USE2DEF_FLAGS;
+                        pnup = &pnu->pn_link;
                     }
+                    dn2->dn_uses = dn->dn_uses;
+                    dn->dn_uses = *pnup;
+                    *pnup = NULL;
+                } 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;
                 }

js/src/tests/js1_8_5/extensions/jstests.list

@@ -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

js/src/tests/js1_8_5/extensions/regress-627859.js

@@ -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);