Parity error in 10.2.1.2.2 CreateMutableBinding(N,D) in Object Environment Record

# Mark S. Miller (15 years ago)

Currently it reads (with emphasis added):

10.2.1.2.2 CreateMutableBinding(N,D)

The concrete Environment Record method CreateMutableBinding for object environment records creates in an environment record’s associated binding object a property whose name is the String value and initializes it to the value undefined. A property named N must not already exist in the binding object. If Boolean argument D is provided and has the value true the new property’s [[Configurable]] attribute is set to true, otherwise it is set to false.

  1. Let envRec be the object environment record for which the method was invoked.
  2. Let bindings be the binding object for envRec. *3. Assert: The result of calling the [[HasProperty]] internal method of bindings, passing N as the property * *name, is false. *
  3. If D is true then let configValue be true otherwise let configValue be false.
  4. Call the [[DefineOwnProperty]] internal method of bindings, passing N, Property Descriptor {[[Value]]:undefined, [[Writable]]: true, [[Enumerable]]: true , [[Configurable]]: configValue}, and false as arguments.

My concern is that last false. It indicates that if the call to [[DefineOwnProperty]] fails, then it should fail silently rather than throwing a TypeError exception. Given the other text I emphasized above, there must already not be any conflicting property on the bindings object. So the only way [[DefineOwnProperty]] can fail is if the bindings object is non-extensible. When the binding is being created to represent a declaration appearing in strict code, this silent error is clearly wrong. Having participated heavily in the discussion and design of strict mode, I am confident this does not reflect our intent and is simply a mistake. I think we have two choices:

  1. We can flip this to be unconditionally true. Since legacy code would never make non-extensible binding objects, this would technically be upwards compatible and would avoid breaking the web. But this behavior would be at odds with other similar non-strict cases.

  2. We can add an "S" parameter to CreateMutableBindings according to whether the declaration in question appears in strict code. We would then need to propagate this to callers, of which there are four in 10.5 (Declaration Binding Instantiation) and one in 12.14 (The try Statement). This change would be straightforward.

Since implementations are proceeding and the window on correcting errata for the ISO version is closing, we should decide soon. I mildly prefer #2, but I would find either acceptable. If there are no objections, let's decide to do .#2.

# Allen Wirfs-Brock (15 years ago)

Looking at all the uses of this method in the spec., the only situation in non-extended ES5 where the [[DefineOwnProperty]] failure could occur would be for global var or function declarations where the global object has been made non-extensible. Whether the implementation dependent global object can be made non-extensible is itself an interesting questions. However, given that possibility I agree that the final argument to [[defineOwnProperty]] in step of 10.2.1.2.2 should be true rather than false. This is sufficient to take care of all situations that occur in the actual ES5 spec. and is the smallest possible correction. Alternative 2 might be needed if extensions were added to the language that required new uses of Object Environment Records. However, that is an issues for future editions of the specification.

Allen

From: es5-discuss-bounces at mozilla.org [mailto:es5-discuss-bounces at mozilla.org] On Behalf Of Mark S. Miller Sent: Sunday, June 06, 2010 12:50 AM To: es5-discuss at mozilla.org; es-discuss Subject: Parity error in 10.2.1.2.2 CreateMutableBinding(N,D) in Object Environment Record

Currently it reads (with emphasis added):

10.2.1.2.2 CreateMutableBinding(N,D)

The concrete Environment Record method CreateMutableBinding for object environment records creates in an environment record's associated binding object a property whose name is the String value and initializes it to the value undefined. A property named N must not already exist in the binding object. If Boolean argument D is provided and has the value true the new property's [[Configurable]] attribute is set to true, otherwise it is set to false.

  1. Let envRec be the object environment record for which the method was invoked.
  2. Let bindings be the binding object for envRec.
  3. Assert: The result of calling the [[HasProperty]] internal method of bindings, passing N as the property name, is false.
  4. If D is true then let configValue be true otherwise let configValue be false.
  5. Call the [[DefineOwnProperty]] internal method of bindings, passing N, Property Descriptor {[[Value]]:undefined, [[Writable]]: true, [[Enumerable]]: true , [[Configurable]]: configValue}, and false as arguments.

My concern is that last false. It indicates that if the call to [[DefineOwnProperty]] fails, then it should fail silently rather than throwing a TypeError exception. Given the other text I emphasized above, there must already not be any conflicting property on the bindings object. So the only way [[DefineOwnProperty]] can fail is if the bindings object is non-extensible. When the binding is being created to represent a declaration appearing in strict code, this silent error is clearly wrong. Having participated heavily in the discussion and design of strict mode, I am confident this does not reflect our intent and is simply a mistake. I think we have two choices:

  1. We can flip this to be unconditionally true. Since legacy code would never make non-extensible binding objects, this would technically be upwards compatible and would avoid breaking the web. But this behavior would be at odds with other similar non-strict cases.

  2. We can add an "S" parameter to CreateMutableBindings according to whether the declaration in question appears in strict code. We would then need to propagate this to callers, of which there are four in 10.5 (Declaration Binding Instantiation) and one in 12.14 (The try Statement). This change would be straightforward.

Since implementations are proceeding and the window on correcting errata for the ISO version is closing, we should decide soon. I mildly prefer #2, but I would find either acceptable. If there are no objections, let's decide to do #2.

# Mark S. Miller (15 years ago)

+1. I find it acceptable to just change this argument to true. The only objection might be that the resulting thrown error might be surprising to non-strict code, since all similar errors in non-strict code are silent. But I am happy not to raise that objection, since I don't particularly care about that surprise.

I doubt any future spec will introduce any new cases for using Object Environment Records. And even if they did, since they would be in a future spec, they'd only need the strict behavior anyway, so just changing the argument to true would remain fine.

# Brendan Eich (15 years ago)

On Jun 15, 2010, at 9:05 PM, Mark S. Miller wrote:

+1. I find it acceptable to just change this argument to true. The only objection might be that the resulting thrown error might be surprising to non-strict code, since all similar errors in non-strict code are silent. But I am happy not to raise that objection, since I don't particularly care about that surprise.

It's really unlikely anyway.

I doubt any future spec will introduce any new cases for using Object Environment Records.

Agreed -- if we could salt the earth so nothing more grew there, we would. ES5 strict banning "with" + lexical scope all the way up in simple modules, if not in any <script type="harmony"> (placeholder type there ;-) are a good start.