[modules] Make Module::Evaluate return the completion value.

This is more useful than always returning undefined.

BUG=v8:1569,v8:5978

Change-Id: Id10cf87f7865db1a85de412460eaead4e4bf3b62
Reviewed-on: https://chromium-review.googlesource.com/446846Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43498}

include/v8.h

@@ -1095,6 +1095,8 @@ class V8_EXPORT Module {
 
   /**
    * ModuleEvaluation
+   *
+   * Returns the completion value.
    */
   V8_WARN_UNUSED_RESULT MaybeLocal<Value> Evaluate(Local<Context> context);
 };

src/objects-inl.h

@@ -8215,6 +8215,16 @@ static inline Handle<Object> MakeEntryPair(Isolate* isolate, Handle<Name> key,
                                                     FAST_ELEMENTS, 2);
 }
 
+JSIteratorResult* JSIteratorResult::cast(Object* object) {
+  SLOW_DCHECK(object->IsJSObject() &&
+              JSObject::cast(object)->map() ==
+                  JSObject::cast(object)
+                      ->GetIsolate()
+                      ->native_context()
+                      ->iterator_result_map());
+  return reinterpret_cast<JSIteratorResult*>(object);
+}
+
 ACCESSORS(JSIteratorResult, value, Object, kValueOffset)
 ACCESSORS(JSIteratorResult, done, Object, kDoneOffset)
 

src/objects.cc

