New early-return step in Evaluate() violates invariants

[Ms2ger]

https://tc39.github.io/proposal-top-level-await/#sec-moduleevaluation

Consider the following modules:

• A: depends on B, [[ModuleAsync]] = true
• B: no deps, [[ModuleAsync]] = false, fails when executing

We run A.Instantiate() with no issues. All [[Status]] fields are "instantiated".

We then run A.Evaluate(). B.ExecuteModule() returns an abrupt completion, which is passed through InnerModuleEvaluation(B) (step 12.a) and InnerModuleEvaluation(A) (step 11.c) back to A.Evaluate(). At this point, stack is [A, B] and all modules have [[Status]] set to "evaluating". Because we encounter the async module A in stack, we return early, without setting the [[Status]] fields to "evaluated".

This violates the claim above the algorithm:

Evaluate transitions this module's [[Status]] from "instantiated" to "evaluated".

and makes it impossible to call Evaluate() again, because of:

Assert: module.[[Status]] is "instantiated" or "evaluated".

[littledan]

Well, oops, good catch. The early return is intended to stop iterating up the stack when the Promise reactions would carry things through. Sounds like it needs some rethinking for this sync -> async transition. Note, all this logic would be done differently in #71.

[guybedford]

The early return at 5. (a) (ii) is a bug in this spec, and that line should be removed. All items on the stack should be transitioned to evaluated on error, as that code block achieves.

I actually already included that one line fix in #71.

[littledan]

This should be fixed now that #74 has landed. It'd be great to have your review on that as well!