remove StackSegment::initialVarOb / JSOPTION_VAROBJFIX ?

RESOLVED WONTFIX

Status

()

Core
JavaScript Engine
RESOLVED WONTFIX
7 years ago
7 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Created attachment 478510 [details] [diff] [review]
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.
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
(Assignee)

Comment 2

7 years ago
Comment on attachment 478510 [details] [diff] [review]
patch

I was hoping you'd say that :)
Attachment #478510 - Flags: review?(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
Attachment #478510 - Flags: review?(brendan) → review-
(Assignee)

Comment 4

7 years ago
Sounds good.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 5

7 years ago
For posterity, the !JSOPTION_VAROBJFIX use case:
http://groups.google.com/group/mozilla.dev.tech.js-engine/msg/f6a4b35232ff1572
You need to log in before you can comment on or make changes to this bug.