<div dir="ltr"><div>On Wed, Apr 29, 2015 at 4:16 PM, Allen Wirfs-Brock <span dir="ltr"><<a href="mailto:allen@wirfs-brock.com" target="_blank">allen@wirfs-brock.com</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div class="h5"><div>On Apr 29, 2015, at 12:40 PM, C. Scott Ananian wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Apr 29, 2015 at 3:32 PM, Allen Wirfs-Brock <span dir="ltr"><<a href="mailto:allen@wirfs-brock.com" target="_blank">allen@wirfs-brock.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div><div>On Apr 29, 2015, at 12:24 PM, C. Scott Ananian wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Apr 29, 2015 at 3:09 PM, Allen Wirfs-Brock <span dir="ltr"><<a href="mailto:allen@wirfs-brock.com" target="_blank">allen@wirfs-brock.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div> class DefensivePromise extends Promise {</div>  constructor(x) {<br>    super(x);<br>    if (new.target === DefensivePromise) {<br></div></blockquote><div>           // I'm assuming this test is just to be subclass friendly and allow subclasses to freeze later? </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">      Object.freeze(this);<br><div>    }<br>  }<br>  static resolve(x) {<br>    switch (true) {</div><div>       default:</div></div></blockquote><div>         // I guess a do { } while(false); would work as well? </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div>          // assuming frozen primordial</div><div>          if (x.constructor !== DefensivePromise) break;  //just a quick exit, doesn't guarantee much</div><div>          if (!Object.isFrozen(x)) break;</div><div>          If (Object.getOwnPropertyDescriptor(x, 'then')) break;</div><div>          //a more subclass friendly approach would walk x's [[Prototype]] chain to ensure that the correct 'then' is inherited from frozen prototypes</div><div>          if (Object.getPrototypeOf(x) !== DefensivePromise.prototype) break;</div><div>          //Assert: x.then === Promise.prototype.then, now and forever after</div><div>          return x;</div><div>     }</div><div>     // must be called on a subclass of DefensivePromise, so we don't need to enforce the 'then' invariant </div><div>     If  (x.constructor ===  this) return x;   //in which case a constructor check is good enough</div></div></blockquote><div>          // ^^ this is a mistake right?  I think this line doesn't belong. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div>     return new this(r => {r(x)});<br>   }<br> }</div><div>Object.freeze(DefensivePromise);</div><div>Object.freeze(DefensivePromise.prototype);</div></div></blockquote></div><br></div><div class="gmail_extra">It's not clear what the `x.constructor` test is still doing in your implementation.</div></div></blockquote><div><br></div></div></div><div>Do you mean the first or second occurrence?</div></div></div></blockquote><div><br></div><div>The second occurrence.  At that point all we know about x is that it's *not* something safe.  We shouldn't be looking at `constructor` in that case.</div></div></div></div></blockquote><div><br></div></div></div><div>the last two lines of the above `resolve` method should be replaced with:</div><div>     return super.resolve(x)</div><div><br></div><div>x.construct === this is the test that I propose Promise.resolve should make instead of  testing [[PromiseConstructor]]</div><div>There should probably also be another test that SpeciesConstrutor(x)===this </div></div></div></blockquote><div><br></div><div>This is still not correct.</div><div><br></div><div>You can't call `super.resolve(x)` safely unless you know that this.constructor is trustworthy.  If you've bailed out of the `switch`, you don't know that.</div><div><br></div><div>A correct implementation just does `return new this(r => { r(x); })` after the switch.<br></div><div><br></div><div>The "another test" that you propose seems to be exactly the fix which Brandon proposed and I codified, to wit:</div><div><div><br></div></div></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">2. If IsPromise(x) is true,<br>    a. Let constructor be the value of SpeciesConstructor(x, %Promise%)<br>    b. If SameValue(constructor, C) is true, return x.</blockquote><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div>I wouldn't jump the gun until we actually have TC39 consensus on a proposal.</div></div></blockquote><div><br></div><div>Mark, after you've slept on this and assuming you haven't changed your mind, would you be willing to be the TC39 champion?  I'm not a participant in that process.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div>What we've shown is that the [[PromiseConstructor]] test was never sufficient to guarantee the invariant that Mark is concerned about and also that there probably isn't a generic check we can do that would satisfy his requirements.  He has do it himself like shown above.  But the specified [[PromiseConstructor]] test doesn't  really hurt anything.  It places some slight limitations on what subclasses can do, but probably doesn't interfere with anything an actual subclass is likely to do.  It seems like a restriction than can be relaxed in the future without anybody (or than conformance tests) noticing.<br></div></div></blockquote><div><br></div><div>Well, the [[PromiseConstructor]] test is breaking `prfun`, `es6-shim` and `core-js` right now, which is why I noticed it in the first place.  (v8 seems to have implemented the `[[PromiseConstructor]]` internal property even though they don't have `new.target` implemented, so it's always hard-wired to `Promise`.)  And `babel` and `core-js` have implemented non-standard semantics for `Promise.resolve` as a result.</div><div><br></div><div>In particular, with the current state of `Promise.resolve`, I can't subclass `Promise` in a meaningful way using ES5 syntax (see, the thread title turns relevant again!).  So I have a vested interest in getting the semantics fixed so that people can start using `Promise` subclasses (like `prfun` does) instead of writing their "extra" Promise helpers directly on the `Promise` global object, while we're all waiting for ES6 syntax to make it to the mainstream.</div><div><br></div><div>(And when I say "getting the semantics fixed" what I really mean is "consensus among TC39" so that shim libraries can implement the correct semantics as early adopters, with assurance the browser engines will follow.)</div><div><br></div><div>Since I believe the goal is to eventually introduce some new methods on the global `Promise` in ES7, I think this list would have a keen interest in ensuring that libraries which stomp on the global Promise (due to lack of an alternative) don't take root.</div><div>  --scott</div><div><br></div></div></div></div>