[async-iteration] implement Async-from-Sync Iterator

test/mjsunit/harmony/async-from-sync-iterator.js

Index: test/mjsunit/harmony/async-from-sync-iterator.js
diff --git a/test/mjsunit/harmony/async-from-sync-iterator.js b/test/mjsunit/harmony/async-from-sync-iterator.js
new file mode 100644
index 0000000000000000000000000000000000000000..2f0e674a316b2c561b0f5d5a6993e8ee27061f33
--- /dev/null
+++ b/test/mjsunit/harmony/async-from-sync-iterator.js
@@ -0,0 +1,437 @@
+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-async-iteration --allow-natives-syntax
+
+let testFailed = false;
+let testFailure;
+
+class MyError extends Error {};
+
+(async function() {
+  function* gen() {
+    yield "sync value";
+    try {
+      yield new Promise(function(resolve) {
+        resolve("async value");
+      });
+    } catch (error) {
+      throw error;

[neis, 2017/02/20 09:41:20]

What's the point of catching and rethrowing here?

[caitp, 2017/02/20 17:09:34]

it's a basic control flow test. The .throw call gets us here rather than to
line22, and rethrowing verifies that the .throw() promise is rejected.

[neis, 2017/02/21 12:48:08]

I don't get your point, but feel free to keep it as is.

+    }
+    assertUnreachable("generator is closed");

[neis, 2017/02/20 09:41:20]

I was at first confused by this. Could you just replace it with "yield 5" or
something like that?

[caitp, 2017/02/20 17:09:35]

I'm not entirely sure what you mean --- after the uncaught `throw` on line 20,
it should be impossible to resume the generator again, because the generator
should be closed. This is part of the "test that async-from-sync delegates to
sync-iterator" behaviour, since it's the sync iterator which is deciding not to
resume execution.

[neis, 2017/02/21 12:48:08]

What confused me is that this line is not unreachable per se, it's just that the
current set of tests doesn't request more than two elements from the iterator.

[caitp, 2017/02/21 14:36:52]

It is unreachable, because the throw on line 20 results in the generator being
closed, so subsequent requests never resume the generator

+  }
+  let iter = %CreateAsyncFromSyncIterator(gen());
+
+  // [Async-from-Sync Iterator] wraps sync iterator values in a Promise
+  let promise = iter.next();
+  assertInstanceof(promise, Promise);
+  let iter_result = await promise;
+  assertEquals({ value: "sync value", done: false }, iter_result);
+
+  // [Async-from-Sync Iterator] will wait for resolution of Promise values
+  promise = iter.next();
+  assertInstanceof(promise, Promise);
+  iter_result = await promise;
+  assertEquals({ value: "async value", done: false }, iter_result);
+
+  // [Async-from-Sync Iterator].throw delegates to .throw() method of sync
+  // iterator.
+  promise = iter.throw(new MyError("Error#1"));
+  assertInstanceof(promise, Promise);
+  try {
+    await promise;
+    assertUnreachable("promise should be rejected");
+  } catch (e) {
+    assertInstanceof(e, MyError);
+    assertEquals("Error#1", e.message);
+  }
+
+  promise = iter.return("generator closed");
+  assertInstanceof(promise, Promise);
+  iter_result = await promise;
+  assertEquals({ value: "generator closed", done: true }, iter_result);
+
+  // .next(), .return() and .throw() delegate to sync iterator methods, without
+  // keeping track of the state of the generator.
+  promise = iter.next("unused");
+  assertInstanceof(promise, Promise);
+  iter_result = await promise;
+  assertEquals({ value: undefined, done: true }, iter_result);
+
+  promise = iter.throw(new MyError("Error#2"));
+  assertInstanceof(promise, Promise);
+  try {
+    await promise;
+  } catch (e) {
+    assertInstanceof(e, MyError);
+    assertEquals("Error#2", e.message);
+  }
+
+  promise = iter.return("return-after-completed");
+  assertInstanceof(promise, Promise);
+  iter_result = await promise;
+  assertEquals({ value: "return-after-completed", done: true }, iter_result);

[neis, 2017/02/20 09:41:20]

You already tested .return above, didn't you?

[caitp, 2017/02/20 17:09:35]

it's good to have an extra one thrown in as it helps to ensure that we didn't
accidentally restart the generator, but I can remove it

+

[neis, 2017/02/20 09:41:20]

Can you somehow separate the tests using gen (above) from those using sync
(below) and from those using neither at the very end?  Maybe have three async
functions at the toplevel.

[caitp, 2017/02/20 17:09:35]

Acknowledged.

+  function resolveLater(value) {
+    return new Promise(function(resolve) {
+      Promise.resolve().then(function() {
+        resolve(value);
+      });
+    });
+  }
+
+  function rejectLater(value) {
+    return new Promise(function(resolve, reject) {
+      Promise.resolve().then(function() {
+        reject(value);
+      });
+    });
+  }
+
+  const kNext = 1;
+  const kThrow = 2;
+  const kReturn = 4;
+  const kNextThrows = kNext | 8;
+  const kReturnThrows = kReturn | 16;
+  const kThrowNormal = kThrow | 32;
+  function sync(array, features, log = []) {

[neis, 2017/02/20 09:41:20]

There shouldn't be a default value for log (see comment further down).

[caitp, 2017/02/20 17:09:35]

Removed the default value, and added a %AbortJS() call if it's not supplied
(tests still passed even without passing it, presumably because the exception
was eaten)

+    let i = 0;
+    let methods = {
+      next(sent) {
+        let done = i >= array.length;
+        let value = array[i];
+        log.push({ method: "next", sent, value, done });
+        if ((features & kNextThrows) === kNextThrows) throw sent;
+        i++;
+        return { value, done };
+      },
+      throw(sent) {
+        let done = i >= array.length;
+        let value = undefined;
+        log.push({ method: "throw", sent, value, done });

[neis, 2017/02/20 09:41:20]

I think you can drop value here and in return below.

[caitp, 2017/02/20 17:09:35]

Done.

+        if ((features & kThrowNormal) === kThrowNormal)
+            return { value: sent, done };
+        throw sent;
+      },
+      return(sent) {
+        let done = true;
+        let value = undefined;
+        log.push({ method: "return", sent, value, done });
+        if ((features & kReturnThrows) === kReturnThrows) throw sent;
+        return { value, done };

[neis, 2017/02/20 09:41:20]

Not sure why you use undefined as the value in return but sent as the value in
throw.

[caitp, 2017/02/20 17:09:35]

It's pretty inconsequential to the test either way, I'll make them the same

+      }
+    };
+    return {
+      [Symbol.iterator]() { return this; },
+      next: (features & kNext) ? methods.next : undefined,
+      throw: (features & kThrow) ? methods.throw : undefined,
+      return: (features & kReturn) ? methods.return : undefined
+    };
+  }
+
+  let log = [];
+  iter = %CreateAsyncFromSyncIterator(sync(["sync-value"], 0));

[neis, 2017/02/20 09:41:20]

You forgot to pass the log.

[caitp, 2017/02/20 17:09:35]

Good catch, thx

+
+  try {
+    await iter.next();
+    assertUnreachable("Iterator.next() method is not optional");
+  } catch (e) {
+    assertInstanceof(e, TypeError);
+    assertEquals([], log);
+  }
+
+  log = [];
+  iter = %CreateAsyncFromSyncIterator(sync(["sync-value"], kNext, log));
+  assertEquals({ value: "sync-value", done: false }, await iter.next("a"));
+  assertEquals([
+    {
+      method: "next",
+      sent: "a",
+      value: "sync-value",
+      done: false
+    }
+  ], log);
+
+  log = [];
+  let asyncValue = resolveLater("async-value");
+  iter = %CreateAsyncFromSyncIterator(sync([asyncValue], kNext, log));
+  assertEquals({ value: "async-value", done: false }, await iter.next("b"));
+  assertEquals([
+    {
+      method: "next",
+      sent: "b",
+      value: asyncValue,
+      done: false
+    }
+  ], log);
+
+  // If [sync_iterator].next() produces a rejected Promise or an exception is
+  // thrown, Promise is rejected with thrown/rejected value.
+  log = [];
+  asyncValue = rejectLater("Boo!");
+  iter = %CreateAsyncFromSyncIterator(sync([asyncValue], kNext, log));
+  try {
+    await iter.next('c');
+    assertUnreachable('Expected `iter.next(\'c\') to throw, but did not throw');

[neis, 2017/02/20 09:41:20]

It's the await that throws, not the iter.next call (same in some other cases
below).

[caitp, 2017/02/20 17:09:35]

throw/reject are admittedly being used inferchangeably, is that confusing?

+  } catch (e) {
+    assertEquals("Boo!", e);
+    assertEquals([
+      {
+        method: 'next',
+        sent: 'c',
+        value: asyncValue,
+        done: false
+      }
+    ], log);
+  }
+
+  // If [sync_iterator].return() does not exist, return a Promise resolved with
+  // the value `{ value: <<sent value>>, done: true }`.
+  log = [];
+  iter = %CreateAsyncFromSyncIterator(sync(['sync-return'], kNext, log));
+  assertEquals({
+    value: 'd',
+    done: true
+  }, await iter.return('d'));
+
+  // [Async-from-Sync Iterator] merely delegates, and does not keep track of
+  // whether [sync_iterator] is completed or not.
+  assertEquals({
+    value: 'sync-return',
+    done: false
+  }, await iter.next('e'));
+
+  assertEquals([
+    {
+      method: 'next',
+      sent: 'e',
+      value: 'sync-return',
+      done: false
+    }
+  ], log);
+
+  // If [sync_iterator].return() does exist, return a Promise resolved with
+  // the iterator result of [sync_iterator].return().
+  log = [];
+  iter = %CreateAsyncFromSyncIterator(sync(['sync-return'],
+                                      kNext|kReturn, log));
+  assertEquals({
+    value: undefined,
+    done: true
+  }, await iter.return('f'));
+
+  // [Async-from-Sync Iterator] merely delegates, and does not keep track of
+  // whether [sync_iterator] is completed or not.
+  assertEquals({
+    value: 'sync-return',
+    done: false
+  }, await iter.next('g'));
+
+  assertEquals([
+    {
+      method: 'return',
+      sent: 'f',
+      value: undefined,
+      done: true
+    },
+    {
+      method: 'next',
+      sent: 'g',
+      value: 'sync-return',
+      done: false
+    }
+  ], log);
+
+  // If [sync_iterator].return() produces a rejected Promise or an exception is
+  // thrown, Promise is rejected with thrown/rejected value.
+  log = [];
+  iter = %CreateAsyncFromSyncIterator(sync(['sync-value'], kNext|kReturnThrows,
+                                      log));
+  try {
+    await iter.return('Boo!!');
+    assertUnreachable('Expected `iter.return(\'Boo!!\') to throw, but did ' +
+                      'not throw');
+  } catch (e) {
+    assertEquals("Boo!!", e);
+  }

[neis, 2017/02/20 09:41:20]

Can you also test the case where the result of the sync .return is not an
object?

And where the iterator value produced by the sync .return is a rejected promise
(here: the argument to .return is a rejected promise).

+
+  // [Async-from-Sync Iterator] merely delegates, and does not keep track of
+  // whether [sync_iterator] is completed or not.
+  assertEquals({ value: 'sync-value', done: false }, await iter.next('h'));
+  assertEquals([
+    {
+      method: 'return',
+      sent: 'Boo!!',
+      value: undefined,
+      done: true
+    },
+    {
+      method: 'next',
+      sent: 'h',
+      value: 'sync-value',
+      done: false
+    }
+  ], log);
+
+  // If [sync_iterator].throw() does exist, return a Promise rejected with
+  // the sent value.

[neis, 2017/02/20 09:41:20]

Do you mean "does not exist" here?

[caitp, 2017/02/20 17:09:35]

"if [sync_iterator] does not have a .throw method"

+  log = [];
+  iter = %CreateAsyncFromSyncIterator(sync(['sync-value'], kNext, log));
+  try {
+    await iter.throw('Boo!!');
+    assertUnreachable('Expected iter.throw(\'Boo!!\') to throw, but did not ' +
+                      'throw');
+  } catch (e) {
+    assertEquals('Boo!!', e);
+  }
+
+  // [Async-from-Sync Iterator] merely delegates, and does not keep track of
+  // whether [sync_iterator] is completed or not.
+  assertEquals({ value: 'sync-value', done: false }, await iter.next('i'));
+  assertEquals([
+    {
+      method: 'next',
+      sent: 'i',
+      value: 'sync-value',
+      done: false
+    }
+  ], log);
+
+  log = [];
+  iter = %CreateAsyncFromSyncIterator(sync(['sync-value'], kNext|kThrow, log));
+  try {
+    await iter.throw('Boo!!');
+    assertUnreachable('Expected iter.throw(\'Boo!!\') to throw, but did not ' +
+                      'throw');
+  } catch (e) {
+    assertEquals('Boo!!', e);
+  }
+
+  // [Async-from-Sync Iterator] merely delegates, and does not keep track of
+  // whether [sync_iterator] is completed or not.
+  assertEquals({ value: 'sync-value', done: false }, await iter.next('j'));
+  assertEquals([
+    {
+      method: 'throw',
+      sent: 'Boo!!',
+      value: undefined,
+      done: false
+    },
+    {
+      method: 'next',
+      sent: 'j',
+      value: 'sync-value',
+      done: false
+    }
+  ], log);
+
+  // If [sync_iterator].throw() returns a resolved Promise or a Completion
+  // with [[Type]] "normal" or "return", return a resolved Promise
+  log = [];

[neis, 2017/02/20 09:41:20]

A function call cannot return a "normal" completion.

+  iter = %CreateAsyncFromSyncIterator(sync(['sync-value'], kNext|kThrowNormal,
+                                      log));
+  assertEquals({
+    value: 'Boo!!',
+    done: false
+  }, await iter.throw('Boo!!'));
+
+  // [Async-from-Sync Iterator] merely delegates, and does not keep track of
+  // whether [sync_iterator] is completed or not.
+  assertEquals({ value: 'sync-value', done: false }, await iter.next('k'));
+  assertEquals([
+    {
+      method: 'throw',
+      sent: 'Boo!!',
+      value: undefined,
+      done: false
+    },
+    {
+      method: 'next',
+      sent: 'k',
+      value: 'sync-value',
+      done: false
+    }
+  ], log);
+
+  // Let nextValue be IteratorValue(nextResult).
+  // IfAbruptRejectPromise(nextValue, promiseCapability).)
+  iter = %CreateAsyncFromSyncIterator({
+    next() { return { get value() { throw "BadValue!" }, done: false }; }
+  });
+  try {
+    await iter.next();
+    assertUnreachable('Expected `iter.next()` to throw, but did not throw');
+  } catch (e) {
+    assertEquals('BadValue!', e);
+  }
+
+  // Let nextDone be IteratorComplete(nextResult).
+  // IfAbruptRejectPromise(nextDone, promiseCapability).
+  iter = %CreateAsyncFromSyncIterator({
+    next() { return { value: undefined, get done() { throw "BadValue!" } }; }
+  });
+  try {
+    await iter.next();
+    assertUnreachable('Expected `iter.next()` to throw, but did not throw');
+  } catch (e) {
+    assertEquals('BadValue!', e);
+  }
+
+  // IfAbruptRejectPromise(returnResult, promiseCapability).
+  // Let returnValue be IteratorValue(returnResult).
+  iter = %CreateAsyncFromSyncIterator({
+    return() { return { get value() { throw "BadValue!" }, done: false }; }
+  });
+  try {
+    await iter.return();
+    assertUnreachable('Expected `iter.return()` to throw, but did not throw');
+  } catch (e) {
+    assertEquals('BadValue!', e);
+  }
+
+  // IfAbruptRejectPromise(returnValue, promiseCapability).
+  // Let returnDone be IteratorComplete(returnResult).
+  iter = %CreateAsyncFromSyncIterator({
+    return() { return { value: undefined, get done() { throw "BadValue!" } }; }
+  });
+  try {
+    await iter.return();
+    assertUnreachable('Expected `iter.return()` to throw, but did not throw');
+  } catch (e) {
+    assertEquals('BadValue!', e);
+  }
+
+  // IfAbruptRejectPromise(throwResult, promiseCapability).
+  // Let throwValue be IteratorValue(throwResult).
+  iter = %CreateAsyncFromSyncIterator({
+    throw() { return { get value() { throw "BadValue!" }, done: false }; }
+  });
+  try {
+    await iter.throw();
+    assertUnreachable('Expected `iter.throw()` to throw, but did not throw');
+  } catch (e) {
+    assertEquals('BadValue!', e);
+  }
+
+  // IfAbruptRejectPromise(throwValue, promiseCapability).
+  // Let throwDone be IteratorComplete(throwResult).
+  iter = %CreateAsyncFromSyncIterator({
+    throw() { return { value: undefined, get done() { throw "BadValue!" } }; }
+  });
+  try {
+    await iter.throw();
+    assertUnreachable('Expected `iter.throw()` to throw, but did not throw');
+  } catch (e) {
+    assertEquals('BadValue!', e);
+  }
+})().catch(function(error) {
+  testFailed = true;
+  testFailure = error;
+});
+
+%RunMicrotasks();
+
+if (testFailed) {
+  throw testFailure;
+}

[neis, 2017/02/20 09:41:20]

Could you please add
- a test where the sync iterator's next/throw/return returns an object whose
"value" property is a promise that is (or becomes) resolved/rejected,
- a test where the sync iterator's next/throw/return returns a non-object.