Closed
Bug 759107
Opened 12 years ago
Closed 11 years ago
SM: Convert engine to use StackIter and ScriptFrameIter
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: djvj, Assigned: djvj)
References
Details
(Whiteboard: [js:t])
Attachments
(1 file)
1.66 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
Currently the engine in many places directly grabs the current frame pointer using js_GetTopStackFrame. These should all be changed to use the proper iterators. See https://bugzilla.mozilla.org/show_bug.cgi?id=757787 for discussion.
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #627731 -
Flags: review?(nicolas.b.pierron)
Comment 2•12 years ago
|
||
Comment on attachment 627731 [details] [diff] [review] Patch to assert js_GetTopStackFrame when called from Ion compiled frames Review of attachment 627731 [details] [diff] [review]: ----------------------------------------------------------------- Apply nits, and r+. ::: js/src/jscntxt.cpp @@ +941,5 @@ > { > return cx->hasfp() ? cx->regs().pc : NULL; > } > > +#ifdef DEBUG you might also want to add ifdef JS_ION. This is not cirtical, because StackIter is totally lacking them for the moment and they are easy fix reported by the compiler. ::: js/src/jscntxtinlines.h @@ +599,5 @@ > js::mjit::ExpandInlineFrames(cx->compartment); > #endif > > +#ifdef JS_ION > +# ifdef DEBUG nit: only one space indentation, based on other files around. @@ +600,5 @@ > #endif > > +#ifdef JS_ION > +# ifdef DEBUG > + bool js_InIonFrame(JSContext *cx); nit: Declare this function out side the body of js_GetTopStackFrame, this is not guarantee to work on all compilers.
Attachment #627731 -
Flags: review?(nicolas.b.pierron) → review+
Updated•12 years ago
|
Whiteboard: [js:t]
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 3•11 years ago
|
||
No longer relevant as js_GetTopStackFrame was eliminated.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•