d at domenic.me (2015-05-11T16:50:16.777Z)
On Wed, Apr 29, 2015 at 4:16 PM, Allen Wirfs-Brock <allen at wirfs-brock.com> wrote:
> the last two lines of the above `resolve` method should be replaced with:
> ```js
> return super.resolve(x)
> ```
> `x.construct === this` is the test that I propose Promise.resolve should
> make instead of testing [[PromiseConstructor]]
> There should probably also be another test that
> SpeciesConstructor(x)===this
>
This is still not correct.
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.
A correct implementation just does `return new this(r => { r(x); })` after the switch.
The "another test" that you propose seems to be exactly the fix which
Brandon proposed and I codified, to wit:
```
2. If IsPromise(x) is true,
a. Let constructor be the value of SpeciesConstructor(x, %Promise%)
b. If SameValue(constructor, C) is true, return x.
```
> I wouldn't jump the gun until we actually have TC39 consensus on a proposal.
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.
> 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.
>
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.
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.
(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.)
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.
On Wed, Apr 29, 2015 at 4:16 PM, Allen Wirfs-Brock <allen at wirfs-brock.com> wrote: > On Apr 29, 2015, at 12:40 PM, C. Scott Ananian wrote: > > On Wed, Apr 29, 2015 at 3:32 PM, Allen Wirfs-Brock <allen at wirfs-brock.com> > wrote: > >> On Apr 29, 2015, at 12:24 PM, C. Scott Ananian wrote: >> >> On Wed, Apr 29, 2015 at 3:09 PM, Allen Wirfs-Brock <allen at wirfs-brock.com >> > wrote: >> >>> class DefensivePromise extends Promise { >>> constructor(x) { >>> super(x); >>> if (new.target === DefensivePromise) { >>> >> // I'm assuming this test is just to be subclass friendly and >> allow subclasses to freeze later? >> >>> Object.freeze(this); >>> } >>> } >>> static resolve(x) { >>> switch (true) { >>> default: >>> >> // I guess a do { } while(false); would work as well? >> >>> // assuming frozen primordial >>> if (x.constructor !== DefensivePromise) break; //just a quick >>> exit, doesn't guarantee much >>> if (!Object.isFrozen(x)) break; >>> If (Object.getOwnPropertyDescriptor(x, 'then')) break; >>> //a more subclass friendly approach would walk x's >>> [[Prototype]] chain to ensure that the correct 'then' is inherited from >>> frozen prototypes >>> if (Object.getPrototypeOf(x) !== DefensivePromise.prototype) >>> break; >>> //Assert: x.then === Promise.prototype.then, now and forever >>> after >>> return x; >>> } >>> // must be called on a subclass of DefensivePromise, so we don't >>> need to enforce the 'then' invariant >>> If (x.constructor === this) return x; //in which case a >>> constructor check is good enough >>> >> // ^^ this is a mistake right? I think this line doesn't >> belong. >> >>> return new this(r => {r(x)}); >>> } >>> } >>> Object.freeze(DefensivePromise); >>> Object.freeze(DefensivePromise.prototype); >>> >> >> It's not clear what the `x.constructor` test is still doing in your >> implementation. >> >> >> Do you mean the first or second occurrence? >> > > 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. > > > the last two lines of the above `resolve` method should be replaced with: > return super.resolve(x) > > x.construct === this is the test that I propose Promise.resolve should > make instead of testing [[PromiseConstructor]] > There should probably also be another test that > SpeciesConstrutor(x)===this > This is still not correct. 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. A correct implementation just does `return new this(r => { r(x); })` after the switch. The "another test" that you propose seems to be exactly the fix which Brandon proposed and I codified, to wit: 2. If IsPromise(x) is true, > a. Let constructor be the value of SpeciesConstructor(x, %Promise%) > b. If SameValue(constructor, C) is true, return x. I wouldn't jump the gun until we actually have TC39 consensus on a proposal. > 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. > 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. > 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. 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. (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.) 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. --scott -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20150429/38306d40/attachment-0001.html>