Invalid code generated for async functions containing multiple for..of loops with same variable

[Treeki]

I ran into a bizarre error after having rewritten some code to use Promises, which I'll include here for the benefit of anyone who may be searching for it:

TypeError: Cannot read property 'done' of undefined
    at $$jscomp$generator$Engine_$.program_ (/Users/ash/src/furcng/tmp.js:502:37)
    at $$jscomp$generator$Engine_$$nextStep_$ [as nextStep_] (/Users/ash/src/furcng/tmp.js:456:38)
    at $$jscomp$generator$Engine_$$next_$ [as next_] (/Users/ash/src/furcng/tmp.js:417:15)
    at $$jscomp$generator$Generator_$.$this$next$ [as next] (/Users/ash/src/furcng/tmp.js:477:22)
    at $passValueToGenerator$$ (/Users/ash/src/furcng/tmp.js:277:25)

If an async function contains multiple for...of loops sharing the same variable name, some of which contain await and some of which do not contain await, Closure Compiler will generate invalid code for accessing the variable in the latter.

Here is a minimal test case which reproduces the issue.

async function breakIt() {
	for (const number of [1,2,3]) {
		await Promise.resolve(true)
	}

	for (const number of [1,2,3]) {
	}
}

breakIt().then(() => console.log('done'))

The generated code (with options --formatting=PRETTY_PRINT --debug=true) is as follows:

function breakIt() {
  return $jscomp.asyncExecutePromiseGeneratorFunction(function $jscomp$generator$function() {
    var $$jscomp$iter$0$$;
    return $jscomp.generator.createGenerator($jscomp$generator$function, function($$jscomp$generator$context$$) {
      switch($$jscomp$generator$context$$.nextAddress) {
        case 1:
          $$jscomp$iter$0$$ = $jscomp.makeIterator([1, 2, 3]), $$jscomp$key$number$$ = $$jscomp$iter$0$$.next();
        case 2:
          if ($$jscomp$key$number$$.done) {
            $$jscomp$generator$context$$.jumpTo(4);
            break;
          }
          return $$jscomp$generator$context$$.yield(Promise.resolve(!0), 3);
        case 3:
          $$jscomp$iter$0$$.next();
          $$jscomp$generator$context$$.jumpTo(2);
          break;
        case 4:
          for (var $$jscomp$iter$1$$ = $jscomp.makeIterator([1, 2, 3]), $$jscomp$key$number$$ = $$jscomp$iter$1$$.next(); !$$jscomp$key$number$$.done; $$jscomp$key$number$$ = $$jscomp$iter$1$$.next()) {
          }
          $$jscomp$generator$context$$.jumpToEnd();
      }
    });
  });
}

Note that the number variable has been renamed to $$jscomp$key$number$$ for both loops. However, the variable is declared as part of the second loop (without await), which breaks the first loop as its value is cleared between generator calls. If the second loop is removed, $$jscomp$key$number$$ is declared outside the generator, and the first loop functions correctly.

[lauraharker]

Thanks for the report!

It looks like this is a problem with our generator transpilation that's triggered by how we rewrite for-of loops. Here's a repro where the input is the code from above, just before reaching generator transpilation:

https://closure-compiler-debugger.appspot.com/#input0%3Dfunction%2520*breakIt()%2520%257B%250A%2520%2520for%2520(var%2520%2524jscomp%2524iter%25240%2520%253D%2520%2524jscomp.makeIterator(%255B1%252C%25202%252C%25203%255D)%252C%250A%2520%2520%2520%2520%2520%2520%2520%2520%2524jscomp%2524key%2524number%2520%253D%2520%2524jscomp%2524iter%25240.next()%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520!%2524jscomp%2524key%2524number.done%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2524jscomp%2524key%2524number%2520%2520%253D%2520%2524jscomp%2524iter%25240.next())%2520%257B%250A%2520%2520%2520%2520%252F**%2520%2540const%2520*%252F%2520var%2520number%2520%253D%2520%2524jscomp%2524key%2524number.value%253B%250A%2520%2520%2520%2520%257B%2520%250A%2520%2520%2520%2520%2520%2520yield%25201%253B%250A%2520%2520%2520%2520%257D%250A%2520%2520%257D%250A%250A%2520%2520for%2520(var%2520%2524jscomp%2524iter%25241%2520%253D%2520%2524jscomp.makeIterator(%255B1%252C%25202%252C%25203%255D)%252C%250A%2520%2520%2520%2520%2520%2520%2520%2520%2524jscomp%2524key%2524number%2520%253D%2520%2524jscomp%2524iter%25241.next()%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520!%2524jscomp%2524key%2524number.done%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2524jscomp%2524key%2524number%2520%2520%253D%2520%2524jscomp%2524iter%25241.next())%2520%257B%250A%2520%2520%2520%2520%252F**%2520%2540const%2520*%252F%2520var%2520number%25240%2520%253D%2520%2524jscomp%2524key%2524number.value%253B%250A%250A%2520%2520%257D%250A%257D%26input1%26conformanceConfig%26externs%3Dvar%2520%2524jscomp%253B%26refasterjs-template%26includeDefaultExterns%3Dtrue%26CHECK_SYMBOLS%3Dtrue%26MISSING_PROPERTIES%3Dtrue%26TRANSPILE%3Dtrue%26SKIP_NON_TRANSPILATION_PASSES%3Dtrue%26CHECK_TYPES%3Dtrue%26CLOSURE_PASS%3Dtrue%26PRESERVE_TYPE_ANNOTATIONS%3Dtrue%26PRETTY_PRINT%3Dtrue

[lauraharker]

A Googler is working on a fix for this internally.

[lauraharker]

For now, using different variable names in each for-of loop will work around the bug.

It appears to happen only when one for loop has an await and the other does not.