Closed Bug 324592 Opened 15 years ago Closed 15 years ago
_Leave Local Root Scope With Result API
and internal equivalent, so we can pop a local root scope without leaving a return value or similar such "result" unrooted. /be
Igor, please feel free to review too. The js_LeaveLocalRootScopeWithResult code needs to be read in full, so I used more unified diff context. /be
Comment on attachment 209539 [details] [diff] [review] proposed fix Throughout this patch, you use JSVAL_NULL as a "not a real return" value. I think that it's better to use JSVAL_VOID, since that isn't a gcthing, and therefore, won't be pushed onto the stack. >Index: jsxml.c >@@ -1466,33 +1466,33 @@ ParseNodeToXML(JSContext *cx, JSParseNod > if (js_PushLocalRoot(cx, cx->localRootStack, (jsval)xml) < 0) > goto fail; This is no longer necessary, right?
Thanks, how's this? /be
Comment on attachment 209670 [details] [diff] [review] proposed fix, v2 I'll never understand your reluctance to use JSVAL_VOID, but it doesn't matter, so r=mrbkap.
Attachment #209670 - Flags: review?(mrbkap) → review+
(In reply to comment #4) > (From update of attachment 209670 [details] [diff] [review] ) > I'll never understand your reluctance to use JSVAL_VOID, but it doesn't matter, > so r=mrbkap. Simple: JSVAL_VOID takes some work to synthesize (via immediate operands) or load (from a 32-bit constant). JSVAL_NULL is 0. So the test in the callee is cheaper than all the uses in the callers. /be
Checked into trunk. /be
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Landed on the 1.8 branch.
You need to log in before you can comment on or make changes to this bug.