Uncovered code as of 07-Jun-14: #2 - checkYourself about native-promise-only

[getify]

it a hole in the promises-aplus-tests, or even a hole in the specification?

Neither of those, per se. I think it's actually something that proper testing of the Promise(..) constructor should catch. Note: the A+ spec isn't testing the semantics of Promise(..) so it wouldn't catch stuff like this.

IIRC, that check is there for a case like:

var p = new Promise(function(resolve,reject){
   resolve(..);
   // later
   resolve(); // this second call should silently fail per A+ spec, but not
                   // tested in A+ spec tests

   // same goes for calling `reject(..)` twice.
});

In other words, the check is, if you've already called resolve(..), then state is no longer 0 and you need to silently fail.

However, later on, I added two other guards, lines 274-277 and lines 282-285, respectively, which I believe have the same effect (but for a slightly different reason).

Boiling it all down, I believe this is indeed probably a case of provably unreachable, given my implementation check on def.triggered already guarding against the only place where this could happen.

I certainly don't advocate removing the guard yet

We definitely should leave it in for now, until we get a full test suite against Promise(..), even tests that try the above stuff. Then we can remove the check and see if any of the tests fail. I highly doubt they will, but doubting and knowing are two different things at this point.

So let's make sure to revisit this later. This check is on the hot path so removing it would tweak the perf up a little bit (perhaps a lot in aggregate?), so I definitely want to once we have tests proving it's not needed. :)

[smikes]

Agreed, this code is no longer present, therefore not uncovered.