@@ -20027,7 +20027,12 @@ MaybeHandle<Object> Module::Evaluate(Handle<Module> module) {
   // Evaluation of module body.
   Handle<JSFunction> resume(
       isolate->native_context()->generator_next_internal(), isolate);
-  return Execution::Call(isolate, resume, generator, 0, nullptr);
+  Handle<Object> result;
+  ASSIGN_RETURN_ON_EXCEPTION(
+      isolate, result, Execution::Call(isolate, resume, generator, 0, nullptr),
+      Object);
+  DCHECK(Handle<JSIteratorResult>::cast(result)->done()->BooleanValue());
+  return handle(Handle<JSIteratorResult>::cast(result)->value(), isolate);
 }
 
 namespace {

src/objects.h

@@ -2639,8 +2639,9 @@ class JSDataPropertyDescriptor: public JSObject {
 // as specified by ES6 section 25.1.1.3 The IteratorResult Interface
 class JSIteratorResult: public JSObject {
  public:
-  DECL_ACCESSORS(value, Object)
+  DECLARE_CAST(JSIteratorResult)
 
+  DECL_ACCESSORS(value, Object)
   DECL_ACCESSORS(done, Object)
 
   // Offsets of object fields.

src/parsing/parser.cc

@@ -1264,7 +1264,11 @@ Statement* Parser::ParseExportDefault(bool* ok) {
 
       Assignment* assignment = factory()->NewAssignment(
           Token::INIT, decl->proxy(), value, kNoSourcePosition);
-      result = factory()->NewExpressionStatement(assignment, kNoSourcePosition);
+      Block* block = factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
+      block->statements()->Add(
+          factory()->NewExpressionStatement(assignment, kNoSourcePosition),
+          zone());
+      result = block;
 
       ExpectSemicolon(CHECK_OK);
       break;

src/parsing/rewriter.cc

@@ -112,10 +112,8 @@ class Processor final : public AstVisitor<Processor> {
 
 
 Statement* Processor::AssignUndefinedBefore(Statement* s) {
-  Expression* result_proxy = factory()->NewVariableProxy(result_);
   Expression* undef = factory()->NewUndefinedLiteral(kNoSourcePosition);
-  Expression* assignment = factory()->NewAssignment(Token::ASSIGN, result_proxy,
-                                                    undef, kNoSourcePosition);
+  Expression* assignment = SetResult(undef);
   Block* b = factory()->NewBlock(NULL, 2, false, kNoSourcePosition);
   b->statements()->Add(
       factory()->NewExpressionStatement(assignment, kNoSourcePosition), zone());
@@ -366,18 +364,41 @@ bool Rewriter::Rewrite(ParseInfo* info) {
   DCHECK_NOT_NULL(function);
   Scope* scope = function->scope();
   DCHECK_NOT_NULL(scope);
-  if (!scope->is_script_scope() && !scope->is_eval_scope()) return true;
+  DCHECK_EQ(scope, scope->GetClosureScope());
 
-  DeclarationScope* closure_scope = scope->GetClosureScope();
+  if (!(scope->is_script_scope() || scope->is_eval_scope() ||
+        scope->is_module_scope())) {
+    return true;
+  }
 
   ZoneList<Statement*>* body = function->body();
+  DCHECK_IMPLIES(scope->is_module_scope(), !body->is_empty());
   if (!body->is_empty()) {
-    Variable* result = closure_scope->NewTemporary(
+    Variable* result = scope->AsDeclarationScope()->NewTemporary(
         info->ast_value_factory()->dot_result_string());
     Processor processor(info->isolate()->stack_guard()->real_climit(),
-                        closure_scope, result, info->ast_value_factory());
+                        scope->AsDeclarationScope(), result,
+                        info->ast_value_factory());
     processor.Process(body);
 
+    DCHECK_IMPLIES(scope->is_module_scope(), processor.result_assigned());
+    if (processor.result_assigned()) {
+      int pos = kNoSourcePosition;
+      Expression* result_value =
+          processor.factory()->NewVariableProxy(result, pos);
+      if (scope->is_module_scope()) {
+        auto args = new (info->zone()) ZoneList<Expression*>(2, info->zone());
+        args->Add(result_value, info->zone());
+        args->Add(processor.factory()->NewBooleanLiteral(true, pos),
+                  info->zone());
+        result_value = processor.factory()->NewCallRuntime(
+            Runtime::kInlineCreateIterResultObject, args, pos);
+      }
+      Statement* result_statement =
+          processor.factory()->NewReturnStatement(result_value, pos);
+      body->Add(result_statement, info->zone());
+    }
+
     // TODO(leszeks): Remove this check and releases once internalization is
     // moved out of parsing/analysis.
     DCHECK(ThreadId::Current().Equals(info->isolate()->thread_id()));
@@ -388,15 +409,6 @@ bool Rewriter::Rewrite(ParseInfo* info) {
     // Internalize any values created during rewriting.
     info->ast_value_factory()->Internalize(info->isolate());
     if (processor.HasStackOverflow()) return false;
-
-    if (processor.result_assigned()) {
-      int pos = kNoSourcePosition;
-      VariableProxy* result_proxy =
-          processor.factory()->NewVariableProxy(result, pos);
-      Statement* result_statement =
-          processor.factory()->NewReturnStatement(result_proxy, pos);
-      body->Add(result_statement, info->zone());
-    }
   }
 
   return true;

test/cctest/interpreter/bytecode_expectations/Modules.golden

@@ -13,7 +13,7 @@ snippet: "
 "
 frame size: 9
 parameter count: 2
-bytecode array length: 131
+bytecode array length: 145
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -70,7 +70,13 @@ bytecodes: [
   /*   13 S> */ B(Return),
                 B(Ldar), R(5),
   /*    0 E> */ B(Throw),
-                B(LdaUndefined),
+                B(Ldar), R(5),
+                B(StaCurrentContextSlot), U8(6),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(3),
+                B(LdaTrue),
+                B(Star), R(4),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(3), U8(2),
   /*   13 S> */ B(Return),
 ]
 constant pool: [
@@ -85,7 +91,7 @@ snippet: "
 "
 frame size: 9
 parameter count: 2
-bytecode array length: 131
+bytecode array length: 145
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -142,7 +148,13 @@ bytecodes: [
   /*   24 S> */ B(Return),
                 B(Ldar), R(5),
   /*    0 E> */ B(Throw),
-                B(LdaUndefined),
+                B(Ldar), R(5),
+                B(StaCurrentContextSlot), U8(6),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(3),
+                B(LdaTrue),
+                B(Star), R(4),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(3), U8(2),
   /*   24 S> */ B(Return),
 ]
 constant pool: [
@@ -159,7 +171,7 @@ snippet: "
 "
 frame size: 10
 parameter count: 2
-bytecode array length: 201
+bytecode array length: 215
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -245,8 +257,13 @@ bytecodes: [
                 B(LdaSmi), I8(42),
                 B(Star), R(6),
   /*   52 E> */ B(Call), R(4), R(5), U8(2), U8(4),
+                B(StaContextSlot), R(1), U8(6), U8(0),
                 B(PopContext), R(1),
-                B(LdaUndefined),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(4),
+                B(LdaTrue),
+                B(Star), R(5),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(4), U8(2),
   /*   64 S> */ B(Return),
 ]
 constant pool: [
@@ -265,7 +282,7 @@ snippet: "
 "
 frame size: 10
 parameter count: 2
-bytecode array length: 166
+bytecode array length: 186
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -335,10 +352,18 @@ bytecodes: [
   /*   34 S> */ B(LdaUndefined),
   /*   34 E> */ B(StaCurrentContextSlot), U8(4),
   /*   39 S> */ B(LdaModuleVariable), I8(1), U8(1),
+                B(ToNumber), R(4),
+                B(Ldar), R(4),
                 B(Inc), U8(3),
   /*   42 E> */ B(StaModuleVariable), I8(1), U8(1),
+                B(Ldar), R(4),
+                B(StaContextSlot), R(1), U8(6), U8(0),
                 B(PopContext), R(1),
-                B(LdaUndefined),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(4),
+                B(LdaTrue),
+                B(Star), R(5),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(4), U8(2),
   /*   49 S> */ B(Return),
 ]
 constant pool: [
@@ -356,7 +381,7 @@ snippet: "
 "
 frame size: 10
 parameter count: 2
-bytecode array length: 170
+bytecode array length: 190
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -428,10 +453,18 @@ bytecodes: [
   /*   34 S> */ B(LdaUndefined),
   /*   34 E> */ B(StaCurrentContextSlot), U8(4),
   /*   39 S> */ B(LdaModuleVariable), I8(1), U8(1),
+                B(ToNumber), R(4),
+                B(Ldar), R(4),
                 B(Inc), U8(3),
   /*   42 E> */ B(StaModuleVariable), I8(1), U8(1),
+                B(Ldar), R(4),
+                B(StaContextSlot), R(1), U8(6), U8(0),
                 B(PopContext), R(1),
-                B(LdaUndefined),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(4),
+                B(LdaTrue),
+                B(Star), R(5),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(4), U8(2),
   /*   49 S> */ B(Return),
 ]
 constant pool: [
@@ -449,7 +482,7 @@ snippet: "
 "
 frame size: 10
 parameter count: 2
-bytecode array length: 174
+bytecode array length: 194
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -521,10 +554,18 @@ bytecodes: [
   /*   36 S> */ B(LdaUndefined),
   /*   36 E> */ B(StaCurrentContextSlot), U8(4),
   /*   41 S> */ B(LdaModuleVariable), I8(1), U8(1),
+                B(ToNumber), R(4),
+                B(Ldar), R(4),
                 B(Inc), U8(3),
   /*   44 E> */ B(CallRuntime), U16(Runtime::kThrowConstAssignError), R(0), U8(0),
+                B(Ldar), R(4),
+                B(StaContextSlot), R(1), U8(6), U8(0),
                 B(PopContext), R(1),
-                B(LdaUndefined),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(4),
+                B(LdaTrue),
+                B(Star), R(5),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(4), U8(2),
   /*   51 S> */ B(Return),
 ]
 constant pool: [
@@ -540,7 +581,7 @@ snippet: "
 "
 frame size: 9
 parameter count: 2
-bytecode array length: 142
+bytecode array length: 156
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -599,9 +640,15 @@ bytecodes: [
   /*   32 S> */ B(Return),
                 B(Ldar), R(5),
   /*    0 E> */ B(Throw),
+                B(Ldar), R(5),
+                B(StaCurrentContextSlot), U8(6),
                 B(CreateClosure), U8(1), U8(2), U8(0),
                 B(StaModuleVariable), I8(1), U8(0),
-                B(LdaUndefined),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(3),
+                B(LdaTrue),
+                B(Star), R(4),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(3), U8(2),
   /*   32 S> */ B(Return),
 ]
 constant pool: [
@@ -617,7 +664,7 @@ snippet: "
 "
 frame size: 9
 parameter count: 2
-bytecode array length: 179
+bytecode array length: 193
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -676,6 +723,8 @@ bytecodes: [
   /*   26 S> */ B(Return),
                 B(Ldar), R(5),
   /*    0 E> */ B(Throw),
+                B(Ldar), R(5),
+                B(StaCurrentContextSlot), U8(7),
   /*   16 S> */ B(CreateClosure), U8(1), U8(2), U8(0),
                 B(Star), R(3),
                 B(LdaTheHole),
@@ -692,7 +741,11 @@ bytecodes: [
                 B(StaCurrentContextSlot), U8(6),
                 B(LdaCurrentContextSlot), U8(6),
   /*   16 E> */ B(StaModuleVariable), I8(1), U8(0),
-                B(LdaUndefined),
+                B(LdaCurrentContextSlot), U8(7),
+                B(Star), R(3),
+                B(LdaTrue),
+                B(Star), R(4),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(3), U8(2),
   /*   26 S> */ B(Return),
 ]
 constant pool: [
@@ -708,7 +761,7 @@ snippet: "
 "
 frame size: 9
 parameter count: 2
-bytecode array length: 131
+bytecode array length: 145
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -765,7 +818,13 @@ bytecodes: [
   /*   30 S> */ B(Return),
                 B(Ldar), R(5),
   /*    0 E> */ B(Throw),
-                B(LdaUndefined),
+                B(Ldar), R(5),
+                B(StaCurrentContextSlot), U8(6),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(3),
+                B(LdaTrue),
+                B(Star), R(4),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(3), U8(2),
   /*   30 S> */ B(Return),
 ]
 constant pool: [
@@ -780,7 +839,7 @@ snippet: "
 "
 frame size: 9
 parameter count: 2
-bytecode array length: 131
+bytecode array length: 145
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -837,7 +896,13 @@ bytecodes: [
   /*   19 S> */ B(Return),
                 B(Ldar), R(5),
   /*    0 E> */ B(Throw),
-                B(LdaUndefined),
+                B(Ldar), R(5),
+                B(StaCurrentContextSlot), U8(6),
+                B(LdaCurrentContextSlot), U8(6),
+                B(Star), R(3),
+                B(LdaTrue),
+                B(Star), R(4),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(3), U8(2),
   /*   19 S> */ B(Return),
 ]
 constant pool: [
@@ -853,7 +918,7 @@ snippet: "
 "
 frame size: 9
 parameter count: 2
-bytecode array length: 170
+bytecode array length: 182
 bytecodes: [
                 B(Ldar), R(new_target),
                 B(JumpIfUndefined), U8(28),
@@ -925,7 +990,12 @@ bytecodes: [
   /*   41 E> */ B(LdaNamedProperty), R(6), U8(2), U8(6),
                 B(Star), R(6),
   /*   31 E> */ B(CallProperty), R(3), R(4), U8(3), U8(2),
-                B(LdaUndefined),
+                B(StaCurrentContextSlot), U8(7),
+                B(LdaCurrentContextSlot), U8(7),
+                B(Star), R(3),
+                B(LdaTrue),
+                B(Star), R(4),
+                B(InvokeIntrinsic), U8(Runtime::k_CreateIterResultObject), R(3), U8(2),
   /*   45 S> */ B(Return),
 ]
 constant pool: [

test/cctest/test-modules.cc

@@ -98,4 +98,67 @@ TEST(ModuleEvaluation) {
   ExpectInt32("Object.expando", 10);
 }
 
+TEST(ModuleEvaluationCompletion1) {
+  Isolate* isolate = CcTest::isolate();
+  HandleScope scope(isolate);
+  LocalContext env;
+
+  const char* sources[] = {
+      "", "var a = 1", "import '42'", "export * from '42'",
+      "export {} from '42'", "export {}", "var a = 1; export {a}",
+      "export function foo() {}",
+      // TODO(neis): v8:6022 "export class C extends null {}",
+      "export let a = 1", "export default 1",
+      "export default function foo() {}", "export default function () {}",
+      "export default (function () {})",
+      // TODO(neis): v8:6022 "export default class C extends null {}",
+      "export default (class C extends null {})",
+      "for (var i = 0; i < 5; ++i) {}",
+  };
+
+  for (auto src : sources) {
+    Local<String> source_text = v8_str(src);
+    ScriptOrigin origin = ModuleOrigin(v8_str("file.js"), CcTest::isolate());
+    ScriptCompiler::Source source(source_text, origin);
+    Local<Module> module =
+        ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
+    CHECK(module->Instantiate(env.local(),
+                              CompileSpecifierAsModuleResolveCallback));
+    CHECK(module->Evaluate(env.local()).ToLocalChecked()->IsUndefined());
+  }
+}
+
+TEST(ModuleEvaluationCompletion2) {
+  Isolate* isolate = CcTest::isolate();
+  HandleScope scope(isolate);
+  LocalContext env;
+
+  const char* sources[] = {
+      "'gaga'; ", "'gaga'; var a = 1", "'gaga'; import '42'",
+      "'gaga'; export * from '42'", "'gaga'; export {} from '42'",
+      "'gaga'; export {}", "'gaga'; var a = 1; export {a}",
+      "'gaga'; export function foo() {}",
+      // TODO(neis): v8:6022 "'gaga'; export class C extends null {}",
+      "'gaga'; export let a = 1", "'gaga'; export default 1",
+      "'gaga'; export default function foo() {}",
+      "'gaga'; export default function () {}",
+      "'gaga'; export default (function () {})",
+      // TODO(neis): v8:6022 "'gaga'; export default class C extends null {}",
+      "'gaga'; export default (class C extends null {})",
+  };
+
+  for (auto src : sources) {
+    Local<String> source_text = v8_str(src);
+    ScriptOrigin origin = ModuleOrigin(v8_str("file.js"), CcTest::isolate());
+    ScriptCompiler::Source source(source_text, origin);
+    Local<Module> module =
+        ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
+    CHECK(module->Instantiate(env.local(),
+                              CompileSpecifierAsModuleResolveCallback));
+    CHECK(module->Evaluate(env.local())
+              .ToLocalChecked()
+              ->StrictEquals(v8_str("gaga")));
+  }
+}
+
 }  // anonymous namespace