Shouldn't unnecessarily chop up loops with abrupt completions

[monsanto]

x-post from https://github.com/babel/babel/issues/590

By abrupts, I am referring to break, continue, throw, return, etc.

For example, if you compile the following:

function *foo() {
  for (;;) {
    bar()
    break
  }
  yield baz()
}

you get

var foo = regeneratorRuntime.mark(function foo() {
  return regeneratorRuntime.wrap(function foo$(context$1$0) {
    while (1) switch (context$1$0.prev = context$1$0.next) {
      case 0:
        bar();
        return context$1$0.abrupt("break", 4);
      case 2:
        context$1$0.next = 0;
        break;
      case 4:
        context$1$0.next = 6;
        return baz();
      case 6:
      case "end":
        return context$1$0.stop();
    }
  }, foo, this);
});

I would expect Regenerator to not chop up the loop like that, unless the loop contained a yield. Instead, it should generate something like this, which is more readable and performant:

var foo = regeneratorRuntime.mark(function foo() {
  return regeneratorRuntime.wrap(function foo$(context$1$0) {
    while (1) switch (context$1$0.prev = context$1$0.next) {
      case 0:
        for (;;) {
          bar();
          break;
        }
        context$1$0.next = 3;
        return baz();
      case 3:
      case "end":
        return context$1$0.stop();
    }
  }, foo, this);
});

(this is the code Regenerator generates if you don't have the break; I manually added it to the final result)

[benjamn]

This issue is exactly the sort of optimization opportunity I was hoping to surface with #180, so thanks very much for reporting it!

Fixing this issue will involve refining the notion of a "leap" to mean "control flow that escapes the current statement and jumps somewhere other than immediately after the statement" rather than just "control flow that might terminate the current statement." The code in lib/meta.js that implements meta.containsLeap is overly conservative, and I've barely touched it since the initial release, so I'm really happy to have a chance to improve it.

Specifically, break and continue statements usually do not escape their enclosing loops, and even when they do there may not be any need to explode the loop. For example, these nested while loops can be emitted intact, even though the break escapes the inner loop:

outer:
while (true) {
  while (true) {
    break outer;
  }
}

The mere presence of the break statement currently causes the loops to be exploded, which produces correct but obviously suboptimal code.

Now, if the outer loop contains a return or a yield, then it will have to be exploded (chopped up), which means it will no longer encapsulate the break outer, which means the inner loop will have to be regarded has containing a "leap," and the break outer will have to be translated to something like return context.abrupt("break", ...). That's the kind of reasoning that meta.containsLeap needs to start doing, in order to resolve this issue.

The return and yield keywords will always result in context.abrupt calls and exploded control flow, but break, continue, and throw statements that are statically contained by a try-catch are all worth reconsidering, since in many cases we may be able to emit them without any modifications.

[ghost]

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.