Promises: some adaptations to spec

src/promise.js

Index: src/promise.js
diff --git a/src/promise.js b/src/promise.js
index 30f4f07b4b7b21c1c83b0c6e590eeb8f4c2803d6..b02f71de5ac4a42bf5cb810e552e9b35a4f215ee 100644
--- a/src/promise.js
+++ b/src/promise.js
@@ -62,11 +62,14 @@ function IsPromise(x) {
 
 function Promise(resolver) {

[yhirano, 2013/12/09 02:31:36]

I'm still not sure.
The spec has not been changed at this point, has it?

[rossberg, 2013/12/10 10:11:15]

No, but the spec requires that exceptions from the function itself are delayed,
so this CL makes it "more conforming" at least. An extra type check would be
needed to implement the synchronous type error.

[yhirano, 2013/12/11 04:46:31]

Then can you write a comment?

[rossberg, 2014/01/09 12:51:43]

Changed to throw

   if (resolver === promiseRaw) return;
+  if (!%_IsConstructCall()) throw MakeTypeError('not_a_promise', [this]);
   var promise = PromiseInit(this);
-  resolver(function(x) { PromiseResolve(promise, x) },
-           function(r) { PromiseReject(promise, r) });
-  // TODO(rossberg): current draft makes exception from this call asynchronous,
-  // but that's probably a mistake.
+  try {
+    resolver(function(x) { PromiseResolve(promise, x) },
+             function(r) { PromiseReject(promise, r) });
+  } catch (e) {
+    PromiseReject(promise, e);
+  }
 }
 
 function PromiseSet(promise, status, value, onResolve, onReject) {
@@ -82,9 +85,10 @@ function PromiseInit(promise) {
 }
 
 function PromiseDone(promise, status, value, promiseQueue) {
-  if (GET_PRIVATE(promise, promiseStatus) !== 0) return;
-  PromiseEnqueue(value, GET_PRIVATE(promise, promiseQueue));
-  PromiseSet(promise, status, value);
+  if (GET_PRIVATE(promise, promiseStatus) === 0) {
+    PromiseEnqueue(value, GET_PRIVATE(promise, promiseQueue));
+    PromiseSet(promise, status, value);
+  }
 }
 
 function PromiseResolve(promise, x) {
@@ -219,25 +223,24 @@ function PromiseThen(onResolve, onReject) {
 PromiseCoerce.table = new $WeakMap;
 
 function PromiseCoerce(constructor, x) {
-  var then;
-  if (IsPromise(x)) {
-    return x;
-  } else if (!IS_NULL_OR_UNDEFINED(x) && %IsCallable(then = x.then)) {
-    if (PromiseCoerce.table.has(x)) {
-      return PromiseCoerce.table.get(x);
-    } else {
-      var deferred = constructor.deferred();
-      PromiseCoerce.table.set(x, deferred.promise);
-      try {
-        %_CallFunction(x, deferred.resolve, deferred.reject, then);
-      } catch(e) {
-        deferred.reject(e);
+  if (!(IsPromise(x) || IS_NULL_OR_UNDEFINED(x))) {
+    var then = x.then;
+    if (typeof then === 'function') {
+      if (PromiseCoerce.table.has(x)) {
+        return PromiseCoerce.table.get(x);
+      } else {
+        var deferred = %_CallFunction(constructor, PromiseDeferred);
+        PromiseCoerce.table.set(x, deferred.promise);
+        try {
+          %_CallFunction(x, deferred.resolve, deferred.reject, then);
+        } catch(e) {
+          deferred.reject(e);
+        }
+        return deferred.promise;
       }
-      return deferred.promise;
     }
-  } else {
-    return x;
   }
+  return x;
 }
 
 
@@ -245,39 +248,44 @@ function PromiseCoerce(constructor, x) {
 
 function PromiseCast(x) {
   // TODO(rossberg): cannot do better until we support @@create.
-  return IsPromise(x) ? x : this.resolved(x);
+  return IsPromise(x) ? x : this.resolve(x);
 }
 
 function PromiseAll(values) {
-  var deferred = this.deferred();
+  var deferred = %_CallFunction(this, PromiseDeferred);
   var resolutions = [];
-  var count = values.length;
-  if (count === 0) {
-    deferred.resolve(resolutions);
-  } else {
-    for (var i = 0; i < values.length; ++i) {
-      this.cast(values[i]).chain(
-        function(i, x) {
-          resolutions[i] = x;
-          if (--count === 0) deferred.resolve(resolutions);
-        }.bind(UNDEFINED, i),  // TODO(rossberg): use let loop once available
-        function(r) {
-          if (count > 0) { count = 0; deferred.reject(r) }
-        }
-      );
+  try {
+    var count = values.length;
+    if (count === 0) {
+      deferred.resolve(resolutions);
+    } else {
+      for (var i = 0; i < values.length; ++i) {
+        this.cast(values[i]).chain(
+          function(i, x) {
+            resolutions[i] = x;
+            if (--count === 0) deferred.resolve(resolutions);
+          }.bind(UNDEFINED, i),  // TODO(rossberg): use let loop once available
+          function(r) { deferred.reject(r) }
+        );
+      }
     }
+  } catch (e) {
+    deferred.reject(e)
   }
   return deferred.promise;
 }
 
 function PromiseOne(values) {  // a.k.a. race
-  var deferred = this.deferred();
-  var done = false;
-  for (var i = 0; i < values.length; ++i) {
-    this.cast(values[i]).chain(
-      function(x) { if (!done) { done = true; deferred.resolve(x) } },
-      function(r) { if (!done) { done = true; deferred.reject(r) } }
-    );
+  var deferred = %_CallFunction(this, PromiseDeferred);
+  try {
+    for (var i = 0; i < values.length; ++i) {
+      this.cast(values[i]).chain(
+        function(x) { deferred.resolve(x) },
+        function(r) { deferred.reject(r) }
+      );
+    }
+  } catch (e) {
+    deferred.reject(e)
   }
   return deferred.promise;
 }
@@ -288,9 +296,9 @@ function SetUpPromise() {
   %CheckIsBootstrapping()
   global.Promise = $Promise;
   InstallFunctions($Promise, DONT_ENUM, [
-    "deferred", PromiseDeferred,
-    "resolved", PromiseResolved,
-    "rejected", PromiseRejected,
+    "defer", PromiseDeferred,
+    "resolve", PromiseResolved,
+    "reject", PromiseRejected,
     "all", PromiseAll,
     "one", PromiseOne,

[arv (Not doing code reviews), 2013/12/10 14:21:24]

This should be race

[rossberg, 2014/01/09 12:51:43]

Done.

     "cast", PromiseCast