Skip to content

Commit

Permalink
Fix transpilation of block-scoped vars in loop bodies to no longer le…
Browse files Browse the repository at this point in the history 
…ak data from one iteration into the TDZ of the next iteration. This makes the compiled output code behave more consistently with the uncompiled ES6 code.

PiperOrigin-RevId: 588854476
  • Loading branch information
@shicks @copybara-github
shicks authored and copybara-github committed Dec 7, 2023
1 parent c22d1dd commit c7049f8
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 45 deletions.
13 changes: 8 additions & 5 deletions src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ private void transformLoopClosure() {
// later.
LoopObject loopObject = loopObjectMap.get(loopNode);
Node objectLitNextIteration = astFactory.createObjectLit();
renameVarsToProperties(loopObject, objectLitNextIteration);
renameVarsToProperties(loopObject, objectLitNextIteration, loopNode);

Node updateLoopObject =
astFactory.createAssign(createLoopObjectNameNode(loopObject), objectLitNextIteration);
Expand Down Expand Up @@ -752,13 +752,16 @@ private void assignLoopVarToLoopObjectProperty(
}

/** Rename all variables in the loop object to properties */
private void renameVarsToProperties(LoopObject loopObject, Node objectLitNextIteration) {
private void renameVarsToProperties(
LoopObject loopObject, Node objectLitNextIteration, Node scopeRoot) {
for (Var var : loopObject.vars) {
String newPropertyName = getLoopObjPropName(var);
Node newPropertyValue =
var.getScopeRoot() == scopeRoot
? createLoopVarReferenceReplacement(loopObject, var.getNameNode(), newPropertyName)
: astFactory.createUndefinedValue();
objectLitNextIteration.addChildToBack(
astFactory.createStringKey(
newPropertyName,
createLoopVarReferenceReplacement(loopObject, var.getNameNode(), newPropertyName)));
astFactory.createStringKey(newPropertyName, newPropertyValue));
}
}

Expand Down
108 changes: 68 additions & 40 deletions test/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclarationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ public void testLoopClosure() {
"var LOOP$0 = {};",
"var i = 0;",
"for (; i < 10; ",
" LOOP$0 = {y: LOOP$0.y},",
" LOOP$0 = {y: void 0},",
" i++) {",
" LOOP$0.y = i;",
" arr.push((function(LOOP$0$PARAM$1) {",
Expand All @@ -679,7 +679,7 @@ public void testLoopClosure() {
"var LOOP$0 = {}",
"while (true) {",
" LOOP$0 = {",
" i: LOOP$0.i",
" i: void 0",
" };",
" LOOP$0.i = 0;",
" arr.push(",
Expand All @@ -706,8 +706,7 @@ public void testLoopClosure() {
"var LOOP$0 = {};",
"LOOP$0.i = 0;",
"for (; LOOP$0.i < 10;",
" LOOP$0 = {y:" + " LOOP$0.y, ",
" i: LOOP$0.i},",
" LOOP$0 = {y: void 0, i: LOOP$0.i},",
" LOOP$0.i++) {",
" LOOP$0.y =" + " LOOP$0.i;",
" arr.push((function(LOOP$0$PARAM$1) {",
Expand Down Expand Up @@ -739,7 +738,7 @@ public void testLoopClosure() {
"var i = 0;",
"for (; i < 10; ",
" LOOP$0 = {",
" i$jscomp$1: LOOP$0.i$jscomp$1",
" i$jscomp$1: void 0",
" },",
" i++) {",
" LOOP$0.i$jscomp$1 = x + 1;",
Expand Down Expand Up @@ -773,7 +772,7 @@ public void testLoopClosure() {
"var i = 0;",
"for (; i < 10;",
" LOOP$0 = {",
" i$jscomp$1: LOOP$0.i$jscomp$1",
" i$jscomp$1: void 0",
" }, i++) {",
" arr.push((function(LOOP$0$PARAM$1) {",
" return function() {",
Expand Down Expand Up @@ -810,8 +809,8 @@ public void testLoopClosure() {
"var i = 0;",
"for (; i < 10;",
" LOOP$0 = {",
" i$jscomp$2: LOOP$0.i$jscomp$2,",
" i$jscomp$1: LOOP$0.i$jscomp$1",
" i$jscomp$2: void 0,",
" i$jscomp$1: void 0",
" }, i++) {",
" if (true) {",
" LOOP$0.i$jscomp$2 = x - 1;",
Expand All @@ -837,7 +836,7 @@ public void testLoopClosure() {
expected(
lines(
"var LOOP$0 = {};",
"for (;;LOOP$0 = {x: LOOP$0.x}) {",
"for (;;LOOP$0 = {x: void 0}) {",
" LOOP$0.x = 3;",
" var f = function(LOOP$0$PARAM$1) {",
" return function() { return LOOP$0$PARAM$1.x; }",
Expand All @@ -850,7 +849,7 @@ public void testLoopClosure() {
expected(
lines(
"var LOOP$0 = {};",
"for (;; LOOP$0 = { x: LOOP$0.x }) {",
"for (;; LOOP$0 = { x: void 0 }) {",
" LOOP$0.x = 3;",
" var f = function(LOOP$0$PARAM$1) {",
" return function() { return LOOP$0$PARAM$1.x; }",
Expand All @@ -864,7 +863,7 @@ public void testLoopClosure() {
expected(
lines(
"var LOOP$0 = {};",
"for (;; LOOP$0 = { x: LOOP$0.x}) {",
"for (;; LOOP$0 = { x: void 0 }) {",
" /** @const */ LOOP$0.x = 3;",
" var f = function(LOOP$0$PARAM$1) {",
" return function() { return LOOP$0$PARAM$1.x}",
Expand All @@ -881,7 +880,7 @@ public void testLoopClosure() {
expected(
lines(
"var LOOP$0 = {};",
"for (;;LOOP$0 = {x: LOOP$0.x, y: LOOP$0.y}) {",
"for (;;LOOP$0 = {x: void 0, y: void 0}) {",
" LOOP$0.x = 3;",
" LOOP$0.y = 4;",
" var f = function(LOOP$0$PARAM$1) {",
Expand All @@ -902,7 +901,7 @@ public void testLoopClosure() {
expected(
lines(
"var LOOP$0 = {};",
"for (;;LOOP$0 = { x: LOOP$0.x, y: LOOP$0.y })" + " {",
"for (;;LOOP$0 = { x: void 0, y: void 0 })" + " {",
" /** @const */ LOOP$0.x = 3;",
" /** @const */ LOOP$0.y = 4;",
" var f = function(LOOP$0$PARAM$1) {",
Expand All @@ -926,7 +925,7 @@ public void testLoopClosure() {
"var i;",
"var LOOP$0={};",
"i = 0;",
"for(;; LOOP$0 = { x: LOOP$0.x }) {",
"for(;; LOOP$0 = { x: void 0 }) {",
" LOOP$0.x = 0;",
" var f = (function(LOOP$0$PARAM$1) {",
" return function() { LOOP$0$PARAM$1.x; };",
Expand All @@ -946,7 +945,7 @@ public void testLoopClosure() {
lines(
"var LOOP$0={};",
"foo();",
"for(;; LOOP$0 = { x: LOOP$0.x }) {",
"for(;; LOOP$0 = { x: void 0 }) {",
" LOOP$0.x = 0;",
" var f = (function(LOOP$0$PARAM$1) {",
" return function() { LOOP$0$PARAM$1.x; };",
Expand All @@ -966,7 +965,7 @@ public void testLoopClosure() {
lines(
"var LOOP$0={};",
"(function foo() {});",
"for(;; LOOP$0 = { x: LOOP$0.x }) {",
"for(;; LOOP$0 = { x: void 0 }) {",
" LOOP$0.x = 0;",
" var f = (function(LOOP$0$PARAM$1) {",
" return function() { LOOP$0$PARAM$1.x; };",
Expand All @@ -986,7 +985,7 @@ public void testLoopClosure() {
expected(
lines(
"var LOOP$0 = {};",
"for(;; LOOP$0 = {x: LOOP$0.x}) {",
"for(;; LOOP$0 = {x: void 0}) {",
" LOOP$0.x = void 0;",
" foo(function(LOOP$0$PARAM$1) {",
" return function() {",
Expand Down Expand Up @@ -1064,7 +1063,7 @@ public void testLoopClosureWithContinue() {
"var LOOP$0 = {};",
"while (values.length > 0) {",
" LOOP$0 = {",
" v: LOOP$0.v",
" v: void 0",
" };",
" /** @const */ LOOP$0.v = values.shift();",
" if (LOOP$0.v == 'skipMe') {",
Expand Down Expand Up @@ -1101,7 +1100,7 @@ public void testLoopClosureWithContinue() {
"var LOOP$0 = {};",
"do {",
" LOOP$0 = {",
" v: LOOP$0.v",
" v: void 0",
" };",
" /** @const */ LOOP$0.v = values.shift();",
" if (LOOP$0.v == 'skipMe') {",
Expand Down Expand Up @@ -1139,7 +1138,7 @@ public void testLoopClosureWithContinue() {
"var LOOP$0 = {};",
"LABEL: while (values.length > 0) {",
" LOOP$0 = {",
" v: LOOP$0.v",
" v: void 0",
" };",
" /** @const */ LOOP$0.v = values.shift();",
" if (LOOP$0.v == 'skipMe') {",
Expand Down Expand Up @@ -1189,14 +1188,14 @@ public void testLoopClosureWithContinue() {
"var LOOP$1 = {};",
"OUTER: while (values.length > 0) {",
" LOOP$1 = {",
" v: LOOP$1.v",
" v: void 0",
" };",
" /** @const */ LOOP$1.v = values.shift();",
" var i = 0;",
" var LOOP$0 = {};",
" while (i < LOOP$1.v.length) {",
" LOOP$0 = {",
" c: LOOP$0.c",
" c: void 0",
" };",
" /** @const */ LOOP$0.c = LOOP$1.v.charAt(i);",
" if (LOOP$0.c == 'a') {",
Expand Down Expand Up @@ -1244,8 +1243,8 @@ public void testLoopClosureCommaInBody() {
"var i = 0;",
"for (; i < 10;",
" LOOP$0 = {",
" i$jscomp$1: LOOP$0.i$jscomp$1,",
" j$jscomp$1: LOOP$0.j$jscomp$1",
" i$jscomp$1: void 0,",
" j$jscomp$1: void 0",
" },",
" i++) {",
" LOOP$0.i$jscomp$1 = void 0;",
Expand Down Expand Up @@ -1654,7 +1653,7 @@ public void testForIn() {
expected(
lines(
"var LOOP$0 = {};",
"for (;; LOOP$0 = { a: LOOP$0.a }) {",
"for (;; LOOP$0 = { a: void 0 }) {",
" LOOP$0.a = getArray();",
" f = (function(LOOP$0$PARAM$1) {",
" return function() {",
Expand Down Expand Up @@ -1691,7 +1690,7 @@ public void testDoWhileForInCapturedLet() {
"var LOOP$1 = {};",
"do {",
" LOOP$1 = {",
" special: LOOP$1.special",
" special: void 0",
" };",
" LOOP$1.special = 99;",
" /** @const */",
Expand Down Expand Up @@ -1748,7 +1747,7 @@ public void testFunctionsInLoop() {
"while (true) {",
" LOOP$0 =",
" {",
" x: LOOP$0.x",
" x: void 0",
" };",
" LOOP$0.x = null;",
" var f = function(LOOP$0$PARAM$1) {",
Expand All @@ -1774,7 +1773,7 @@ public void testFunctionsInLoop() {
"var LOOP$0 = {};",
"while (true) {",
" LOOP$0 =",
" { x: LOOP$0.x };",
" { x: void 0 };",
" LOOP$0.x = null;",
" var f = function(LOOP$0$PARAM$1) {",
" return function() {",
Expand All @@ -1799,7 +1798,7 @@ public void testFunctionsInLoop() {
"var LOOP$0 = {};",
"while (true) {",
" LOOP$0 =",
" { x: LOOP$0.x };",
" { x: void 0 };",
" LOOP$0.x = null;",
" (function(LOOP$0$PARAM$1) {",
" return function() {",
Expand All @@ -1808,6 +1807,35 @@ public void testFunctionsInLoop() {
" })(LOOP$0)();",
"}"));
loopClosureTest(srcs, expected);

// NOTE: if the function itself is closed over then it must be included in the loop object
srcs =
srcs(
lines(
"while (true) {",
" let x = 1;",
" let f = function(i) {",
" if (i < x) f(i + 1);",
" };",
" use(f);",
"}"));
expected =
expected(
lines(
"var LOOP$0 = {};",
"while (true) {",
" LOOP$0 = {x: void 0, f: void 0};",
" LOOP$0.x = 1;",
" LOOP$0.f = (function(LOOP$0$PARAM$1) {",
" return function(i) {",
" if (i < LOOP$0$PARAM$1.x) {",
" (0,LOOP$0$PARAM$1.f)(i + 1);",
" }",
" };",
" })(LOOP$0);",
" use(LOOP$0.f);",
"}"));
loopClosureTest(srcs, expected);
}

// https://github.com/google/closure-compiler/issues/1557
Expand All @@ -1829,8 +1857,8 @@ public void testNormalizeDeclarations() {
"var LOOP$0 = {};",
"while (true) {",
" LOOP$0 = {",
" x: LOOP$0.x,",
" y: LOOP$0.y",
" x: void 0,",
" y: void 0",
" };",
" LOOP$0.x = void 0;",
" LOOP$0.y = void 0;",
Expand Down Expand Up @@ -1859,8 +1887,8 @@ public void testNormalizeDeclarations() {
"var LOOP$0 = {};",
"while (true) {",
" LOOP$0 = {",
" y: LOOP$0.y,",
" x: LOOP$0.x",
" y: void 0,",
" x: void 0",
" };",
" LOOP$0.x = void 0;",
" LOOP$0.y = void 0;",
Expand Down Expand Up @@ -1952,9 +1980,9 @@ public void testLetForInitializers() {
" var vx = 1, vy = 2, vz = 3;",
" for (; vx < 10; ",
" LOOP$0 = {",
" lx: LOOP$0.lx,",
" ly: LOOP$0.ly,",
" lz: LOOP$0.lz",
" lx: void 0,",
" ly: void 0,",
" lz: void 0",
" },",
" vx++){",
" LOOP$0.lx = vx;",
Expand Down Expand Up @@ -2029,7 +2057,7 @@ public void testReferenceToLoopScopedLetInObjectGetterAndSetter() {
"var LOOP$0 = {};",
"var i = 0;",
"for (; i < 2;",
" LOOP$0 = { bar:LOOP$0.bar }, i++) {",
" LOOP$0 = { bar: void 0 }, i++) {",
" LOOP$0.bar = 42;",
// Note that we wrap the entire object literal in an IIFE, because that's simpler
// than trying to individually wrap the getter and setter methods defined in it.
Expand Down Expand Up @@ -2071,7 +2099,7 @@ public void testReferenceToMultipleLoopScopedLetConstInObjectWithGetter() {
"var LOOP$0 = {};",
"var i = 0;",
"for (; i < 2;",
" LOOP$0 = { bar: LOOP$0.bar, baz:" + " LOOP$0.baz },",
" LOOP$0 = { bar: void 0, baz: void 0 },",
" i++) {",
" LOOP$0.bar = 42;",
" /** @const */",
Expand Down Expand Up @@ -2109,8 +2137,8 @@ public void testReferenceToMultipleLoopScopedLetsInObjectWithSetter() {
"var i = 0;",
"for (; i < 2;",
" LOOP$0 = {",
" bar: LOOP$0.bar,",
" baz: LOOP$0.baz",
" bar: void 0,",
" baz: void 0",
" },",
" i++) {",
" LOOP$0.bar = 42;",
Expand Down

0 comments on commit c7049f8

Please sign in to comment.