758499 - Remove JSRESOLVE_WITH

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

JSRESOLVE_WITH was added long ago in bug 463997 as a hackaround for some oddness with |with (window) var x = 17;|.  The details of exactly went wrong are never *quite* fully expressed in the bug with an exactly minimal browser testcase, alas, but that was the expressed gist of it.

With the changes in bug 632003 and bug 577325, however, I suspect the hackaround that JSRESOLVE_WITH implemented to be no longer necessary -- we define vars and names declared by function statements rather than set them.  Thus I expect it's no longer necessary to special-case window expandos when being set for vars inside |with|, and it's no longer necessary to special-case with-based lookups via the resolve flag mechanism.

And indeed, if I remove JSRESOLVE_WITH and run JS tests, the test mentioned in bug 463997 as failing does not fail!  Let's do this.

...after seeing if embedders (sigh) are using this.  Given the extreme obscurity of it and that it was added only to placate Gecko, I am cautiously optimistic.  Thread started:

https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.tech.js-engine/DT1cqbDir0A

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

Comment 2

[Jeff Walden [:Waldo]]

Comment on attachment 627295 [details] [diff] [review]
Patch

Review of attachment 627295 [details] [diff] [review]:
-----------------------------------------------------------------

Feel free to wait however long you feel is right for embedder feedback (I only posted last night, so this is still a bit fresh).  But note that this flag was never even documented anywhere, so the odds anyone but us used it, and will comment in the newsgroup, are slim to none.

Comment 3

[Jeff Walden [:Waldo]]

https://tbpl.mozilla.org/?tree=Try&rev=d7880a675681 is all green so far with a J result or two on every platform, fwiw.

Comment 4

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/12b0df6ff60f \o/

Comment 5

[Jeff Walden [:Waldo]]

I documented the presence of JSRESOLVE_WITH, and its obsoletion, here:

https://developer.mozilla.org/En/SpiderMonkey/JSAPI_Reference/JSNewResolveOp

Also I added a note about its removal to the 1.8.8 notes:

https://developer.mozilla.org/en/SpiderMonkey/1.8.8

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/12b0df6ff60f