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.
Created attachment 553986 [details] [diff] [review]
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]
>+ 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: