archives

« Bugzilla Issues Index

#159 — assertion in 10.2.1.1.3 step 2 is incorrect


+++ This bug was initially created as a clone of Bug #79 +++

from: https://mail.mozilla.org/pipermail/es5-discuss/2010-November/003839.html

In answering some questions about how SpiderMonkey implements assignment in the face of crazy concerns like those raised in the "Assigning to globals in strict mode" thread, I wrote this example to demonstrate the precise semantics specified and implemented:

var x = "global";
function fun(s) { eval(s); return function(s) { return eval(s); }; }
var closure = fun("var x = 'local'; x = (delete x, 'overwrite');");
assert(x === "global");
assert(closure("x") === "overwrite");

When I went to verify our behavior/implementation conforms to ES5, I discovered that ES5 asserts this situation to be impossible!

eval calls CreateMutableBinding for x. The binding is configurable because it is introduced by eval code. The assignment to x starts by evaluating x to a Reference rx whose base is the lexical environment for the call to fun and whose name is "x". Next we evaluate the right-hand side, along the way calling DeleteBinding for x and successfully removing that binding because it was configurable. The assignment algorithm completes by calling PutValue(rx, "overwrite"). Step 5 of PutValue calls SetMutableBinding on the lexical environment. Step 2 of SetMutableBinding asserts that a binding for x already exists -- but it doesn't because we deleted it!

ES5 can't assert the binding exists. SpiderMonkey, when it assigns to a deleted binding, appears to reintroduce a configurable, mutable binding:

[jwalden at find-waldo-now src]$ dbg/js
js> var x = "global";
js> function foo(s) { eval(s); return function(s) { return eval(s); }; }
js> var closure = foo("var x = 'local'; " +
"x = (delete x, 'overwrite'); " +
"print(x); " +
"delete x; " +
"print(x); " +
"eval('var x = \"local2\";'); " +
"print(x);")
overwrite
global
local2

Thus I *think* the right change (awful as it is) is to replace step 2 with:

2. If envRec does not have a binding for N, call CreateMutableBinding(N, true).

and perhaps add a note to the end of the algorithm explaining how this can occur. But I could well be missing something here -- please poke holes in my suggestion. :-)

Jeff


fixed in rev30 editor's draft.

See 8.1.1.1.5

Created a replacement binding as suggested. It would probably be a better fix to re-resolve the identifier using the scope chain. However, in the specification, the current scope chain isn't available within SetMutableBinding. It would take more rework than is probably justified to make the scope chain available.


fixed in rev30