Closed Bug 669369 Opened 9 years ago Closed 9 years ago
Try simplifying Parser::set
Function Kinds and removing funarg analysis
In bug 592202 comment 52 I wrote: > I'd like another follow-up bug to look at the `if (!fn->isFunArg())` block > in Parser::setFunctionKinds. I bet those two blocks of code (the if block and > the else block) can be combined into something shorter and saner. Brendan replied in comment 55: > As mentioned on IRC, that condition is a stricter one than the condition we > need now: that the function isn't induced by a hoisted declaration form. We > cannot flatten non-lambdas. ...which is not total disagreement, so I think I will try it. There's also this, in CanFlattenUpvar: > /* > * If this function is reaching up across an enclosing funarg, then we > * cannot copy dn's value into a flat closure slot (the display stops > * working once the funarg escapes). > */ > if (!afunbox || afunbox->node->isFunArg()) > return false; This looks bogus to me; I think it can be deleted. If setFunctionKinds and CanFlattenUpvar can be simplified this way, then the funarg analysis can be removed.
This patch makes me happy. As long as the implementation of js::GetUpvar is to walk the stack, we can't get rid of the funarg analysis.
Assignee: general → jorendorff
Attachment #553986 - Flags: review?(dmandelin)
Attachment #553986 - Flags: review?(dmandelin) → review+
Comment on attachment 553986 [details] [diff] [review] v1 >+ uintN hasUpvars = false; Why is this an int?
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
(In reply to Ms2ger from comment #3) > Comment on attachment 553986 [details] [diff] [review] > >+ uintN hasUpvars = false; > Why is this an int? Oh, it shouldn't be. The obvious fix is now inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/0504a441fef7
Version: Other Branch → Trunk
You need to log in before you can comment on or make changes to this bug.