599585 - remove StackSegment::initialVarOb / JSOPTION_VAROBJFIX ?

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Attachment: patch

Bug 535656 removed JSStackFrame::varobj by having JSStackFrame::varobj() either return the call object (for function frames) or the current segment's "initial variables object" (for global frames).  But really, the "initial variables object" is *almost* just the global object of a global frame's scope chain, so it would be a nice simplification to do away with this whole "initial variables object" business and use scopeChain().getGlobal() instead.  There may have originally been some hot-ish access paths that, but I think dvander removed them when he souped up global name access and the attached patch does not affect SS or V8 on my old thinkpad.

The only caveat warranting the *almost* above is this kooky case where JSOPTION_VAROBJFIX is not set and js::Execute is called with a scope chain that is not a global object.  In this case, the proposed change would effectively apply JSOPTION_VAROBJFIX.  So, can we just remove JSOPTION_VAROBJFIX and have it be the implicit default behavior?  jsapi.h seems to already have been warning against the kooky case.

Comment 1

[Brendan Eich [:brendan]]

Please!

m.d.t.js-engine post as usual, you'll get nought but crickets, although I know of embeddings from the first age will break if they ever upgrade. But that will be as good for them as you fixing this bug will be for us!

/be

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 478510 [details] [diff] [review]
patch

I was hoping you'd say that :)

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 478510 [details] [diff] [review]
patch

I was wrong about crickets, and ES5 strict mode or equivalent does combine well with lack of JSOPTION_VAROBJFIX to create layered scopes. Let's not bother if this is not a perf win. We can kill it later with modules, which will require a serious scope re-do.

/be

Comment 4

[Luke Wagner [:luke]]

Sounds good.

Comment 5

[Luke Wagner [:luke]]

For posterity, the !JSOPTION_VAROBJFIX use case:
http://groups.google.com/group/mozilla.dev.tech.js-engine/msg/f6a4b35232ff1572