632064 - remove/deprecate JS_GetScopeChain in lieu of JS_GetGlobalForScopeChain

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

mxr shoes that 100% of the uses of JS_GetScopeChain in Mozilla are to get at the scope chain's global.  (Is there any other valid thing to do with it?  Hopefully not implement some form of dynamic name lookup.)  Since JS_GetGlobalForScopeChain is what everybody wants (and its more efficient in more than one way), can we nix JS_GetScopeChain or deprecate it and just have it return JS_GetGlobalForScopeChain?  My motivation is that the less leaky API will give us more flexibility (bug 631135 is a weak example) and, when we get around to the scope chain overhaul (bug ?), scope chains won't be objects (while global objects clearly will).

Simple patch.  Will ask on the listserv if no immediate objections.

Comment 1

[Luke Wagner [:luke]]

(In reply to comment #0)
> (bug 631135 is a weak example)

Hah, 1 hour later it becomes a strong example (end of bug 631135 comment 23).

Comment 2

[Luke Wagner [:luke]]

Waldo's on it!

Comment 3

[Wan-Teh Chang]

Luke: bug 511781 is not the bug you have in mind.

Comment 4

[Luke Wagner [:luke]]

D'oh!

Comment 5

[Luke Wagner [:luke]]

Bug 511781 never got fixed, but it will be fixed by compartment-per-global, so I'll fix this bug now.

Comment 6

[Luke Wagner [:luke]]

Attachment: rm

This patch also fixes some annoying && || warnings.

Comment 7

[Luke Wagner [:luke]]

Attachment: rm xml uses of GetScopeChain

The assumption of this patch is that, whatever E4X wants to find on the scope chain, it causes a call-obj to be created and put on the scope chain, hence the lazy call-obj creation done by GetScopeChain is unnecessary.  Is that correct Igor?

Comment 8

[Igor Bukanov]

Comment on attachment 564690 [details] [diff] [review]
rm xml uses of GetScopeChain

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

::: js/src/jsxml.cpp
@@ +1762,5 @@
>  
>      {
>          Parser parser(cx);
>          if (parser.init(chars, length, filename, lineno, cx->findVersion())) {
> +            JSObject *scopeChain = GetCurrentScopeChain(cx);

IIRC this scope chain can be replaced by the global. If so, then we can fold the GetCurrentScopeChain into js_GetDefaultXMLNamespace. But this can wait another bug.

Comment 9

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/151f190c27ab
https://hg.mozilla.org/integration/mozilla-inbound/rev/d749864ebd01

Comment 10

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/151f190c27ab
https://hg.mozilla.org/mozilla-central/rev/d749864ebd01

Comment 11

[Tom S [:evilpie]]

Short notice at https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_GetScopeChain, that suggests using JS_GetGlobalForScopeChain.
Added entry to 1.8.7 release notes.