Allen Wirfs-Brock (2015-04-29T20:16:33.000Z)
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 

>  
>> But, regardless of the details of our implementations, can we agree that "tamper proof" promises don't seem to need the [[PromiseConstructor]] property?
> 
> I agree with you, but I'm not the Promise champion.
> 
> Regardless, too late for ES2015.  It will have to proposed as an ES2016 change.
> 
> True, but since subclassable Promises haven't been implemented in the major engines yet, still in time to get everyone to agree to implement the new semantics before use "in the wild" locks in the warts of ES2015.
> 
> If we get consensus, I can certainly ensure that es6-shim, core-js, and babel implement the "ES2016" semantics.

I wouldn't jump the gun until we actually have TC39 consensus on a proposal.

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.

Allen

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20150429/25b01b9c/attachment.html>
d at domenic.me (2015-05-11T16:48:57.374Z)
On Apr 29, 2015, at 12:40 PM, C. Scott Ananian wrote:

> 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:

```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 

> If we get consensus, I can certainly ensure that es6-shim, core-js, and babel implement the "ES2016" semantics.

I wouldn't jump the gun until we actually have TC39 consensus on a proposal.

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.