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.

Comment on attachment 553986 [details] [diff] [review] v1 >+ uintN hasUpvars = false; Why is this an int?

(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