Closed Bug 620316 Opened 9 years ago Closed 9 years ago
Escaping Closure and js::Get Call Var Checked are dead code
It's unused right now, Brendan claims fallout from bug 558451. Should it be used, or can it be removed?
We should be able to write a testcase (or find one from the bug that introduced it: bug 496790) and make it bark on a build before the patch-stack for bug 558451 landed. Checking... /be
I added JS_ASSERT(0) to the top of WrapEscapingClosure in jsfun.cpp and ran the test suite. No failures. I think it has been dead since JSOP_UPVAR was removed (bug 592202). JSOP_UPVAR was used in closures on the assumption that they could not escape. Now we emit JSOP_GETFCSLOT or JSOP_NAME in such cases, and those are harmless (though JSOP_NAME gives the wrong answer). If WrapEscapingClosure is dead, GetCallVarChecked is definitely dead, as are the _DBG opcodes. There may be more dead code here, but that's enough for one bug. Taking.
Assignee: brendan → jorendorff
Summary: Use js::GetCallVarChecked or remove it → WrapEscapingClosure and js::GetCallVarChecked are dead code
This passes tests just fine, but I can't think of any sane reason for keeping the needsWrapper assertions around, so I'm going to delete those too.
Comment on attachment 544077 [details] [diff] [review] v2 Whew, that lightened the load. Should have cleaned this up when UPVAR went, thanks for doing it now. Rather than leave JSOP_UNUSED23 around, I'd just slide down the ops above. /be
Attachment #544077 - Flags: review?(brendan) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.