[].push wrt properties along the [[Prototype]] chain

[Norbert Lindenberg]

Using Jeff's tests, I don't see exceptions being thrown in any of these browsers: IE 9 and the current versions of Safari on Mac/iOS, Firefox/Chrome/Opera on Mac/Windows. I don't have IE 10.

I also checked the tests in directory 15.4.4.7 of Test262; they don't cover these cases.

Adding non-writable and accessor properties on an actual Array instance isn't interesting because push always adds new properties with higher indices than any existing property on that instance.

Norbert


On Feb 16, 2013, at 10:51 , Allen Wirfs-Brock wrote:

> 
> On Feb 15, 2013, at 6:30 PM, Jeff Walden wrote:
> 
>> Consider:
>> 
>>  Object.defineProperty(Object.prototype, "0", { value: 17, writable: false, configurable: false });
>>  [].push(42);
>> 
>> Per ES5, I think this is supposed to throw a TypeError.  The push should be setting property "0" with Throw = true, which means that when [[CanPut]] fails, a TypeError gets thrown.  No engine I can test does this, I suspect because everyone's mis-implemented an optimization.
> 
> Right, ES5 carefully put in all those Throw = true's, to account for exactly those situations.   Did you also test for non-writable and accessor properties on an actual Array instance.
> 
> Did you try this on IE9/10 (I don't have copies available with me right now).  All the other browsers had implementations of push and other Array methods  that predate ES5, so it is conceivable that those implementations have not been updated to match the spec. IE9, on the other-hand, was a new implementation that post dates ES5 and was supposed to be following the spec.
> 
> I'm shocked if Test262 doesn't include any tests for these situations.  I'm remember including them in a list of things that needed to be tested for all of the Array methods that was handed off to the team that wrote the  Test262 tests.
> 
>> 
>> On IRC it was pointed out that http://wiki.ecmascript.org/doku.php?id=strawman:fixing_override_mistake is supposed to fix this: you should be able to shadow a non-writable property on a prototype.  Or something.  But there's contention there about this not actually being a mistake.  (I think that contention's probably right, for what it's worth, but I digress.)
>> 
>> But suppose it isn't, for the moment.  What then of this:
>> 
>>  Object.defineProperty(Object.prototype, 0, { set: function() { throw "FAIL"; } });
>>  [].push(42)
>> 
>> I think this should throw, again because it's *setting* property "0".  But again, no engine I can test actually throws for this.
>> 
>> My gut says this is a case where every engine attempted to optimize, and optimized wrongly such that incorrect semantics resulted.  The question is, since no engine's following the spec, whether the spec should change, the engines should change, or what.  Given that indexed properties on Array.prototype and Object.prototype are not something anyone sane does, I tend to think changing it to [[DefineOwnProperty]] would be good.  But maybe the spec really should win, and everyone should change.  I dunno.  Please sort this out!  :-)
> 
> Prior to ES5, implementations were doing various random things for the edge cases of accessor properties in arrays.  ES5 now tells them what they are supposed to do.