See bug 1103441 comment 4. This has two parts: (1) Kill JSOP_GETXPROP (so that we don't have to lookup the pc to check for this case). I think JSOP_GETXPROP was an optimization to avoid doing two scope chain walks in this case: eval(""); x += 55; 00025: bindname "x" 00030: dup 00031: getxprop "x" 00036: int8 55 00038: add 00039: setname "x" With all the scope chain optimizations, we don't really have to worry about this anymore: NAME (not GNAME!) ops are rare/slow anyway and we can afford emitting a JSOP_NAME: 00025: bindname "x" 00030: name "x" 00035: int8 55 00037: add 00038: setname "x" (2) We should not do the script/pc lookup if the compartment has extraWarnings disabled.

As described in comment 0, pretty simple patch. (JSOP_GETXPROP was not Ion-compiled, another reason to remove it.)

Comment on attachment 8531467 [details] [diff] [review] Patch Review of attachment 8531467 [details] [diff] [review]: ----------------------------------------------------------------- I think GETXPROP is necessary for ES6 compliance in some dumb cases. Unfortunately. Here's a test I think we will fail with the patch: var x = "wrong"; var t = {x: "x"}; var p = new Proxy(t, { has(t, id) { print("has"); var found = id in t; delete t[id]; return found; }, get(t, id) { print("get"); return t[id]; } }); with (p) x += " x"; assertEq(t.x, "undefined x"); Current behavior is correct except I think ES6 says we have to call "has" twice before calling "set"; currently we only call it once. With the patch, we would set t.x to "wrong x", I think.

(In reply to Jason Orendorff [:jorendorff] from comment #2) > I think GETXPROP is necessary for ES6 compliance in some dumb cases. > Unfortunately. Thanks for catching that. I should have seen this :( > Current behavior is correct except I think ES6 says we have to call "has" > twice before calling "set"; currently we only call it once. > > With the patch, we would set t.x to "wrong x", I think. Yup, confirmed. Any thoughts on what's the right way to implement this and avoid both bugs? Getting the script location and checking for GETXPROP on all "missing property" getprops, is too much of a performance penalty. Maybe we should have a JSOP_GETNAME-like op that handles this? We could only emit it if there's something weird on the scope chain..

This patch just passes a flag when we have a JSOP_GETXPROP. Also added the test from comment 2. Got r+ from Jason on IRC.